Skip to content

Commit

Permalink
downgrade borrowck suggestion level due to possible span conflict
Browse files Browse the repository at this point in the history
  • Loading branch information
@dingxiangfei2009
dingxiangfei2009 committed Sep 10, 2024
1 parent e2120a7 commit 89682a5
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 48 deletions.
43 changes: 29 additions & 14 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
&& 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)) {
Expand Down Expand Up @@ -292,7 +292,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
&& 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);
}
}
}
Expand Down Expand Up @@ -428,34 +428,49 @@ impl<'tcx> BorrowExplanation<'tcx> {
}
}

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,
);
}
}
Expand Down
26 changes: 0 additions & 26 deletions tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed
View file

This file was deleted.

10 changes: 9 additions & 1 deletion tests/ui/drop/if-let-rescope-borrowck-suggestions.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//@ edition: 2024
//@ compile-flags: -Z validate-mir -Zunstable-options
//@ run-rustfix

#![feature(if_let_rescope)]
#![deny(if_let_rescope)]
Expand All @@ -22,4 +21,13 @@ fn do_something<T>(_: &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
});
}
77 changes: 70 additions & 7 deletions tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,26 +1,89 @@
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;
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`.

0 comments on commit 89682a5

Please sign in to comment.