Make conflicting borrow description more robust.

This commit improves the logic for place descriptions in conflicting
borrow errors so that borrows of union fields have better messages even
when the unions are embedded in other unions or structs.

diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs
index 6f12ff994c4b2976c36723b434c730fb5877b1fe..aeba56cb7e26094849877e9c9b8f5ecc673ff986 100644 (file)
--- a/src/librustc_mir/borrow_check/error_reporting.rs
+++ b/src/librustc_mir/borrow_check/error_reporting.rs
@@ -329,16 +329,12 @@ pub(super) fn report_conflicting_borrow(
             "closure"
         };
 
-        let (desc_place, msg_place, msg_borrow) = if issued_borrow.borrowed_place == *place {
-            let desc_place = self.describe_place(place).unwrap_or_else(|| "_".to_owned());
-            (desc_place, "".to_string(), "".to_string())
-        } else {
-            let (desc_place, msg_place) = self.describe_place_for_conflicting_borrow(place);
-            let (_, msg_borrow) = self.describe_place_for_conflicting_borrow(
-                &issued_borrow.borrowed_place
-            );
-            (desc_place, msg_place, msg_borrow)
-        };
+        let (desc_place, msg_place, msg_borrow) = self.describe_place_for_conflicting_borrow(
+            place, &issued_borrow.borrowed_place,
+        );
+        let via = |msg: String| if msg.is_empty() { msg } else { format!(" (via `{}`)", msg) };
+        let msg_place = via(msg_place);
+        let msg_borrow = via(msg_borrow);
 
         let explanation = self.explain_why_borrow_contains_point(context, issued_borrow, None);
         let second_borrow_desc = if explanation.is_explained() {
@@ -526,33 +522,97 @@ pub(super) fn report_conflicting_borrow(
         err.buffer(&mut self.errors_buffer);
     }
 
-    /// Returns a description of a place and an associated message for the purposes of conflicting
-    /// borrow diagnostics.
+    /// Returns the description of the root place for a conflicting borrow and the full
+    /// descriptions of the places that caused the conflict.
+    ///
+    /// In the simplest case, where there are no unions involved, if a mutable borrow of `x` is
+    /// attempted while a shared borrow is live, then this function will return:
+    ///
+    ///     ("x", "", "")
     ///
-    /// If the borrow is of the field `b` of a union `u`, then the return value will be
-    /// `("u", " (via \`u.b\`)")`. Otherwise, for some variable `a`, the return value will be
-    /// `("a", "")`.
+    /// In the simple union case, if a mutable borrow of a union field `x.z` is attempted while
+    /// a shared borrow of another field `x.y`, then this function will return:
+    ///
+    ///     ("x", "x.z", "x.y")
+    ///
+    /// In the more complex union case, where the union is a field of a struct, then if a mutable
+    /// borrow of a union field in a struct `x.u.z` is attempted while a shared borrow of
+    /// another field `x.u.y`, then this function will return:
+    ///
+    ///     ("x.u", "x.u.z", "x.u.y")
+    ///
+    /// This is used when creating error messages like below:
+    ///
+    /// >  cannot borrow `a.u` (via `a.u.z.c`) as immutable because it is also borrowed as
+    /// >  mutable (via `a.u.s.b`) [E0502]
     pub(super) fn describe_place_for_conflicting_borrow(
         &self,
-        place: &Place<'tcx>,
-    ) -> (String, String) {
-        place.base_local()
-            .filter(|local| {
-                // Filter out non-unions.
-                self.mir.local_decls[*local].ty
-                    .ty_adt_def()
-                    .map(|adt| adt.is_union())
-                    .unwrap_or(false)
+        first_borrowed_place: &Place<'tcx>,
+        second_borrowed_place: &Place<'tcx>,
+    ) -> (String, String, String) {
+        // Define a small closure that we can use to check if the type of a place
+        // is a union.
+        let is_union = |place: &Place<'tcx>| -> bool {
+            place.ty(self.mir, self.infcx.tcx)
+                .to_ty(self.infcx.tcx)
+                .ty_adt_def()
+                .map(|adt| adt.is_union())
+                .unwrap_or(false)
+        };
+
+        // Start with an empty tuple, so we can use the functions on `Option` to reduce some
+        // code duplication (particularly around returning an empty description in the failure
+        // case).
+        Some(())
+            .filter(|_| {
+                // If we have a conflicting borrow of the same place, then we don't want to add
+                // an extraneous "via x.y" to our diagnostics, so filter out this case.
+                first_borrowed_place != second_borrowed_place
             })
-            .and_then(|local| {
-                let desc_base = self.describe_place(&Place::Local(local))
-                    .unwrap_or_else(|| "_".to_owned());
-                let desc_original = self.describe_place(place)
-                    .unwrap_or_else(|| "_".to_owned());
-                return Some((desc_base, format!(" (via `{}`)", desc_original)));
+            .and_then(|_| {
+                // We're going to want to traverse the first borrowed place to see if we can find
+                // field access to a union. If we find that, then we will keep the place of the
+                // union being accessed and the field that was being accessed so we can check the
+                // second borrowed place for the same union and a access to a different field.
+                let mut current = first_borrowed_place;
+                while let Place::Projection(box PlaceProjection { base, elem }) = current {
+                    match elem {
+                        ProjectionElem::Field(field, _) if is_union(base) => {
+                            return Some((base, field));
+                        },
+                        _ => current = base,
+                    }
+                }
+                None
+            })
+            .and_then(|(target_base, target_field)| {
+                // With the place of a union and a field access into it, we traverse the second
+                // borrowed place and look for a access to a different field of the same union.
+                let mut current = second_borrowed_place;
+                while let Place::Projection(box PlaceProjection { base, elem }) = current {
+                    match elem {
+                        ProjectionElem::Field(field, _) if {
+                            is_union(base) && field != target_field && base == target_base
+                        } => {
+                            let desc_base = self.describe_place(base)
+                                .unwrap_or_else(|| "_".to_owned());
+                            let desc_first = self.describe_place(first_borrowed_place)
+                                .unwrap_or_else(|| "_".to_owned());
+                            let desc_second = self.describe_place(second_borrowed_place)
+                                .unwrap_or_else(|| "_".to_owned());
+                            return Some((desc_base, desc_first, desc_second));
+                        },
+                        _ => current = base,
+                    }
+                }
+                None
             })
             .unwrap_or_else(|| {
-                (self.describe_place(place).unwrap_or_else(|| "_".to_owned()), "".to_string())
+                // If we didn't find a field access into a union, or both places match, then
+                // only return the description of the first place.
+                let desc_place = self.describe_place(first_borrowed_place)
+                    .unwrap_or_else(|| "_".to_owned());
+                (desc_place, "".to_string(), "".to_string())
             })
     }
 

diff --git a/src/test/ui/nll/issue-57100.rs b/src/test/ui/nll/issue-57100.rs
new file mode 100644 (file)
index 0000000..f669fe0
--- /dev/null
+++ b/src/test/ui/nll/issue-57100.rs
@@ -0,0 +1,69 @@
+#![allow(unused)]
+#![feature(nll)]
+
+// ignore-tidy-linelength
+
+// This tests the error messages for borrows of union fields when the unions are embedded in other
+// structs or unions.
+
+#[derive(Clone, Copy, Default)]
+struct Leaf {
+    l1_u8: u8,
+    l2_u8: u8,
+}
+
+#[derive(Clone, Copy)]
+union First {
+    f1_leaf: Leaf,
+    f2_leaf: Leaf,
+    f3_union: Second,
+}
+
+#[derive(Clone, Copy)]
+union Second {
+    s1_leaf: Leaf,
+    s2_leaf: Leaf,
+}
+
+struct Root {
+    r1_u8: u8,
+    r2_union: First,
+}
+
+// Borrow a different field of the nested union.
+fn nested_union() {
+    unsafe {
+        let mut r = Root {
+            r1_u8: 3,
+            r2_union: First { f3_union: Second { s2_leaf: Leaf { l1_u8: 8, l2_u8: 4 } } }
+        };
+
+        let mref = &mut r.r2_union.f3_union.s1_leaf.l1_u8;
+        //                                  ^^^^^^^
+        *mref = 22;
+        let nref = &r.r2_union.f3_union.s2_leaf.l1_u8;
+        //                              ^^^^^^^
+        //~^^ ERROR cannot borrow `r.r2_union.f3_union` (via `r.r2_union.f3_union.s2_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f3_union.s1_leaf.l1_u8`) [E0502]
+        println!("{} {}", mref, nref)
+    }
+}
+
+// Borrow a different field of the first union.
+fn first_union() {
+    unsafe {
+        let mut r = Root {
+            r1_u8: 3,
+            r2_union: First { f3_union: Second { s2_leaf: Leaf { l1_u8: 8, l2_u8: 4 } } }
+        };
+
+        let mref = &mut r.r2_union.f2_leaf.l1_u8;
+        //                         ^^^^^^^
+        *mref = 22;
+        let nref = &r.r2_union.f1_leaf.l1_u8;
+        //                     ^^^^^^^
+        //~^^ ERROR cannot borrow `r.r2_union` (via `r.r2_union.f1_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f2_leaf.l1_u8`) [E0502]
+        println!("{} {}", mref, nref)
+    }
+}
+
+fn main() {}

diff --git a/src/test/ui/nll/issue-57100.stderr b/src/test/ui/nll/issue-57100.stderr
new file mode 100644 (file)
index 0000000..34dcdfc
--- /dev/null
+++ b/src/test/ui/nll/issue-57100.stderr
@@ -0,0 +1,27 @@
+error[E0502]: cannot borrow `r.r2_union.f3_union` (via `r.r2_union.f3_union.s2_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f3_union.s1_leaf.l1_u8`)
+  --> $DIR/issue-57100.rs:44:20
+   |
+LL |         let mref = &mut r.r2_union.f3_union.s1_leaf.l1_u8;
+   |                    -------------------------------------- mutable borrow occurs here (via `r.r2_union.f3_union.s1_leaf.l1_u8`)
+...
+LL |         let nref = &r.r2_union.f3_union.s2_leaf.l1_u8;
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here (via `r.r2_union.f3_union.s2_leaf.l1_u8`)
+...
+LL |         println!("{} {}", mref, nref)
+   |                           ---- mutable borrow later used here
+
+error[E0502]: cannot borrow `r.r2_union` (via `r.r2_union.f1_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f2_leaf.l1_u8`)
+  --> $DIR/issue-57100.rs:62:20
+   |
+LL |         let mref = &mut r.r2_union.f2_leaf.l1_u8;
+   |                    ----------------------------- mutable borrow occurs here (via `r.r2_union.f2_leaf.l1_u8`)
+...
+LL |         let nref = &r.r2_union.f1_leaf.l1_u8;
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here (via `r.r2_union.f1_leaf.l1_u8`)
+...
+LL |         println!("{} {}", mref, nref)
+   |                           ---- mutable borrow later used here
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0502`.