use maybe_body_owned_by for closure

chenyukang · chenyukang · commit 3ddf6f7c1793 · 2023-07-14T07:12:35.000+08:00
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -363,21 +363,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             }
         }
         let hir = self.infcx.tcx.hir();
-        if let Some(hir::Node::Item(hir::Item {
-            kind: hir::ItemKind::Fn(_, _, body_id),
-            ..
-        })) = hir.find(self.mir_hir_id())
-            && let Some(hir::Node::Expr(expr)) = hir.find(body_id.hir_id)
-        {
+        if let Some(body_id) = hir.maybe_body_owned_by(self.mir_def_id()) {
+            let expr = hir.body(body_id).value;
             let place = &self.move_data.move_paths[mpi].place;
-            let span = place.as_local()
-                .map(|local| self.body.local_decls[local].source_info.span);
-            let mut finder = ExpressionFinder {
-                expr_span: move_span,
-                expr: None,
-                pat: None,
-                parent_pat: None,
-            };
+            let span = place.as_local().map(|local| self.body.local_decls[local].source_info.span);
+            let mut finder =
+                ExpressionFinder { expr_span: move_span, expr: None, pat: None, parent_pat: None };
             finder.visit_expr(expr);
             if let Some(span) = span && let Some(expr) = finder.expr {
                 for (_, expr) in hir.parent_iter(expr.hir_id) {
diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
@@ -2,7 +2,6 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
 use rustc_hir as hir;
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::Node;
-use rustc_middle::hir::map::Map;
 use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_middle::{
@@ -646,14 +645,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
             }
             let hir_map = self.infcx.tcx.hir();
             let def_id = self.body.source.def_id();
-            let hir_id = hir_map.local_def_id_to_hir_id(def_id.as_local().unwrap());
-            let node = hir_map.find(hir_id);
-            let Some(hir::Node::Item(item)) = node else {
-                return;
-            };
-            let hir::ItemKind::Fn(.., body_id) = item.kind else {
-                return;
-            };
+            let Some(local_def_id) = def_id.as_local() else { return };
+            let Some(body_id) = hir_map.maybe_body_owned_by(local_def_id) else { return };
             let body = self.infcx.tcx.hir().body(body_id);
 
             let mut v = V { assign_span: span, err, ty, suggested: false };
@@ -790,23 +783,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
     // In the future, attempt in all path but initially for RHS of for_loop
     fn suggest_similar_mut_method_for_for_loop(&self, err: &mut Diagnostic) {
         use hir::{
-            BodyId, Expr,
+            Expr,
             ExprKind::{Block, Call, DropTemps, Match, MethodCall},
-            HirId, ImplItem, ImplItemKind, Item, ItemKind,
         };
 
-        fn maybe_body_id_of_fn(hir_map: Map<'_>, id: HirId) -> Option<BodyId> {
-            match hir_map.find(id) {
-                Some(Node::Item(Item { kind: ItemKind::Fn(_, _, body_id), .. }))
-                | Some(Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(_, body_id), .. })) => {
-                    Some(*body_id)
-                }
-                _ => None,
-            }
-        }
         let hir_map = self.infcx.tcx.hir();
-        let mir_body_hir_id = self.mir_hir_id();
-        if let Some(fn_body_id) = maybe_body_id_of_fn(hir_map, mir_body_hir_id) {
+        if let Some(body_id) = hir_map.maybe_body_owned_by(self.mir_def_id()) {
             if let Block(
                 hir::Block {
                     expr:
@@ -840,7 +822,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     ..
                 },
                 _,
-            ) = hir_map.body(fn_body_id).value.kind
+            ) = hir_map.body(body_id).value.kind
             {
                 let opt_suggestions = self
                     .infcx
@@ -1102,46 +1084,45 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 }
                 let hir_map = self.infcx.tcx.hir();
                 let def_id = self.body.source.def_id();
-                let hir_id = hir_map.local_def_id_to_hir_id(def_id.expect_local());
-                let node = hir_map.find(hir_id);
-                let hir_id = if let Some(hir::Node::Item(item)) = node
-                && let hir::ItemKind::Fn(.., body_id) = item.kind
-            {
-                let body = hir_map.body(body_id);
-                let mut v = BindingFinder {
-                    span: err_label_span,
-                    hir_id: None,
+                let hir_id = if let Some(local_def_id) = def_id.as_local() &&
+                    let Some(body_id) = hir_map.maybe_body_owned_by(local_def_id)
+                {
+                    let body = hir_map.body(body_id);
+                    let mut v = BindingFinder {
+                        span: err_label_span,
+                        hir_id: None,
+                    };
+                    v.visit_body(body);
+                    v.hir_id
+                } else {
+                    None
                 };
-                v.visit_body(body);
-                v.hir_id
-            } else {
-                None
-            };
+
                 if let Some(hir_id) = hir_id
                 && let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
-            {
-                let (changing, span, sugg) = match local.ty {
-                    Some(ty) => ("changing", ty.span, message),
-                    None => (
-                        "specifying",
-                        local.pat.span.shrink_to_hi(),
-                        format!(": {message}"),
-                    ),
-                };
-                err.span_suggestion_verbose(
-                    span,
-                    format!("consider {changing} this binding's type"),
-                    sugg,
-                    Applicability::HasPlaceholders,
-                );
-            } else {
-                err.span_label(
-                    err_label_span,
-                    format!(
-                        "consider changing this binding's type to be: `{message}`"
-                    ),
-                );
-            }
+                {
+                    let (changing, span, sugg) = match local.ty {
+                        Some(ty) => ("changing", ty.span, message),
+                        None => (
+                            "specifying",
+                            local.pat.span.shrink_to_hi(),
+                            format!(": {message}"),
+                        ),
+                    };
+                    err.span_suggestion_verbose(
+                        span,
+                        format!("consider {changing} this binding's type"),
+                        sugg,
+                        Applicability::HasPlaceholders,
+                    );
+                } else {
+                    err.span_label(
+                        err_label_span,
+                        format!(
+                            "consider changing this binding's type to be: `{message}`"
+                        ),
+                    );
+                }
             }
             None => {}
         }
diff --git a/tests/ui/borrowck/copy-suggestion-region-vid.stderr b/tests/ui/borrowck/copy-suggestion-region-vid.stderr
@@ -8,6 +8,11 @@ LL |         HelperStruct { helpers, is_empty: helpers[0].is_empty() }
    |                        -------            ^^^^^^^^^^ value borrowed here after move
    |                        |
    |                        value moved here
+   |
+help: consider cloning the value if the performance cost is acceptable
+   |
+LL |         HelperStruct { helpers.clone(), is_empty: helpers[0].is_empty() }
+   |                               ++++++++
 
 error: aborting due to previous error
 
diff --git a/tests/ui/borrowck/issue-85765-closure.rs b/tests/ui/borrowck/issue-85765-closure.rs
@@ -0,0 +1,31 @@
+fn main() {
+    let _ = || {
+        let mut test = Vec::new();
+        let rofl: &Vec<Vec<i32>> = &mut test;
+        //~^ HELP consider changing this binding's type
+        rofl.push(Vec::new());
+        //~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
+        //~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
+
+        let mut mutvar = 42;
+        let r = &mutvar;
+        //~^ HELP consider changing this to be a mutable reference
+        *r = 0;
+        //~^ ERROR cannot assign to `*r`, which is behind a `&` reference
+        //~| NOTE `r` is a `&` reference, so the data it refers to cannot be written
+
+        #[rustfmt::skip]
+        let x: &usize = &mut{0};
+        //~^ HELP consider changing this binding's type
+        *x = 1;
+        //~^ ERROR cannot assign to `*x`, which is behind a `&` reference
+        //~| NOTE `x` is a `&` reference, so the data it refers to cannot be written
+
+        #[rustfmt::skip]
+        let y: &usize = &mut(0);
+        //~^ HELP consider changing this binding's type
+        *y = 1;
+        //~^ ERROR cannot assign to `*y`, which is behind a `&` reference
+        //~| NOTE `y` is a `&` reference, so the data it refers to cannot be written
+    };
+}
diff --git a/tests/ui/borrowck/issue-85765-closure.stderr b/tests/ui/borrowck/issue-85765-closure.stderr
@@ -0,0 +1,48 @@
+error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
+  --> $DIR/issue-85765-closure.rs:6:9
+   |
+LL |         rofl.push(Vec::new());
+   |         ^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
+   |
+help: consider changing this binding's type
+   |
+LL |         let rofl: &mut Vec<Vec<i32>> = &mut test;
+   |                   ~~~~~~~~~~~~~~~~~~
+
+error[E0594]: cannot assign to `*r`, which is behind a `&` reference
+  --> $DIR/issue-85765-closure.rs:13:9
+   |
+LL |         *r = 0;
+   |         ^^^^^^ `r` is a `&` reference, so the data it refers to cannot be written
+   |
+help: consider changing this to be a mutable reference
+   |
+LL |         let r = &mut mutvar;
+   |                  +++
+
+error[E0594]: cannot assign to `*x`, which is behind a `&` reference
+  --> $DIR/issue-85765-closure.rs:20:9
+   |
+LL |         *x = 1;
+   |         ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
+   |
+help: consider changing this binding's type
+   |
+LL |         let x: &mut usize = &mut{0};
+   |                ~~~~~~~~~~
+
+error[E0594]: cannot assign to `*y`, which is behind a `&` reference
+  --> $DIR/issue-85765-closure.rs:27:9
+   |
+LL |         *y = 1;
+   |         ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written
+   |
+help: consider changing this binding's type
+   |
+LL |         let y: &mut usize = &mut(0);
+   |                ~~~~~~~~~~
+
+error: aborting due to 4 previous errors
+
+Some errors have detailed explanations: E0594, E0596.
+For more information about an error, try `rustc --explain E0594`.
diff --git a/tests/ui/btreemap/btreemap-index-mut-2.rs b/tests/ui/btreemap/btreemap-index-mut-2.rs
@@ -0,0 +1,8 @@
+use std::collections::BTreeMap;
+
+fn main() {
+    let _ = || {
+        let mut map = BTreeMap::<u32, u32>::new();
+        map[&0] = 1; //~ ERROR cannot assign
+    };
+}
diff --git a/tests/ui/btreemap/btreemap-index-mut-2.stderr b/tests/ui/btreemap/btreemap-index-mut-2.stderr
@@ -0,0 +1,19 @@
+error[E0594]: cannot assign to data in an index of `BTreeMap<u32, u32>`
+  --> $DIR/btreemap-index-mut-2.rs:6:9
+   |
+LL |         map[&0] = 1;
+   |         ^^^^^^^^^^^ cannot assign
+   |
+   = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>`
+help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
+   |
+LL |         map.insert(&0, 1);
+   |            ~~~~~~~~  ~  +
+LL |         map.get_mut(&0).map(|val| { *val = 1; });
+   |            ~~~~~~~~~  ~~~~~~~~~~~~~~~~~~    ++++
+LL |         let val = map.entry(&0).or_insert(1);
+   |         +++++++++    ~~~~~~~  ~~~~~~~~~~~~ +
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0594`.
diff --git a/tests/ui/liveness/liveness-move-call-arg-2.rs b/tests/ui/liveness/liveness-move-call-arg-2.rs
@@ -0,0 +1,12 @@
+fn take(_x: Box<isize>) {}
+
+
+fn main() {
+    let _ = || {
+        let x: Box<isize> = Box::new(25);
+
+        loop {
+            take(x); //~ ERROR use of moved value: `x`
+        }
+    };
+}
diff --git a/tests/ui/liveness/liveness-move-call-arg-2.stderr b/tests/ui/liveness/liveness-move-call-arg-2.stderr
@@ -0,0 +1,26 @@
+error[E0382]: use of moved value: `x`
+  --> $DIR/liveness-move-call-arg-2.rs:9:18
+   |
+LL |         let x: Box<isize> = Box::new(25);
+   |             - move occurs because `x` has type `Box<isize>`, which does not implement the `Copy` trait
+LL |
+LL |         loop {
+   |         ---- inside of this loop
+LL |             take(x);
+   |                  ^ value moved here, in previous iteration of loop
+   |
+note: consider changing this parameter type in function `take` to borrow instead if owning the value isn't necessary
+  --> $DIR/liveness-move-call-arg-2.rs:1:13
+   |
+LL | fn take(_x: Box<isize>) {}
+   |    ----     ^^^^^^^^^^ this parameter takes ownership of the value
+   |    |
+   |    in this function
+help: consider cloning the value if the performance cost is acceptable
+   |
+LL |             take(x.clone());
+   |                   ++++++++
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0382`.
diff --git a/tests/ui/suggestions/suggest-mut-method-for-loop-closure.rs b/tests/ui/suggestions/suggest-mut-method-for-loop-closure.rs
@@ -0,0 +1,19 @@
+use std::collections::HashMap;
+struct X(usize);
+struct Y {
+    v: u32,
+}
+
+fn main() {
+    let _ = || {
+        let mut buzz = HashMap::new();
+        buzz.insert("a", Y { v: 0 });
+
+        for mut t in buzz.values() {
+            //~^ HELP
+            //~| SUGGESTION values_mut()
+            t.v += 1;
+            //~^ ERROR cannot assign
+        }
+    };
+}
diff --git a/tests/ui/suggestions/suggest-mut-method-for-loop-closure.stderr b/tests/ui/suggestions/suggest-mut-method-for-loop-closure.stderr
@@ -0,0 +1,15 @@
+error[E0594]: cannot assign to `t.v`, which is behind a `&` reference
+  --> $DIR/suggest-mut-method-for-loop-closure.rs:15:13
+   |
+LL |         for mut t in buzz.values() {
+   |                      -------------
+   |                      |    |
+   |                      |    help: use mutable method: `values_mut()`
+   |                      this iterator yields `&` references
+...
+LL |             t.v += 1;
+   |             ^^^^^^^^ `t` is a `&` reference, so the data it refers to cannot be written
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0594`.
