Auto merge of #122661 - estebank:assert-macro-span, r=<try>

Change the desugaring of `assert!` for better error output

try-job: i686-gnu-nopt-1

Author: rust-bors[bot]

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_trait_selection/src/error_reporting/infer/mod.rs

@@ -1619,8 +1619,18 @@ 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 ObligationCauseCode::Pattern { span, .. } = cause.code()
+                && let Some(span) = span
+                && !span.from_expansion()
+                && cause.span.from_expansion()
+            {
+                // When the type error comes from a macro like `assert!()`, and we are pointing at
+                // code the user wrote the cause and effect are reversed as the expected value is
+                // what the macro expanded to.
+                (found, expected) = (expected, found);
+            }
             if expected == found {
                 label_or_note(span, terr.to_string(self.tcx));
             } else {
@@ -2143,7 +2153,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),
@@ -2182,15 +2194,35 @@ 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 ObligationCauseCode::Pattern { span, .. } = cause.code()
+            && let Some(span) = span
+            && !span.from_expansion()
+            && cause.span.from_expansion()
+        {
+            // When the type error comes from a macro like `assert!()`, and we are pointing at
+            // code the user wrote, the cause and effect are reversed as the expected value is
+            // what the macro expanded to. So if the user provided a `Type` when the macro is
+            // written in such a way that a `bool` was expected, we want to print:
+            // = note: expected `bool`
+            //            found `Type`"
+            // but as far as the compiler is concerned, after expansion what was expected was `Type`
+            // = note: expected `Type`
+            //            found `bool`"
+            // so we reverse them here to match user expectation.
+            (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

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/const_is_empty.rs

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

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

@@ -158,10 +158,16 @@ LL |     let _ = val.is_empty();
    |             ^^^^^^^^^^^^^^
 
 error: this expression always evaluates to true
-  --> tests/ui/const_is_empty.rs:202:9
+  --> tests/ui/const_is_empty.rs:198:17
+   |
+LL |         assert!(EMPTY_STR.is_empty());
+   |                 ^^^^^^^^^^^^^^^^^^^^
+
+error: this expression always evaluates to true
+  --> tests/ui/const_is_empty.rs:203:9
    |
 LL |         EMPTY_STR.is_empty();
    |         ^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 27 previous errors
+error: aborting due to 28 previous errors
 

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

@@ -1,6 +1,6 @@
 #![warn(clippy::incompatible_msrv)]
 #![feature(custom_inner_attributes)]
-#![allow(stable_features)]
+#![allow(stable_features, clippy::diverging_sub_expression)]
 #![feature(strict_provenance)] // For use in test
 #![clippy::msrv = "1.3.0"]
 

tests/ui/codemap_tests/issue-28308.rs

@@ -1,4 +1,16 @@
 fn main() {
-    assert!("foo");
-    //~^ ERROR cannot apply unary operator `!`
+    assert!("foo"); //~ ERROR mismatched types
+    //~^ NOTE expected `bool`, found `str`
+    //~| NOTE in this expansion of assert!
+    let x = Some(&1);
+    assert!(x); //~ ERROR mismatched types
+    //~^ NOTE expected `bool`, found `Option<&{integer}>`
+    //~| NOTE expected enum `bool`
+    //~| NOTE in this expansion of assert!
+    //~| NOTE in this expansion of assert!
+    assert!(x, ""); //~ ERROR mismatched types
+    //~^ NOTE expected `bool`, found `Option<&{integer}>`
+    //~| NOTE expected enum `bool`
+    //~| NOTE in this expansion of assert!
+    //~| NOTE in this expansion of assert!
 }

tests/ui/codemap_tests/issue-28308.stderr

@@ -1,9 +1,27 @@
-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 `!`
+   |             ^^^^^ expected `bool`, found `str`
 
-error: aborting due to 1 previous error
+error[E0308]: mismatched types
+  --> $DIR/issue-28308.rs:6:13
+   |
+LL |     assert!(x);
+   |             ^ expected `bool`, found `Option<&{integer}>`
+   |
+   = note: expected enum `bool`
+              found type `Option<&{integer}>`
+
+error[E0308]: mismatched types
+  --> $DIR/issue-28308.rs:11:13
+   |
+LL |     assert!(x, "");
+   |             ^ expected `bool`, found `Option<&{integer}>`
+   |
+   = note: expected enum `bool`
+              found type `Option<&{integer}>`
+
+error: aborting due to 3 previous errors
 
-For more information about this error, try `rustc --explain E0600`.
+For more information about this error, try `rustc --explain E0308`.

tests/ui/consts/control-flow/assert.stderr

@@ -1,8 +1,8 @@
 error[E0080]: evaluation panicked: assertion failed: false
-  --> $DIR/assert.rs:5:15
+  --> $DIR/assert.rs:5:23
    |
 LL | const _: () = assert!(false);
-   |               ^^^^^^^^^^^^^^ evaluation of `_` failed here
+   |                       ^^^^^ evaluation of `_` failed here
 
 error: aborting due to 1 previous error
 

tests/ui/generics/post_monomorphization_error_backtrace.stderr

@@ -1,8 +1,8 @@
 error[E0080]: evaluation panicked: assertion failed: std::mem::size_of::<T>() == 0
-  --> $DIR/post_monomorphization_error_backtrace.rs:6:23
+  --> $DIR/post_monomorphization_error_backtrace.rs:6:31
    |
 LL |         const V: () = assert!(std::mem::size_of::<T>() == 0);
-   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `assert_zst::F::<u32>::V` failed here
+   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `assert_zst::F::<u32>::V` failed here
 
 note: erroneous constant encountered
   --> $DIR/post_monomorphization_error_backtrace.rs:14:5
@@ -17,10 +17,10 @@ LL |     assert_zst::<U>()
    |     ^^^^^^^^^^^^^^^^^
 
 error[E0080]: evaluation panicked: assertion failed: std::mem::size_of::<T>() == 0
-  --> $DIR/post_monomorphization_error_backtrace.rs:6:23
+  --> $DIR/post_monomorphization_error_backtrace.rs:6:31
    |
 LL |         const V: () = assert!(std::mem::size_of::<T>() == 0);
-   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `assert_zst::F::<i32>::V` failed here
+   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `assert_zst::F::<i32>::V` failed here
 
 note: erroneous constant encountered
   --> $DIR/post_monomorphization_error_backtrace.rs:14:5