Skip to content

Commit 670133e

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 #122159.
1 parent 3cdcdaf commit 670133e

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

+217
-170
lines changed

‎compiler/rustc_builtin_macros/src/assert.rs

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ mod context;
33
use crate::edition_panic::use_panic_2021;
44
use crate::errors;
55
use rustc_ast::ptr::P;
6-
use rustc_ast::token;
76
use rustc_ast::token::Delimiter;
87
use rustc_ast::tokenstream::{DelimSpan, TokenStream};
8+
use rustc_ast::{self as ast, token};
99
use rustc_ast::{DelimArgs, Expr, ExprKind, MacCall, Path, PathSegment, UnOp};
1010
use rustc_ast_pretty::pprust;
1111
use rustc_errors::PResult;
@@ -20,7 +20,7 @@ pub 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 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 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 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: &mut ExtCtxt<'a>, sp: Span, stream: TokenStream) -> PRes
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: &mut ExtCtxt<'a>, sp: Span, stream: TokenStream) -> PRes
154160
return 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
@@ -3775,7 +3775,7 @@ impl<'a> Parser<'a> {
37753775
P(Expr { kind, span, attrs, id: DUMMY_NODE_ID, tokens: None })
37763776
}
37773777

3778-
pub(crate) fn mk_expr(&self, span: Span, kind: ExprKind) -> P<Expr> {
3778+
pub fn mk_expr(&self, span: Span, kind: ExprKind) -> P<Expr> {
37793779
self.mk_expr_with_attrs(span, kind, AttrVec::new())
37803780
}
37813781

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1399,7 +1399,8 @@ impl<'a> Parser<'a> {
13991399
})
14001400
}
14011401

1402-
pub(super) fn mk_pat_ident(&self, span: Span, ann: BindingAnnotation, ident: Ident) -> P<Pat> {
1402+
// `pub` because we use it from `rustc_builtin_macros`.
1403+
pub fn mk_pat_ident(&self, span: Span, ann: BindingAnnotation, ident: Ident) -> P<Pat> {
14031404
self.mk_pat(span, PatKind::Ident(ann, ident, None))
14041405
}
14051406

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -833,12 +833,7 @@ impl<'a> Parser<'a> {
833833
Ok(Some(stmt))
834834
}
835835

836-
pub(super) fn mk_block(
837-
&self,
838-
stmts: ThinVec<Stmt>,
839-
rules: BlockCheckMode,
840-
span: Span,
841-
) -> P<Block> {
836+
pub fn mk_block(&self, stmts: ThinVec<Stmt>, rules: BlockCheckMode, span: Span) -> P<Block> {
842837
P(Block {
843838
stmts,
844839
id: DUMMY_NODE_ID,
@@ -849,7 +844,7 @@ impl<'a> Parser<'a> {
849844
})
850845
}
851846

852-
pub(super) fn mk_stmt(&self, span: Span, kind: StmtKind) -> Stmt {
847+
pub fn mk_stmt(&self, span: Span, kind: StmtKind) -> Stmt {
853848
Stmt { id: DUMMY_NODE_ID, kind, span }
854849
}
855850

‎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
@@ -41,11 +41,11 @@ fn test() {
4141
let mut v1 = vec![0u16, 0u16, 0u16];
4242

4343
copy(v0.as_ptr().offset(1), v1.as_mut_ptr().offset(1), 1);
44-
assert!((v1[0] == 0u16 && v1[1] == 32001u16 && v1[2] == 0u16));
44+
assert!(v1[0] == 0u16 && v1[1] == 32001u16 && v1[2] == 0u16);
4545
copy(v0.as_ptr().offset(2), v1.as_mut_ptr(), 1);
46-
assert!((v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 0u16));
46+
assert!(v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 0u16);
4747
copy(v0.as_ptr(), v1.as_mut_ptr().offset(2), 1);
48-
assert!((v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 32000u16));
48+
assert!(v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 32000u16);
4949
}
5050
}
5151

‎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();

‎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
}

‎tests/ui/cfg/conditional-compile.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ pub fn main() {
8585
pub fn main() {
8686
// Exercise some of the configured items in ways that wouldn't be possible
8787
// if they had the bogus definition
88-
assert!((b));
88+
assert!(b);
8989
let _x: t = true;
9090
let _y: tg = tg::bar;
9191

‎tests/ui/codemap_tests/issue-28308.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
fn main() {
22
assert!("foo");
3-
//~^ ERROR cannot apply unary operator `!`
3+
//~^ ERROR mismatched types
44
}

‎tests/ui/codemap_tests/issue-28308.stderr

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
error[E0600]: cannot apply unary operator `!` to type `&'static str`
2-
--> $DIR/issue-28308.rs:2:5
1+
error[E0308]: mismatched types
2+
--> $DIR/issue-28308.rs:2:13
33
|
44
LL | assert!("foo");
5-
| ^^^^^^^^^^^^^^ cannot apply unary operator `!`
6-
|
7-
= note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)
5+
| ^^^^^
6+
| |
7+
| expected `bool`, found `&str`
8+
| expected due to this
89

910
error: aborting due to 1 previous error
1011

11-
For more information about this error, try `rustc --explain E0600`.
12+
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)