Skip to content

Commit 3737314

Browse files
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 22e241e commit 3737314

Some content is hidden

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

44 files changed

+219
-172
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);

src/tools/rust-analyzer/crates/parser/test_data/parser/ok/0035_weird_exprs.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ fn notsure() {
6464

6565
fn canttouchthis() -> usize {
6666
fn p() -> bool { true }
67-
let _a = (assert!((true)) == (assert!(p())));
68-
let _c = (assert!((p())) == ());
67+
let _a = (assert!(true) == (assert!(p())));
68+
let _c = (assert!(p()) == ());
6969
let _b: bool = (println!("{}", 0) == (return 0));
7070
}
7171

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() {

0 commit comments

Comments
 (0)