Change the desugaring of assert! for better error output

In the desugaring of `assert!`, we now expand to a `match` expression
instead of `if !cond {..}`.

The span of incorrect conditions will point only at the expression, and not
the whole `assert!` invocation.

```
error[E0308]: mismatched types
  --> $DIR/issue-14091.rs:2:13
   |
LL |     assert!(1,1);
   |             ^ expected `bool`, found integer
```

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`
```

`assert!(val)` now desugars to:

```rust
match val {
    true => {},
    _ => $crate::panic::panic_2021!(),
}
```

Fix #122159.

We make some minor changes to some diagnostics to avoid span overlap on
type mismatch or inverted "expected"/"found" on type errors.

We remove some unnecessary parens from core, alloc and miri.

Author: estebank

compiler/rustc_builtin_macros/src/assert.rs

@@ -1,8 +1,8 @@
 mod context;
 
-use rustc_ast::token::Delimiter;
+use rustc_ast::token::{self, Delimiter};
 use rustc_ast::tokenstream::{DelimSpan, TokenStream};
-use rustc_ast::{DelimArgs, Expr, ExprKind, MacCall, Path, PathSegment, UnOp, token};
+use rustc_ast::{DelimArgs, Expr, ExprKind, MacCall, Path, PathSegment};
 use rustc_ast_pretty::pprust;
 use rustc_errors::PResult;
 use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacEager, MacroExpanderResult};
@@ -29,7 +29,7 @@ pub(crate) fn expand_assert<'cx>(
 
     // `core::panic` and `std::panic` are different macros, so we use call-site
     // context to pick up whichever is currently in scope.
-    let call_site_span = cx.with_call_site_ctxt(span);
+    let call_site_span = cx.with_call_site_ctxt(cond_expr.span);
 
     let panic_path = || {
         if use_panic_2021(span) {
@@ -63,7 +63,7 @@ pub(crate) fn expand_assert<'cx>(
                 }),
             })),
         );
-        expr_if_not(cx, call_site_span, cond_expr, then, None)
+        assert_cond_check(cx, call_site_span, cond_expr, then)
     }
     // If `generic_assert` is enabled, generates rich captured outputs
     //
@@ -88,26 +88,33 @@ pub(crate) fn expand_assert<'cx>(
                 )),
             )],
         );
-        expr_if_not(cx, call_site_span, cond_expr, then, None)
+        assert_cond_check(cx, call_site_span, cond_expr, then)
     };
 
     ExpandResult::Ready(MacEager::expr(expr))
 }
 
+/// `assert!($cond_expr, $custom_message)`
 struct Assert {
     cond_expr: Box<Expr>,
     custom_message: Option<TokenStream>,
 }
 
-// if !{ ... } { ... } else { ... }
-fn expr_if_not(
-    cx: &ExtCtxt<'_>,
-    span: Span,
-    cond: Box<Expr>,
-    then: Box<Expr>,
-    els: Option<Box<Expr>>,
-) -> Box<Expr> {
-    cx.expr_if(span, cx.expr(span, ExprKind::Unary(UnOp::Not, cond)), then, els)
+/// `match <cond> { true => {} _ => <then> }`
+fn assert_cond_check(cx: &ExtCtxt<'_>, span: Span, cond: Box<Expr>, then: Box<Expr>) -> Box<Expr> {
+    // Instead of expanding to `if !<cond> { <then> }`, we expand to
+    // `match <cond> { true => {} _ => <then> }`.
+    // This allows us to always complain about mismatched types instead of "cannot apply unary
+    // operator `!` to type `X`" when passing an invalid `<cond>`, while also allowing `<cond>` to
+    // be `&true`.
+    let els = cx.expr_block(cx.block(span, thin_vec![]));
+    let mut arms = thin_vec![];
+    arms.push(cx.arm(span, cx.pat_lit(span, cx.expr_bool(span, true)), els));
+    arms.push(cx.arm(span, cx.pat_wild(span), then));
+
+    // We wrap the `match` in a statement to limit the length of any borrows introduced in the
+    // condition.
+    cx.expr_block(cx.block(span, [cx.stmt_expr(cx.expr_match(span, cond, arms))].into()))
 }
 
 fn parse_assert<'a>(cx: &ExtCtxt<'a>, sp: Span, stream: TokenStream) -> PResult<'a, Assert> {

compiler/rustc_hir_typeck/src/demand.rs

@@ -698,7 +698,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     ) {
         match (self.tcx.parent_hir_node(expr.hir_id), error) {
             (hir::Node::LetStmt(hir::LetStmt { ty: Some(ty), init: Some(init), .. }), _)
-                if init.hir_id == expr.hir_id =>
+                if init.hir_id == expr.hir_id && !ty.span.source_equal(init.span) =>
             {
                 // Point at `let` assignment type.
                 err.span_label(ty.span, "expected due to this");

compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs

@@ -459,7 +459,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                             span,
                             format!("this is an iterator with items of type `{}`", args.type_at(0)),
                         );
-                    } else {
+                    } else if !span.overlaps(cause.span) {
                         let expected_ty = self.tcx.short_string(expected_ty, err.long_ty_path());
                         err.span_label(span, format!("this expression has type `{expected_ty}`"));
                     }
@@ -1620,8 +1620,16 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
         {
             let e = self.tcx.erase_regions(e);
             let f = self.tcx.erase_regions(f);
-            let expected = with_forced_trimmed_paths!(e.sort_string(self.tcx));
-            let found = with_forced_trimmed_paths!(f.sort_string(self.tcx));
+            let mut expected = with_forced_trimmed_paths!(e.sort_string(self.tcx));
+            let mut found = with_forced_trimmed_paths!(f.sort_string(self.tcx));
+            if let Some(def_id) = cause.span.ctxt().outer_expn_data().macro_def_id
+                && self.tcx.is_diagnostic_item(sym::assert_macro, def_id)
+            {
+                // When the type error comes from `assert!()`, the cause and effect are reversed
+                // because that macro expands to `match val { false => {panic!()}, _ => {} }`, which
+                // would say something like "expected `Type`, found `bool`", confusing the user.
+                (found, expected) = (expected, found);
+            }
             if expected == found {
                 label_or_note(span, terr.to_string(self.tcx));
             } else {
@@ -2144,7 +2152,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
     ) -> Option<(DiagStyledString, DiagStyledString)> {
         match values {
             ValuePairs::Regions(exp_found) => self.expected_found_str(exp_found),
-            ValuePairs::Terms(exp_found) => self.expected_found_str_term(exp_found, long_ty_path),
+            ValuePairs::Terms(exp_found) => {
+                self.expected_found_str_term(cause, exp_found, long_ty_path)
+            }
             ValuePairs::Aliases(exp_found) => self.expected_found_str(exp_found),
             ValuePairs::ExistentialTraitRef(exp_found) => self.expected_found_str(exp_found),
             ValuePairs::ExistentialProjection(exp_found) => self.expected_found_str(exp_found),
@@ -2183,15 +2193,27 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
 
     fn expected_found_str_term(
         &self,
+        cause: &ObligationCause<'tcx>,
         exp_found: ty::error::ExpectedFound<ty::Term<'tcx>>,
         long_ty_path: &mut Option<PathBuf>,
     ) -> Option<(DiagStyledString, DiagStyledString)> {
         let exp_found = self.resolve_vars_if_possible(exp_found);
         if exp_found.references_error() {
             return None;
         }
+        let (mut expected, mut found) = (exp_found.expected, exp_found.found);
+        if let Some(def_id) = cause.span.ctxt().outer_expn_data().macro_def_id
+            && self.tcx.is_diagnostic_item(sym::assert_macro, def_id)
+        {
+            // When the type error comes from `assert!()`, the cause and effect are reversed
+            // because that macro expands to `match val { false => {panic!()}, _ => {} }`, which
+            // would say something like
+            // = note: expected `Type`
+            //            found `bool`"
+            (expected, found) = (found, expected);
+        }
 
-        Some(match (exp_found.expected.kind(), exp_found.found.kind()) {
+        Some(match (expected.kind(), found.kind()) {
             (ty::TermKind::Ty(expected), ty::TermKind::Ty(found)) => {
                 let (mut exp, mut fnd) = self.cmp(expected, found);
                 // Use the terminal width as the basis to determine when to compress the printed

library/coretests/tests/tuple.rs

@@ -37,7 +37,7 @@ fn test_partial_ord() {
     assert!(!((1.0f64, 2.0f64) <= (f64::NAN, 3.0)));
     assert!(!((1.0f64, 2.0f64) > (f64::NAN, 3.0)));
     assert!(!((1.0f64, 2.0f64) >= (f64::NAN, 3.0)));
-    assert!(((1.0f64, 2.0f64) < (2.0, f64::NAN)));
+    assert!((1.0f64, 2.0f64) < (2.0, f64::NAN));
     assert!(!((2.0f64, 2.0f64) < (2.0, f64::NAN)));
 }
 

library/std/tests/env.rs

@@ -16,7 +16,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/clippy/clippy_lints/src/bool_assert_comparison.rs

@@ -130,13 +130,27 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
 
                 let mut suggestions = vec![(name_span, non_eq_mac.to_string()), (lit_span, String::new())];
 
-                if bool_value ^ eq_macro {
-                    let Some(sugg) = Sugg::hir_opt(cx, non_lit_expr) else {
-                        return;
-                    };
-                    suggestions.push((non_lit_expr.span, (!sugg).to_string()));
+                if let ty::Bool = non_lit_ty.kind() {
+                    if bool_value ^ eq_macro {
+                        let Some(sugg) = Sugg::hir_opt(cx, non_lit_expr) else {
+                            return;
+                        };
+                        suggestions.push((non_lit_expr.span, (!sugg).to_string()));
+                    }
+                } else {
+                    // If we have a `value` that is *not* `bool` but that `!value` *is*, we suggest
+                    // `!!value`.
+                    suggestions.push((
+                        non_lit_expr.span.shrink_to_lo(),
+                        if bool_value ^ eq_macro {
+                            "!".to_string()
+                        } else {
+                            "!!".to_string()
+                        },
+                    ));
                 }
 
+
                 diag.multipart_suggestion(
                     format!("replace it with `{non_eq_mac}!(..)`"),
                     suggestions,

src/tools/clippy/clippy_lints/src/missing_asserts_for_indexing.rs

@@ -11,7 +11,7 @@ use rustc_ast::{BinOpKind, LitKind, RangeLimits};
 use rustc_data_structures::packed::Pu128;
 use rustc_data_structures::unhash::UnindexMap;
 use rustc_errors::{Applicability, Diag};
-use rustc_hir::{Block, Body, Expr, ExprKind, UnOp};
+use rustc_hir::{Body, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::source_map::Spanned;
@@ -135,12 +135,12 @@ fn assert_len_expr<'hir>(
     cx: &LateContext<'_>,
     expr: &'hir Expr<'hir>,
 ) -> Option<(LengthComparison, usize, &'hir Expr<'hir>)> {
-    let (cmp, asserted_len, slice_len) = if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr)
-        && let ExprKind::Unary(UnOp::Not, condition) = &cond.kind
-        && let ExprKind::Binary(bin_op, left, right) = &condition.kind
+    let (cmp, asserted_len, slice_len) = if let Some(
+        higher::IfLetOrMatch::Match(cond, [_, then], _)
+    ) = higher::IfLetOrMatch::parse(cx, expr)
+        && let ExprKind::Binary(bin_op, left, right) = &cond.kind
         // check if `then` block has a never type expression
-        && let ExprKind::Block(Block { expr: Some(then_expr), .. }, _) = then.kind
-        && cx.typeck_results().expr_ty(then_expr).is_never()
+        && cx.typeck_results().expr_ty(then.body).is_never()
     {
         len_comparison(bin_op.node, left, right)?
     } else if let Some((macro_call, bin_op)) = first_node_macro_backtrace(cx, expr).find_map(|macro_call| {

src/tools/clippy/tests/ui/bool_assert_comparison.fixed

@@ -94,7 +94,7 @@ fn main() {
     assert_eq!(a!(), "".is_empty());
     assert_eq!("".is_empty(), b!());
     assert_eq!(a, true);
-    assert!(b);
+    assert!(!!b);
     //~^ bool_assert_comparison
 
     assert_ne!("a".len(), 1);
@@ -122,7 +122,7 @@ fn main() {
     debug_assert_eq!(a!(), "".is_empty());
     debug_assert_eq!("".is_empty(), b!());
     debug_assert_eq!(a, true);
-    debug_assert!(b);
+    debug_assert!(!!b);
     //~^ bool_assert_comparison
 
     debug_assert_ne!("a".len(), 1);
@@ -167,7 +167,7 @@ fn main() {
 
     use debug_assert_eq as renamed;
     renamed!(a, true);
-    debug_assert!(b);
+    debug_assert!(!!b);
     //~^ bool_assert_comparison
 
     let non_copy = NonCopy;

src/tools/clippy/tests/ui/bool_assert_comparison.stderr

@@ -45,7 +45,7 @@ LL |     assert_eq!(b, true);
 help: replace it with `assert!(..)`
    |
 LL -     assert_eq!(b, true);
-LL +     assert!(b);
+LL +     assert!(!!b);
    |
 
 error: used `assert_ne!` with a literal bool
@@ -141,7 +141,7 @@ LL |     debug_assert_eq!(b, true);
 help: replace it with `debug_assert!(..)`
    |
 LL -     debug_assert_eq!(b, true);
-LL +     debug_assert!(b);
+LL +     debug_assert!(!!b);
    |
 
 error: used `debug_assert_ne!` with a literal bool
@@ -297,7 +297,7 @@ LL |     renamed!(b, true);
 help: replace it with `debug_assert!(..)`
    |
 LL -     renamed!(b, true);
-LL +     debug_assert!(b);
+LL +     debug_assert!(!!b);
    |
 
 error: used `assert_eq!` with a literal bool

src/tools/clippy/tests/ui/const_is_empty.rs

@@ -196,6 +196,7 @@ fn issue_13106() {
 
     const {
         assert!(EMPTY_STR.is_empty());
+        //~^ const_is_empty
     }
 
     const {