Skip to content

Commit 5fe603c

Browse files
estebankestebank
committed
Assert that the first assert! expression is bool
In the desugaring of `assert!`, assign the condition expression to a `bool` biding in order to provide better type errors when passed the wrong thing. The span will point only at the expression, and not the whole `assert!` invokation. ``` error[E0308]: mismatched types --> $DIR/issue-14091.rs:2:13 | LL | assert!(1,1); | ^ | | | expected `bool`, found integer | expected due to this ``` We no longer mention the expression needing to implement the `Not` trait. ``` error[E0308]: mismatched types --> $DIR/issue-14091-2.rs:15:13 | LL | assert!(x, x); | ^ | | | expected `bool`, found `BytePos` | expected due to this ``` Fix rust-lang#122159.
1 parent dee7d0e commit 5fe603c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+216
-169
lines changed

‎compiler/rustc_builtin_macros/src/assert.rs

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ mod context;
33
use rustc_ast::ptr::P;
44
use rustc_ast::token::Delimiter;
55
use rustc_ast::tokenstream::{DelimSpan, TokenStream};
6-
use rustc_ast::{DelimArgs, Expr, ExprKind, MacCall, Path, PathSegment, UnOp, token};
6+
use rustc_ast::{self as ast, DelimArgs, Expr, ExprKind, MacCall, Path, PathSegment, UnOp, token};
77
use rustc_ast_pretty::pprust;
88
use rustc_errors::PResult;
99
use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacEager, MacroExpanderResult};
@@ -20,7 +20,7 @@ pub(crate) fn expand_assert<'cx>(
2020
span: Span,
2121
tts: TokenStream,
2222
) -> MacroExpanderResult<'cx> {
23-
let Assert { cond_expr, custom_message } = match parse_assert(cx, span, tts) {
23+
let Assert { cond_expr, inner_cond_expr, custom_message } = match parse_assert(cx, span, tts) {
2424
Ok(assert) => assert,
2525
Err(err) => {
2626
let guar = err.emit();
@@ -70,7 +70,9 @@ pub(crate) fn expand_assert<'cx>(
7070
//
7171
// FIXME(c410-f3r) See https://github.com/rust-lang/rust/issues/96949
7272
else if cx.ecfg.features.generic_assert() {
73-
context::Context::new(cx, call_site_span).build(cond_expr, panic_path())
73+
// FIXME(estebank): we use the condition the user passed without coercing to `bool` when
74+
// `generic_assert` is enabled, but we could use `cond_expr` instead.
75+
context::Context::new(cx, call_site_span).build(inner_cond_expr, panic_path())
7476
}
7577
// If `generic_assert` is not enabled, only outputs a literal "assertion failed: ..."
7678
// string
@@ -85,7 +87,7 @@ pub(crate) fn expand_assert<'cx>(
8587
DUMMY_SP,
8688
Symbol::intern(&format!(
8789
"assertion failed: {}",
88-
pprust::expr_to_string(&cond_expr)
90+
pprust::expr_to_string(&inner_cond_expr)
8991
)),
9092
)],
9193
);
@@ -95,8 +97,12 @@ pub(crate) fn expand_assert<'cx>(
9597
ExpandResult::Ready(MacEager::expr(expr))
9698
}
9799

100+
// `assert!($cond_expr, $custom_message)`
98101
struct Assert {
102+
// `{ let assert_macro: bool = $cond_expr; assert_macro }`
99103
cond_expr: P<Expr>,
104+
// We keep the condition without the `bool` coercion for the panic message.
105+
inner_cond_expr: P<Expr>,
100106
custom_message: Option<TokenStream>,
101107
}
102108

@@ -118,7 +124,7 @@ fn parse_assert<'a>(cx: &ExtCtxt<'a>, sp: Span, stream: TokenStream) -> PResult<
118124
return Err(cx.dcx().create_err(errors::AssertRequiresBoolean { span: sp }));
119125
}
120126

121-
let cond_expr = parser.parse_expr()?;
127+
let inner_cond_expr = parser.parse_expr()?;
122128

123129
// Some crates use the `assert!` macro in the following form (note extra semicolon):
124130
//
@@ -154,7 +160,54 @@ fn parse_assert<'a>(cx: &ExtCtxt<'a>, sp: Span, stream: TokenStream) -> PResult<
154160
parser.unexpected()?;
155161
}
156162

157-
Ok(Assert { cond_expr, custom_message })
163+
let cond_expr = expand_cond(cx, parser, inner_cond_expr.clone());
164+
Ok(Assert { cond_expr, inner_cond_expr, custom_message })
165+
}
166+
167+
fn expand_cond(cx: &ExtCtxt<'_>, parser: Parser<'_>, cond_expr: P<Expr>) -> P<Expr> {
168+
let span = cx.with_call_site_ctxt(cond_expr.span);
169+
// Coerce the expression to `bool` for more accurate errors. If `assert!` is passed an
170+
// expression that isn't `bool`, the type error will point at only the expression and not the
171+
// entire macro call. If a non-`bool` is passed that doesn't implement `trait Not`, we won't
172+
// talk about traits, we'll just state the appropriate type error.
173+
// `let assert_macro: bool = $expr;`
174+
let ident = Ident::new(sym::assert_macro, span);
175+
let local = P(ast::Local {
176+
ty: Some(P(ast::Ty {
177+
kind: ast::TyKind::Path(None, ast::Path::from_ident(Ident::new(sym::bool, span))),
178+
id: ast::DUMMY_NODE_ID,
179+
span,
180+
tokens: None,
181+
})),
182+
pat: parser.mk_pat_ident(span, ast::BindingAnnotation::NONE, ident),
183+
kind: ast::LocalKind::Init(cond_expr),
184+
id: ast::DUMMY_NODE_ID,
185+
span,
186+
colon_sp: None,
187+
attrs: Default::default(),
188+
tokens: None,
189+
});
190+
// `{ let assert_macro: bool = $expr; assert_macro }`
191+
parser.mk_expr(
192+
span,
193+
ast::ExprKind::Block(
194+
parser.mk_block(
195+
thin_vec![
196+
parser.mk_stmt(span, ast::StmtKind::Let(local)),
197+
parser.mk_stmt(
198+
span,
199+
ast::StmtKind::Expr(parser.mk_expr(
200+
span,
201+
ast::ExprKind::Path(None, ast::Path::from_ident(ident))
202+
)),
203+
),
204+
],
205+
ast::BlockCheckMode::Default,
206+
span,
207+
),
208+
None,
209+
),
210+
)
158211
}
159212

160213
fn parse_custom_message(parser: &mut Parser<'_>) -> Option<TokenStream> {

‎compiler/rustc_parse/src/parser/expr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3860,7 +3860,7 @@ impl<'a> Parser<'a> {
38603860
P(Expr { kind, span, attrs, id: DUMMY_NODE_ID, tokens: None })
38613861
}
38623862

3863-
pub(crate) fn mk_expr(&self, span: Span, kind: ExprKind) -> P<Expr> {
3863+
pub fn mk_expr(&self, span: Span, kind: ExprKind) -> P<Expr> {
38643864
self.mk_expr_with_attrs(span, kind, AttrVec::new())
38653865
}
38663866

‎compiler/rustc_parse/src/parser/pat.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1732,6 +1732,7 @@ impl<'a> Parser<'a> {
17321732
})
17331733
}
17341734

1735+
// `pub` because we use it from `rustc_builtin_macros`.
17351736
pub(super) fn mk_pat_ident(&self, span: Span, ann: BindingMode, ident: Ident) -> P<Pat> {
17361737
self.mk_pat(span, PatKind::Ident(ann, ident, None))
17371738
}

‎compiler/rustc_parse/src/parser/stmt.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -988,12 +988,7 @@ impl<'a> Parser<'a> {
988988
Ok(Some(stmt))
989989
}
990990

991-
pub(super) fn mk_block(
992-
&self,
993-
stmts: ThinVec<Stmt>,
994-
rules: BlockCheckMode,
995-
span: Span,
996-
) -> P<Block> {
991+
pub fn mk_block(&self, stmts: ThinVec<Stmt>, rules: BlockCheckMode, span: Span) -> P<Block> {
997992
P(Block {
998993
stmts,
999994
id: DUMMY_NODE_ID,
@@ -1004,7 +999,7 @@ impl<'a> Parser<'a> {
1004999
})
10051000
}
10061001

1007-
pub(super) fn mk_stmt(&self, span: Span, kind: StmtKind) -> Stmt {
1002+
pub fn mk_stmt(&self, span: Span, kind: StmtKind) -> Stmt {
10081003
Stmt { id: DUMMY_NODE_ID, kind, span }
10091004
}
10101005

‎library/core/tests/bool.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,14 @@ fn test_bool() {
7171
#[test]
7272
pub fn test_bool_not() {
7373
if !false {
74-
assert!((true));
74+
assert!(true);
7575
} else {
76-
assert!((false));
76+
assert!(false);
7777
}
7878
if !true {
79-
assert!((false));
79+
assert!(false);
8080
} else {
81-
assert!((true));
81+
assert!(true);
8282
}
8383
}
8484

‎library/core/tests/ptr.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ fn test() {
4242
let mut v1 = vec![0u16, 0u16, 0u16];
4343

4444
copy(v0.as_ptr().offset(1), v1.as_mut_ptr().offset(1), 1);
45-
assert!((v1[0] == 0u16 && v1[1] == 32001u16 && v1[2] == 0u16));
45+
assert!(v1[0] == 0u16 && v1[1] == 32001u16 && v1[2] == 0u16);
4646
copy(v0.as_ptr().offset(2), v1.as_mut_ptr(), 1);
47-
assert!((v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 0u16));
47+
assert!(v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 0u16);
4848
copy(v0.as_ptr(), v1.as_mut_ptr().offset(2), 1);
49-
assert!((v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 32000u16));
49+
assert!(v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 32000u16);
5050
}
5151
}
5252

‎library/std/src/env/tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ fn test_self_exe_path() {
1313

1414
#[test]
1515
fn test() {
16-
assert!((!Path::new("test-path").is_absolute()));
16+
assert!(!Path::new("test-path").is_absolute());
1717

1818
#[cfg(not(target_env = "sgx"))]
1919
current_dir().unwrap();

‎library/std/tests/istr.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ fn test_stack_assign() {
55
let t: String = "a".to_string();
66
assert_eq!(s, t);
77
let u: String = "b".to_string();
8-
assert!((s != u));
8+
assert!(s != u);
99
}
1010

1111
#[test]
@@ -19,7 +19,7 @@ fn test_heap_assign() {
1919
let t: String = "a big ol' string".to_string();
2020
assert_eq!(s, t);
2121
let u: String = "a bad ol' string".to_string();
22-
assert!((s != u));
22+
assert!(s != u);
2323
}
2424

2525
#[test]

‎library/std/tests/seq-compare.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
#[test]
22
fn seq_compare() {
3-
assert!(("hello".to_string() < "hellr".to_string()));
4-
assert!(("hello ".to_string() > "hello".to_string()));
5-
assert!(("hello".to_string() != "there".to_string()));
6-
assert!((vec![1, 2, 3, 4] > vec![1, 2, 3]));
7-
assert!((vec![1, 2, 3] < vec![1, 2, 3, 4]));
8-
assert!((vec![1, 2, 4, 4] > vec![1, 2, 3, 4]));
9-
assert!((vec![1, 2, 3, 4] < vec![1, 2, 4, 4]));
10-
assert!((vec![1, 2, 3] <= vec![1, 2, 3]));
11-
assert!((vec![1, 2, 3] <= vec![1, 2, 3, 3]));
12-
assert!((vec![1, 2, 3, 4] > vec![1, 2, 3]));
3+
assert!("hello".to_string() < "hellr".to_string());
4+
assert!("hello ".to_string() > "hello".to_string());
5+
assert!("hello".to_string() != "there".to_string());
6+
assert!(vec![1, 2, 3, 4] > vec![1, 2, 3]);
7+
assert!(vec![1, 2, 3] < vec![1, 2, 3, 4]);
8+
assert!(vec![1, 2, 4, 4] > vec![1, 2, 3, 4]);
9+
assert!(vec![1, 2, 3, 4] < vec![1, 2, 4, 4]);
10+
assert!(vec![1, 2, 3] <= vec![1, 2, 3]);
11+
assert!(vec![1, 2, 3] <= vec![1, 2, 3, 3]);
12+
assert!(vec![1, 2, 3, 4] > vec![1, 2, 3]);
1313
assert_eq!(vec![1, 2, 3], vec![1, 2, 3]);
14-
assert!((vec![1, 2, 3] != vec![1, 1, 3]));
14+
assert!(vec![1, 2, 3] != vec![1, 1, 3]);
1515
}

‎src/tools/miri/tests/pass/binops.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,23 @@
22

33
fn test_nil() {
44
assert_eq!((), ());
5-
assert!((!(() != ())));
6-
assert!((!(() < ())));
7-
assert!((() <= ()));
8-
assert!((!(() > ())));
9-
assert!((() >= ()));
5+
assert!(!(() != ()));
6+
assert!(!(() < ()));
7+
assert!(() <= ());
8+
assert!(!(() > ()));
9+
assert!(() >= ());
1010
}
1111

1212
fn test_bool() {
13-
assert!((!(true < false)));
14-
assert!((!(true <= false)));
15-
assert!((true > false));
16-
assert!((true >= false));
13+
assert!(!(true < false));
14+
assert!(!(true <= false));
15+
assert!(true > false);
16+
assert!(true >= false);
1717

18-
assert!((false < true));
19-
assert!((false <= true));
20-
assert!((!(false > true)));
21-
assert!((!(false >= true)));
18+
assert!(false < true);
19+
assert!(false <= true);
20+
assert!(!(false > true));
21+
assert!(!(false >= true));
2222

2323
// Bools support bitwise binops
2424
assert_eq!(false & false, false);

‎tests/ui/binding/expr-match-generic.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ type compare<T> = extern "Rust" fn(T, T) -> bool;
55

66
fn test_generic<T:Clone>(expected: T, eq: compare<T>) {
77
let actual: T = match true { true => { expected.clone() }, _ => panic!("wat") };
8-
assert!((eq(expected, actual)));
8+
assert!(eq(expected, actual));
99
}
1010

1111
fn test_bool() {

‎tests/ui/binding/expr-match.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@
77

88
fn test_basic() {
99
let mut rs: bool = match true { true => { true } false => { false } };
10-
assert!((rs));
10+
assert!(rs);
1111
rs = match false { true => { false } false => { true } };
12-
assert!((rs));
12+
assert!(rs);
1313
}
1414

1515
fn test_inferrence() {
1616
let rs = match true { true => { true } false => { false } };
17-
assert!((rs));
17+
assert!(rs);
1818
}
1919

2020
fn test_alt_as_alt_head() {
@@ -25,7 +25,7 @@ fn test_alt_as_alt_head() {
2525
true => { false }
2626
false => { true }
2727
};
28-
assert!((rs));
28+
assert!(rs);
2929
}
3030

3131
fn test_alt_as_block_result() {
@@ -34,7 +34,7 @@ fn test_alt_as_block_result() {
3434
true => { false }
3535
false => { match true { true => { true } false => { false } } }
3636
};
37-
assert!((rs));
37+
assert!(rs);
3838
}
3939

4040
pub fn main() {

‎tests/ui/binop/binops.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,23 @@
55

66
fn test_nil() {
77
assert_eq!((), ());
8-
assert!((!(() != ())));
9-
assert!((!(() < ())));
10-
assert!((() <= ()));
11-
assert!((!(() > ())));
12-
assert!((() >= ()));
8+
assert!(!(() != ()));
9+
assert!(!(() < ()));
10+
assert!(() <= ());
11+
assert!(!(() > ()));
12+
assert!(() >= ());
1313
}
1414

1515
fn test_bool() {
16-
assert!((!(true < false)));
17-
assert!((!(true <= false)));
18-
assert!((true > false));
19-
assert!((true >= false));
16+
assert!(!(true < false));
17+
assert!(!(true <= false));
18+
assert!(true > false);
19+
assert!(true >= false);
2020

21-
assert!((false < true));
22-
assert!((false <= true));
23-
assert!((!(false > true)));
24-
assert!((!(false >= true)));
21+
assert!(false < true);
22+
assert!(false <= true);
23+
assert!(!(false > true));
24+
assert!(!(false >= true));
2525

2626
// Bools support bitwise binops
2727
assert_eq!(false & false, false);
@@ -76,9 +76,9 @@ fn test_class() {
7676
}
7777
assert_eq!(q, r);
7878
r.y = 17;
79-
assert!((r.y != q.y));
79+
assert!(r.y != q.y);
8080
assert_eq!(r.y, 17);
81-
assert!((q != r));
81+
assert!(q != r);
8282
}
8383

8484
pub fn main() {

‎tests/ui/binop/structured-compare.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ pub fn main() {
1717
let a = (1, 2, 3);
1818
let b = (1, 2, 3);
1919
assert_eq!(a, b);
20-
assert!((a != (1, 2, 4)));
21-
assert!((a < (1, 2, 4)));
22-
assert!((a <= (1, 2, 4)));
23-
assert!(((1, 2, 4) > a));
24-
assert!(((1, 2, 4) >= a));
20+
assert!(a != (1, 2, 4));
21+
assert!(a < (1, 2, 4));
22+
assert!(a <= (1, 2, 4));
23+
assert!((1, 2, 4) > a);
24+
assert!((1, 2, 4) >= a);
2525
let x = foo::large;
2626
let y = foo::small;
27-
assert!((x != y));
27+
assert!(x != y);
2828
assert_eq!(x, foo::large);
29-
assert!((x != foo::small));
29+
assert!(x != foo::small);
3030
}

0 commit comments

Comments
 (0)