about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEsteban Küber <esteban@kuber.com.ar>2022-07-14 18:41:12 -0700
committerEsteban Küber <esteban@kuber.com.ar>2022-07-15 11:04:45 -0700
commit635c38187bbd7adc10abf417c68b1ff06753ff5f (patch)
treeb771d8cac34741ac5c2afeeb5744151aee80af15
parent20b5aaf11103d65752adafe549be1acecfa9acdd (diff)
downloadrust-635c38187bbd7adc10abf417c68b1ff06753ff5f.tar.gz
rust-635c38187bbd7adc10abf417c68b1ff06753ff5f.zip
Avoid incorrect suggestion
We check that there's a single level of block nesting to ensure always
correct suggestions. If we don't, then we only provide a free-form
message to avoid misleading users in cases like
`src/test/ui/nll/borrowed-temporary-error.rs`.

We could expand the analysis to suggest hoising all of the relevant
parts of the users' code to make the code compile, but that could be
too much.
-rw-r--r--compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs40
-rw-r--r--src/test/ui/nll/borrowed-temporary-error.stderr8
-rw-r--r--src/test/ui/span/borrowck-let-suggestion-suffixes.rs3
-rw-r--r--src/test/ui/span/borrowck-let-suggestion-suffixes.stderr12
4 files changed, 42 insertions, 21 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
index 6923fc1c1f6..c4eed784b88 100644
--- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -1503,15 +1503,49 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 let sm = self.infcx.tcx.sess.source_map();
                 let mut suggested = false;
                 let msg = "consider using a `let` binding to create a longer lived value";
-                if let Some(scope) =
-                    self.body.source_scopes.get(self.body.source_info(location).scope)
+
+                use rustc_hir::intravisit::Visitor;
+
+                /// We check that there's a single level of block nesting to ensure always correct
+                /// suggestions. If we don't, then we only provide a free-form message to avoid
+                /// misleading users in cases like `src/test/ui/nll/borrowed-temporary-error.rs`.
+                /// We could expand the analysis to suggest hoising all of the relevant parts of
+                /// the users' code to make the code compile, but that could be too much.
+                struct NestedStatementVisitor {
+                    span: Span,
+                    current: usize,
+                    found: usize,
+                }
+
+                impl<'tcx> Visitor<'tcx> for NestedStatementVisitor {
+                    fn visit_block(&mut self, block: &hir::Block<'tcx>) {
+                        self.current += 1;
+                        rustc_hir::intravisit::walk_block(self, block);
+                        self.current -= 1;
+                    }
+                    fn visit_expr(&mut self, expr: &hir::Expr<'tcx>) {
+                        if self.span == expr.span {
+                            self.found = self.current;
+                        }
+                        rustc_hir::intravisit::walk_expr(self, expr);
+                    }
+                }
+                let source_info = self.body.source_info(location);
+                if let Some(scope) = self.body.source_scopes.get(source_info.scope)
                     && let ClearCrossCrate::Set(scope_data) = &scope.local_data
                     && let Some(node) = self.infcx.tcx.hir().find(scope_data.lint_root)
                     && let Some(id) = node.body_id()
                     && let hir::ExprKind::Block(block, _) = self.infcx.tcx.hir().body(id).value.kind
                 {
                     for stmt in block.stmts {
-                        if stmt.span.contains(proper_span)
+                        let mut visitor = NestedStatementVisitor {
+                            span: proper_span,
+                            current: 0,
+                            found: 0,
+                        };
+                        visitor.visit_stmt(stmt);
+                        if visitor.found == 0
+                            && stmt.span.contains(proper_span)
                             && let Some(p) = sm.span_to_margin(stmt.span)
                             && let Ok(s) = sm.span_to_snippet(proper_span)
                         {
diff --git a/src/test/ui/nll/borrowed-temporary-error.stderr b/src/test/ui/nll/borrowed-temporary-error.stderr
index 59e6dd61b0f..2c6bd92641f 100644
--- a/src/test/ui/nll/borrowed-temporary-error.stderr
+++ b/src/test/ui/nll/borrowed-temporary-error.stderr
@@ -9,13 +9,7 @@ LL |     });
 LL |     println!("{:?}", x);
    |                      - borrow later used here
    |
-help: consider using a `let` binding to create a longer lived value
-   |
-LL ~     let binding = (v,);
-LL ~     let x = gimme({
-LL |         let v = 22;
-LL ~         &binding
-   |
+   = note: consider using a `let` binding to create a longer lived value
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/span/borrowck-let-suggestion-suffixes.rs b/src/test/ui/span/borrowck-let-suggestion-suffixes.rs
index 70e136e7e61..6240f103c99 100644
--- a/src/test/ui/span/borrowck-let-suggestion-suffixes.rs
+++ b/src/test/ui/span/borrowck-let-suggestion-suffixes.rs
@@ -22,7 +22,7 @@ fn f() {
     //~| NOTE temporary value is freed at the end of this statement
     //~| HELP consider using a `let` binding to create a longer lived value
 
-    { //~ HELP consider using a `let` binding to create a longer lived value
+    {
 
         let mut v4 = Vec::new(); // (sub) statement 0
 
@@ -30,6 +30,7 @@ fn f() {
         //~^ ERROR temporary value dropped while borrowed
         //~| NOTE creates a temporary which is freed while still in use
         //~| NOTE temporary value is freed at the end of this statement
+        //~| NOTE consider using a `let` binding to create a longer lived value
         v4.use_ref();
         //~^ NOTE borrow later used here
     }                       // (statement 7)
diff --git a/src/test/ui/span/borrowck-let-suggestion-suffixes.stderr b/src/test/ui/span/borrowck-let-suggestion-suffixes.stderr
index ab68d111597..a236dab3ae5 100644
--- a/src/test/ui/span/borrowck-let-suggestion-suffixes.stderr
+++ b/src/test/ui/span/borrowck-let-suggestion-suffixes.stderr
@@ -38,18 +38,10 @@ LL |         v4.push(&id('y'));
 LL |         v4.use_ref();
    |         ------------ borrow later used here
    |
-help: consider using a `let` binding to create a longer lived value
-   |
-LL ~     let binding = id('y');
-LL ~     {
-LL | 
-LL |         let mut v4 = Vec::new(); // (sub) statement 0
-LL | 
-LL ~         v4.push(&binding);
-   |
+   = note: consider using a `let` binding to create a longer lived value
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/borrowck-let-suggestion-suffixes.rs:39:14
+  --> $DIR/borrowck-let-suggestion-suffixes.rs:40:14
    |
 LL |     v5.push(&id('z'));
    |              ^^^^^^^ - temporary value is freed at the end of this statement