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.

Author: estebank

compiler/rustc_builtin_macros/src/assert.rs

@@ -3,9 +3,9 @@ mod context;
 use crate::edition_panic::use_panic_2021;
 use crate::errors;
 use rustc_ast::ptr::P;
-use rustc_ast::token;
 use rustc_ast::token::Delimiter;
 use rustc_ast::tokenstream::{DelimSpan, TokenStream};
+use rustc_ast::{self as ast, token};
 use rustc_ast::{DelimArgs, Expr, ExprKind, MacCall, Path, PathSegment, UnOp};
 use rustc_ast_pretty::pprust;
 use rustc_errors::PResult;
@@ -20,7 +20,7 @@ pub fn expand_assert<'cx>(
     span: Span,
     tts: TokenStream,
 ) -> MacroExpanderResult<'cx> {
-    let Assert { cond_expr, custom_message } = match parse_assert(cx, span, tts) {
+    let Assert { cond_expr, inner_cond_expr, custom_message } = match parse_assert(cx, span, tts) {
         Ok(assert) => assert,
         Err(err) => {
             let guar = err.emit();
@@ -70,7 +70,9 @@ pub fn expand_assert<'cx>(
     //
     // FIXME(c410-f3r) See https://github.com/rust-lang/rust/issues/96949
     else if cx.ecfg.features.generic_assert {
-        context::Context::new(cx, call_site_span).build(cond_expr, panic_path())
+        // FIXME(estebank): we use the condition the user passed without coercing to `bool` when
+        // `generic_assert` is enabled, but we could use `cond_expr` instead.
+        context::Context::new(cx, call_site_span).build(inner_cond_expr, panic_path())
     }
     // If `generic_assert` is not enabled, only outputs a literal "assertion failed: ..."
     // string
@@ -85,7 +87,7 @@ pub fn expand_assert<'cx>(
                 DUMMY_SP,
                 Symbol::intern(&format!(
                     "assertion failed: {}",
-                    pprust::expr_to_string(&cond_expr)
+                    pprust::expr_to_string(&inner_cond_expr)
                 )),
             )],
         );
@@ -95,8 +97,12 @@ pub fn expand_assert<'cx>(
     ExpandResult::Ready(MacEager::expr(expr))
 }
 
+// `assert!($cond_expr, $custom_message)`
 struct Assert {
+    // `{ let assert_macro: bool = $cond_expr; assert_macro }`
     cond_expr: P<Expr>,
+    // We keep the condition without the `bool` coercion for the panic message.
+    inner_cond_expr: P<Expr>,
     custom_message: Option<TokenStream>,
 }
 
@@ -118,7 +124,7 @@ fn parse_assert<'a>(cx: &mut ExtCtxt<'a>, sp: Span, stream: TokenStream) -> PRes
         return Err(cx.dcx().create_err(errors::AssertRequiresBoolean { span: sp }));
     }
 
-    let cond_expr = parser.parse_expr()?;
+    let inner_cond_expr = parser.parse_expr()?;
 
     // Some crates use the `assert!` macro in the following form (note extra semicolon):
     //
@@ -154,7 +160,54 @@ fn parse_assert<'a>(cx: &mut ExtCtxt<'a>, sp: Span, stream: TokenStream) -> PRes
         return parser.unexpected();
     }
 
-    Ok(Assert { cond_expr, custom_message })
+    let cond_expr = expand_cond(cx, parser, inner_cond_expr.clone());
+    Ok(Assert { cond_expr, inner_cond_expr, custom_message })
+}
+
+fn expand_cond(cx: &ExtCtxt<'_>, parser: Parser<'_>, cond_expr: P<Expr>) -> P<Expr> {
+    let span = cx.with_call_site_ctxt(cond_expr.span);
+    // Coerce the expression to `bool` for more accurate errors. If `assert!` is passed an
+    // expression that isn't `bool`, the type error will point at only the expression and not the
+    // entire macro call. If a non-`bool` is passed that doesn't implement `trait Not`, we won't
+    // talk about traits, we'll just state the appropriate type error.
+    // `let assert_macro: bool = $expr;`
+    let ident = Ident::new(sym::assert_macro, span);
+    let local = P(ast::Local {
+        ty: Some(P(ast::Ty {
+            kind: ast::TyKind::Path(None, ast::Path::from_ident(Ident::new(sym::bool, span))),
+            id: ast::DUMMY_NODE_ID,
+            span,
+            tokens: None,
+        })),
+        pat: parser.mk_pat_ident(span, ast::BindingAnnotation::NONE, ident),
+        kind: ast::LocalKind::Init(cond_expr),
+        id: ast::DUMMY_NODE_ID,
+        span,
+        colon_sp: None,
+        attrs: Default::default(),
+        tokens: None,
+    });
+    // `{ let assert_macro: bool = $expr; assert_macro }`
+    parser.mk_expr(
+        span,
+        ast::ExprKind::Block(
+            parser.mk_block(
+                thin_vec![
+                    parser.mk_stmt(span, ast::StmtKind::Let(local)),
+                    parser.mk_stmt(
+                        span,
+                        ast::StmtKind::Expr(parser.mk_expr(
+                            span,
+                            ast::ExprKind::Path(None, ast::Path::from_ident(ident))
+                        )),
+                    ),
+                ],
+                ast::BlockCheckMode::Default,
+                span,
+            ),
+            None,
+        ),
+    )
 }
 
 fn parse_custom_message(parser: &mut Parser<'_>) -> Option<TokenStream> {

compiler/rustc_parse/src/parser/expr.rs

@@ -3775,7 +3775,7 @@ impl<'a> Parser<'a> {
         P(Expr { kind, span, attrs, id: DUMMY_NODE_ID, tokens: None })
     }
 
-    pub(crate) fn mk_expr(&self, span: Span, kind: ExprKind) -> P<Expr> {
+    pub fn mk_expr(&self, span: Span, kind: ExprKind) -> P<Expr> {
         self.mk_expr_with_attrs(span, kind, AttrVec::new())
     }
 

compiler/rustc_parse/src/parser/pat.rs

@@ -1399,7 +1399,8 @@ impl<'a> Parser<'a> {
         })
     }
 
-    pub(super) fn mk_pat_ident(&self, span: Span, ann: BindingAnnotation, ident: Ident) -> P<Pat> {
+    // `pub` because we use it from `rustc_builtin_macros`.
+    pub fn mk_pat_ident(&self, span: Span, ann: BindingAnnotation, ident: Ident) -> P<Pat> {
         self.mk_pat(span, PatKind::Ident(ann, ident, None))
     }
 

compiler/rustc_parse/src/parser/stmt.rs

@@ -833,12 +833,7 @@ impl<'a> Parser<'a> {
         Ok(Some(stmt))
     }
 
-    pub(super) fn mk_block(
-        &self,
-        stmts: ThinVec<Stmt>,
-        rules: BlockCheckMode,
-        span: Span,
-    ) -> P<Block> {
+    pub fn mk_block(&self, stmts: ThinVec<Stmt>, rules: BlockCheckMode, span: Span) -> P<Block> {
         P(Block {
             stmts,
             id: DUMMY_NODE_ID,
@@ -849,7 +844,7 @@ impl<'a> Parser<'a> {
         })
     }
 
-    pub(super) fn mk_stmt(&self, span: Span, kind: StmtKind) -> Stmt {
+    pub fn mk_stmt(&self, span: Span, kind: StmtKind) -> Stmt {
         Stmt { id: DUMMY_NODE_ID, kind, span }
     }
 

library/core/tests/bool.rs

@@ -71,14 +71,14 @@ fn test_bool() {
 #[test]
 pub fn test_bool_not() {
     if !false {
-        assert!((true));
+        assert!(true);
     } else {
-        assert!((false));
+        assert!(false);
     }
     if !true {
-        assert!((false));
+        assert!(false);
     } else {
-        assert!((true));
+        assert!(true);
     }
 }
 

library/core/tests/ptr.rs

@@ -41,11 +41,11 @@ fn test() {
         let mut v1 = vec![0u16, 0u16, 0u16];
 
         copy(v0.as_ptr().offset(1), v1.as_mut_ptr().offset(1), 1);
-        assert!((v1[0] == 0u16 && v1[1] == 32001u16 && v1[2] == 0u16));
+        assert!(v1[0] == 0u16 && v1[1] == 32001u16 && v1[2] == 0u16);
         copy(v0.as_ptr().offset(2), v1.as_mut_ptr(), 1);
-        assert!((v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 0u16));
+        assert!(v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 0u16);
         copy(v0.as_ptr(), v1.as_mut_ptr().offset(2), 1);
-        assert!((v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 32000u16));
+        assert!(v1[0] == 32002u16 && v1[1] == 32001u16 && v1[2] == 32000u16);
     }
 }
 

library/std/src/env/tests.rs

@@ -13,7 +13,7 @@ fn test_self_exe_path() {
 
 #[test]
 fn test() {
-    assert!((!Path::new("test-path").is_absolute()));
+    assert!(!Path::new("test-path").is_absolute());
 
     #[cfg(not(target_env = "sgx"))]
     current_dir().unwrap();

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

@@ -2,23 +2,23 @@
 
 fn test_nil() {
     assert_eq!((), ());
-    assert!((!(() != ())));
-    assert!((!(() < ())));
-    assert!((() <= ()));
-    assert!((!(() > ())));
-    assert!((() >= ()));
+    assert!(!(() != ()));
+    assert!(!(() < ()));
+    assert!(() <= ());
+    assert!(!(() > ()));
+    assert!(() >= ());
 }
 
 fn test_bool() {
-    assert!((!(true < false)));
-    assert!((!(true <= false)));
-    assert!((true > false));
-    assert!((true >= false));
+    assert!(!(true < false));
+    assert!(!(true <= false));
+    assert!(true > false);
+    assert!(true >= false);
 
-    assert!((false < true));
-    assert!((false <= true));
-    assert!((!(false > true)));
-    assert!((!(false >= true)));
+    assert!(false < true);
+    assert!(false <= true);
+    assert!(!(false > true));
+    assert!(!(false >= true));
 
     // Bools support bitwise binops
     assert_eq!(false & false, false);

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

@@ -5,7 +5,7 @@ type compare<T> = extern "Rust" fn(T, T) -> bool;
 
 fn test_generic<T:Clone>(expected: T, eq: compare<T>) {
   let actual: T = match true { true => { expected.clone() }, _ => panic!("wat") };
-    assert!((eq(expected, actual)));
+    assert!(eq(expected, actual));
 }
 
 fn test_bool() {

tests/ui/binding/expr-match.rs

@@ -7,14 +7,14 @@
 
 fn test_basic() {
     let mut rs: bool = match true { true => { true } false => { false } };
-    assert!((rs));
+    assert!(rs);
     rs = match false { true => { false } false => { true } };
-    assert!((rs));
+    assert!(rs);
 }
 
 fn test_inferrence() {
     let rs = match true { true => { true } false => { false } };
-    assert!((rs));
+    assert!(rs);
 }
 
 fn test_alt_as_alt_head() {
@@ -25,7 +25,7 @@ fn test_alt_as_alt_head() {
           true => { false }
           false => { true }
         };
-    assert!((rs));
+    assert!(rs);
 }
 
 fn test_alt_as_block_result() {
@@ -34,7 +34,7 @@ fn test_alt_as_block_result() {
           true => { false }
           false => { match true { true => { true } false => { false } } }
         };
-    assert!((rs));
+    assert!(rs);
 }
 
 pub fn main() {

tests/ui/binop/binops.rs

@@ -5,23 +5,23 @@
 
 fn test_nil() {
     assert_eq!((), ());
-    assert!((!(() != ())));
-    assert!((!(() < ())));
-    assert!((() <= ()));
-    assert!((!(() > ())));
-    assert!((() >= ()));
+    assert!(!(() != ()));
+    assert!(!(() < ()));
+    assert!(() <= ());
+    assert!(!(() > ()));
+    assert!(() >= ());
 }
 
 fn test_bool() {
-    assert!((!(true < false)));
-    assert!((!(true <= false)));
-    assert!((true > false));
-    assert!((true >= false));
+    assert!(!(true < false));
+    assert!(!(true <= false));
+    assert!(true > false);
+    assert!(true >= false);
 
-    assert!((false < true));
-    assert!((false <= true));
-    assert!((!(false > true)));
-    assert!((!(false >= true)));
+    assert!(false < true);
+    assert!(false <= true);
+    assert!(!(false > true));
+    assert!(!(false >= true));
 
     // Bools support bitwise binops
     assert_eq!(false & false, false);
@@ -76,9 +76,9 @@ fn test_class() {
   }
   assert_eq!(q, r);
   r.y = 17;
-  assert!((r.y != q.y));
+  assert!(r.y != q.y);
   assert_eq!(r.y, 17);
-  assert!((q != r));
+  assert!(q != r);
 }
 
 pub fn main() {

tests/ui/binop/structured-compare.rs

@@ -17,14 +17,14 @@ pub fn main() {
     let a = (1, 2, 3);
     let b = (1, 2, 3);
     assert_eq!(a, b);
-    assert!((a != (1, 2, 4)));
-    assert!((a < (1, 2, 4)));
-    assert!((a <= (1, 2, 4)));
-    assert!(((1, 2, 4) > a));
-    assert!(((1, 2, 4) >= a));
+    assert!(a != (1, 2, 4));
+    assert!(a < (1, 2, 4));
+    assert!(a <= (1, 2, 4));
+    assert!((1, 2, 4) > a);
+    assert!((1, 2, 4) >= a);
     let x = foo::large;
     let y = foo::small;
-    assert!((x != y));
+    assert!(x != y);
     assert_eq!(x, foo::large);
-    assert!((x != foo::small));
+    assert!(x != foo::small);
 }

tests/ui/cfg/conditional-compile.rs

@@ -85,7 +85,7 @@ pub fn main() {
 pub fn main() {
     // Exercise some of the configured items in ways that wouldn't be possible
     // if they had the bogus definition
-    assert!((b));
+    assert!(b);
     let _x: t = true;
     let _y: tg = tg::bar;
 

tests/ui/codemap_tests/issue-28308.rs

@@ -1,4 +1,4 @@
 fn main() {
     assert!("foo");
-    //~^ ERROR cannot apply unary operator `!`
+    //~^ ERROR mismatched types
 }

tests/ui/codemap_tests/issue-28308.stderr

@@ -1,11 +1,12 @@
-error[E0600]: cannot apply unary operator `!` to type `&'static str`
-  --> $DIR/issue-28308.rs:2:5
+error[E0308]: mismatched types
+  --> $DIR/issue-28308.rs:2:13
    |
 LL |     assert!("foo");
-   |     ^^^^^^^^^^^^^^ cannot apply unary operator `!`
-   |
-   = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)
+   |             ^^^^^
+   |             |
+   |             expected `bool`, found `&str`
+   |             expected due to this
 
 error: aborting due to 1 previous error
 
-For more information about this error, try `rustc --explain E0600`.
+For more information about this error, try `rustc --explain E0308`.