From 89682a53131803c81c8d6c5f013d1bbe410c8ae1 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Wed, 11 Sep 2024 04:07:13 +0800 Subject: [PATCH] downgrade borrowck suggestion level due to possible span conflict --- .../src/diagnostics/explain_borrow.rs | 43 +++++++---- .../if-let-rescope-borrowck-suggestions.fixed | 26 ------- .../if-let-rescope-borrowck-suggestions.rs | 10 ++- ...if-let-rescope-borrowck-suggestions.stderr | 77 +++++++++++++++++-- 4 files changed, 108 insertions(+), 48 deletions(-) delete mode 100644 tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index 22327d2ba0d..d9b5fd8b5c1 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -128,7 +128,7 @@ pub(crate) fn add_explanation_to_diagnostic( && pat.span.can_be_used_for_suggestions() && let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span) { - suggest_rewrite_if_let(expr, &pat, init, conseq, alt, err); + suggest_rewrite_if_let(tcx, expr, &pat, init, conseq, alt, err); } else if path_span.map_or(true, |path_span| path_span == var_or_use_span) { // We can use `var_or_use_span` if either `path_span` is not present, or both spans are the same if borrow_span.map_or(true, |sp| !sp.overlaps(var_or_use_span)) { @@ -292,7 +292,7 @@ pub(crate) fn add_explanation_to_diagnostic( && pat.span.can_be_used_for_suggestions() && let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span) { - suggest_rewrite_if_let(expr, &pat, init, conseq, alt, err); + suggest_rewrite_if_let(tcx, expr, &pat, init, conseq, alt, err); } } } @@ -428,34 +428,49 @@ fn add_lifetime_bound_suggestion_to_diagnostic( } } -fn suggest_rewrite_if_let<'tcx>( - expr: &hir::Expr<'tcx>, +fn suggest_rewrite_if_let( + tcx: TyCtxt<'_>, + expr: &hir::Expr<'_>, pat: &str, - init: &hir::Expr<'tcx>, - conseq: &hir::Expr<'tcx>, - alt: Option<&hir::Expr<'tcx>>, + init: &hir::Expr<'_>, + conseq: &hir::Expr<'_>, + alt: Option<&hir::Expr<'_>>, err: &mut Diag<'_>, ) { + let source_map = tcx.sess.source_map(); err.span_note( - conseq.span.shrink_to_hi(), - "lifetime for temporaries generated in `if let`s have been shorted in Edition 2024", + source_map.end_point(conseq.span), + "lifetimes for temporaries generated in `if let`s have been shortened in Edition 2024 so that they are dropped here instead", ); if expr.span.can_be_used_for_suggestions() && conseq.span.can_be_used_for_suggestions() { + let needs_block = if let Some(hir::Node::Expr(expr)) = + alt.and_then(|alt| tcx.hir().parent_iter(alt.hir_id).next()).map(|(_, node)| node) + { + matches!(expr.kind, hir::ExprKind::If(..)) + } else { + false + }; let mut sugg = vec![ - (expr.span.shrink_to_lo().between(init.span), "match ".into()), + ( + expr.span.shrink_to_lo().between(init.span), + if needs_block { "{ match ".into() } else { "match ".into() }, + ), (conseq.span.shrink_to_lo(), format!(" {{ {pat} => ")), ]; let expr_end = expr.span.shrink_to_hi(); + let mut expr_end_code; if let Some(alt) = alt { - sugg.push((conseq.span.between(alt.span), format!(" _ => "))); - sugg.push((expr_end, "}".into())); + sugg.push((conseq.span.between(alt.span), " _ => ".into())); + expr_end_code = "}".to_string(); } else { - sugg.push((expr_end, " _ => {} }".into())); + expr_end_code = " _ => {} }".into(); } + expr_end_code.push('}'); + sugg.push((expr_end, expr_end_code)); err.multipart_suggestion( "consider rewriting the `if` into `match` which preserves the extended lifetime", sugg, - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); } } diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed b/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed deleted file mode 100644 index 129e78ea9fe..00000000000 --- a/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed +++ /dev/null @@ -1,26 +0,0 @@ -//@ edition: 2024 -//@ compile-flags: -Z validate-mir -Zunstable-options -//@ run-rustfix - -#![feature(if_let_rescope)] -#![deny(if_let_rescope)] - -struct Droppy; -impl Drop for Droppy { - fn drop(&mut self) { - println!("dropped"); - } -} -impl Droppy { - fn get_ref(&self) -> Option<&u8> { - None - } -} - -fn do_something(_: &T) {} - -fn main() { - let binding = Droppy; - do_something(match binding.get_ref() { Some(value) => { value } _ => { &0 }}); - //~^ ERROR: temporary value dropped while borrowed -} diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs b/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs index 1970d5c0f7e..2476f7cf258 100644 --- a/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs @@ -1,6 +1,5 @@ //@ edition: 2024 //@ compile-flags: -Z validate-mir -Zunstable-options -//@ run-rustfix #![feature(if_let_rescope)] #![deny(if_let_rescope)] @@ -22,4 +21,13 @@ fn do_something(_: &T) {} fn main() { do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); //~^ ERROR: temporary value dropped while borrowed + do_something(if let Some(value) = Droppy.get_ref() { + //~^ ERROR: temporary value dropped while borrowed + value + } else if let Some(value) = Droppy.get_ref() { + //~^ ERROR: temporary value dropped while borrowed + value + } else { + &0 + }); } diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr b/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr index fa154f01a12..0c6f1ea28d2 100644 --- a/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr @@ -1,16 +1,16 @@ error[E0716]: temporary value dropped while borrowed - --> $DIR/if-let-rescope-borrowck-suggestions.rs:23:39 + --> $DIR/if-let-rescope-borrowck-suggestions.rs:22:39 | LL | do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); | ^^^^^^ - temporary value is freed at the end of this statement | | | creates a temporary value which is freed while still in use | -note: lifetime for temporaries generated in `if let`s have been shorted in Edition 2024 - --> $DIR/if-let-rescope-borrowck-suggestions.rs:23:65 +note: lifetimes for temporaries generated in `if let`s have been shortened in Edition 2024 so that they are dropped here instead + --> $DIR/if-let-rescope-borrowck-suggestions.rs:22:64 | LL | do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); - | ^ + | ^ help: consider using a `let` binding to create a longer lived value | LL ~ let binding = Droppy; @@ -18,9 +18,72 @@ LL ~ do_something(if let Some(value) = binding.get_ref() { value } else { &0 | help: consider rewriting the `if` into `match` which preserves the extended lifetime | -LL | do_something(match Droppy.get_ref() { Some(value) => { value } _ => { &0 }}); - | ~~~~~ ++++++++++++++++ ~~~~ + +LL | do_something({ match Droppy.get_ref() { Some(value) => { value } _ => { &0 }}}); + | ~~~~~~~ ++++++++++++++++ ~~~~ ++ -error: aborting due to 1 previous error +error[E0716]: temporary value dropped while borrowed + --> $DIR/if-let-rescope-borrowck-suggestions.rs:24:39 + | +LL | do_something(if let Some(value) = Droppy.get_ref() { + | ^^^^^^ creates a temporary value which is freed while still in use +... +LL | } else if let Some(value) = Droppy.get_ref() { + | - temporary value is freed at the end of this statement + | +note: lifetimes for temporaries generated in `if let`s have been shortened in Edition 2024 so that they are dropped here instead + --> $DIR/if-let-rescope-borrowck-suggestions.rs:27:5 + | +LL | } else if let Some(value) = Droppy.get_ref() { + | ^ +help: consider using a `let` binding to create a longer lived value + | +LL ~ let binding = Droppy; +LL ~ do_something(if let Some(value) = binding.get_ref() { + | +help: consider rewriting the `if` into `match` which preserves the extended lifetime + | +LL ~ do_something({ match Droppy.get_ref() { Some(value) => { +LL | +LL | value +LL ~ } _ => if let Some(value) = Droppy.get_ref() { +LL | +... +LL | &0 +LL ~ }}}); + | + +error[E0716]: temporary value dropped while borrowed + --> $DIR/if-let-rescope-borrowck-suggestions.rs:27:33 + | +LL | } else if let Some(value) = Droppy.get_ref() { + | ^^^^^^ creates a temporary value which is freed while still in use +... +LL | } else { + | - temporary value is freed at the end of this statement + | +note: lifetimes for temporaries generated in `if let`s have been shortened in Edition 2024 so that they are dropped here instead + --> $DIR/if-let-rescope-borrowck-suggestions.rs:30:5 + | +LL | } else { + | ^ +help: consider using a `let` binding to create a longer lived value + | +LL ~ let binding = Droppy; +LL ~ do_something(if let Some(value) = Droppy.get_ref() { +LL | +LL | value +LL ~ } else if let Some(value) = binding.get_ref() { + | +help: consider rewriting the `if` into `match` which preserves the extended lifetime + | +LL ~ } else { match Droppy.get_ref() { Some(value) => { +LL | +LL | value +LL ~ } _ => { +LL | &0 +LL ~ }}}); + | + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0716`.