itsscb/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-10-02 10:18:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Change the desugaring of assert! for better error output

Browse Source 
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.

address review comments
This commit is contained in:
Esteban Küber 2024-03-17 20:46:36 +00:00
parent d9dba3a554
commit c439a59dbd
14 changed files with 134 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
compiler/rustc_builtin_macros/src/assert.rs
View File

@ -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> {

40
compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs
View File

@ -1620,8 +1620,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 {
@ -2144,7 +2154,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,6 +2195,7 @@ 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)> {
@ -2190,8 +2203,27 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
if exp_found.references_error() {
return None;
}
let (mut expected, mut found) = (exp_found.expected, exp_found.found);
Some(match (exp_found.expected.kind(), exp_found.found.kind()) {
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 (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

12
src/tools/clippy/clippy_lints/src/missing_asserts_for_indexing.rs
View File

@ -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| {

1
src/tools/clippy/tests/ui/const_is_empty.rs
View File

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

10
src/tools/clippy/tests/ui/const_is_empty.stderr
View File

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

2
src/tools/clippy/tests/ui/incompatible_msrv.rs
View File

@ -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"]

16
tests/ui/codemap_tests/issue-28308.rs
View File

@ -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!
}

28
tests/ui/codemap_tests/issue-28308.stderr
View File

@ -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}>`
For more information about this error, try `rustc --explain E0600`.
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 E0308`.

4
tests/ui/consts/control-flow/assert.stderr
View File

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

8
tests/ui/generics/post_monomorphization_error_backtrace.stderr
View File

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

4
tests/ui/inline-const/const-expr-generic-err.stderr
View File

@ -1,8 +1,8 @@
error[E0080]: evaluation panicked: assertion failed: std::mem::size_of::<T>() == 0
--> $DIR/const-expr-generic-err.rs:4:13
--> $DIR/const-expr-generic-err.rs:4:21
|
LL | const { assert!(std::mem::size_of::<T>() == 0); }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `foo::<i32>::{constant#0}` failed here
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `foo::<i32>::{constant#0}` failed here
note: erroneous constant encountered
--> $DIR/const-expr-generic-err.rs:4:5

8
tests/ui/macros/assert-macro-lifetimes.rs Normal file
View File

@ -0,0 +1,8 @@
//@ check-pass
#[derive(PartialEq, Eq, Hash)]
struct S;
fn main() {
let foo = std::rc::Rc::new(std::cell::RefCell::new(std::collections::HashMap::<S, S>::new()));
// Ensure that the lifetimes of the borrow do not leak past the end of `main`.
assert!(matches!(foo.borrow().get(&S).unwrap(), S))
}

4
tests/ui/simd/const-err-trumps-simd-err.stderr
View File

@ -1,8 +1,8 @@
error[E0080]: evaluation panicked: assertion failed: LANE < 4
--> $DIR/const-err-trumps-simd-err.rs:17:13
--> $DIR/const-err-trumps-simd-err.rs:17:21
|
LL | const { assert!(LANE < 4); } // the error should be here...
| ^^^^^^^^^^^^^^^^^ evaluation of `get_elem::<4>::{constant#0}` failed here
| ^^^^^^^^ evaluation of `get_elem::<4>::{constant#0}` failed here
note: erroneous constant encountered
--> $DIR/const-err-trumps-simd-err.rs:17:5

12
tests/ui/transmutability/uninhabited.stderr
View File

@ -41,10 +41,10 @@ LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`
error[E0080]: evaluation panicked: assertion failed: false
--> $DIR/uninhabited.rs:41:9
--> $DIR/uninhabited.rs:41:17
|
LL | assert!(false);
| ^^^^^^^^^^^^^^ evaluation of `yawning_void_struct::_` failed here
| ^^^^^ evaluation of `yawning_void_struct::_` failed here
error[E0277]: `()` cannot be safely transmuted into `yawning_void_enum::Void`
--> $DIR/uninhabited.rs:71:41
@ -68,10 +68,10 @@ LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`
error[E0080]: evaluation panicked: assertion failed: false
--> $DIR/uninhabited.rs:63:9
--> $DIR/uninhabited.rs:63:17
|
LL | assert!(false);
| ^^^^^^^^^^^^^^ evaluation of `yawning_void_enum::_` failed here
| ^^^^^ evaluation of `yawning_void_enum::_` failed here
error[E0277]: `u128` cannot be safely transmuted into `DistantVoid`
--> $DIR/uninhabited.rs:92:43
@ -95,10 +95,10 @@ LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`
error[E0080]: evaluation panicked: assertion failed: false
--> $DIR/uninhabited.rs:87:9
--> $DIR/uninhabited.rs:87:17
|
LL | assert!(false);
| ^^^^^^^^^^^^^^ evaluation of `distant_void::_` failed here
| ^^^^^ evaluation of `distant_void::_` failed here
error[E0277]: `Src` cannot be safely transmuted into `issue_126267::Error`
--> $DIR/uninhabited.rs:108:42