Don't fire CONST_ITEM_MUTATION lint when borrowing a deref

Fixes #78819 This extends the check for dereferences added in PR #77324 to cover mutable borrows, as well as direct writes. If we're operating on a dereference of a `const` item, we shouldn't be firing the lint.
rust-lang · Nov 8, 2020 · e4e9bb4 · e4e9bb4
1 parent b2d115f
commit e4e9bb4
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 29 deletions.
diff --git a/compiler/rustc_mir/src/transform/check_const_item_mutation.rs b/compiler/rustc_mir/src/transform/check_const_item_mutation.rs
@@ -61,22 +61,35 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> {
 
     fn lint_const_item_usage(
         &self,
+        place: &Place<'tcx>,
         const_item: DefId,
         location: Location,
         decorate: impl for<'b> FnOnce(LintDiagnosticBuilder<'b>) -> DiagnosticBuilder<'b>,
     ) {
-        let source_info = self.body.source_info(location);
-        let lint_root = self.body.source_scopes[source_info.scope]
-            .local_data
-            .as_ref()
-            .assert_crate_local()
-            .lint_root;
+        // Don't lint on borrowing/assigning to a dereference
+        // e.g:
+        //
+        // `unsafe { *FOO = 0; *BAR.field = 1; }`
+        // `unsafe { &mut *FOO }`
+        if !matches!(place.projection.last(), Some(PlaceElem::Deref)) {
+            let source_info = self.body.source_info(location);
+            let lint_root = self.body.source_scopes[source_info.scope]
+                .local_data
+                .as_ref()
+                .assert_crate_local()
+                .lint_root;
 
-        self.tcx.struct_span_lint_hir(CONST_ITEM_MUTATION, lint_root, source_info.span, |lint| {
-            decorate(lint)
-                .span_note(self.tcx.def_span(const_item), "`const` item defined here")
-                .emit()
-        });
+            self.tcx.struct_span_lint_hir(
+                CONST_ITEM_MUTATION,
+                lint_root,
+                source_info.span,
+                |lint| {
+                    decorate(lint)
+                        .span_note(self.tcx.def_span(const_item), "`const` item defined here")
+                        .emit()
+                },
+            );
+        }
     }
 }
 
@@ -88,15 +101,11 @@ impl<'a, 'tcx> Visitor<'tcx> for ConstMutationChecker<'a, 'tcx> {
             // so emitting a lint would be redundant.
             if !lhs.projection.is_empty() {
                 if let Some(def_id) = self.is_const_item_without_destructor(lhs.local) {
-                    // Don't lint on writes through a pointer
-                    // (e.g. `unsafe { *FOO = 0; *BAR.field = 1; }`)
-                    if !matches!(lhs.projection.last(), Some(PlaceElem::Deref)) {
-                        self.lint_const_item_usage(def_id, loc, |lint| {
-                            let mut lint = lint.build("attempting to modify a `const` item");
-                            lint.note("each usage of a `const` item creates a new temporary - the original `const` item will not be modified");
-                            lint
-                        })
-                    }
+                    self.lint_const_item_usage(&lhs, def_id, loc, |lint| {
+                        let mut lint = lint.build("attempting to modify a `const` item");
+                        lint.note("each usage of a `const` item creates a new temporary - the original `const` item will not be modified");
+                        lint
+                    })
                 }
             }
             // We are looking for MIR of the form:
@@ -127,7 +136,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ConstMutationChecker<'a, 'tcx> {
                 });
                 let lint_loc =
                     if method_did.is_some() { self.body.terminator_loc(loc.block) } else { loc };
-                self.lint_const_item_usage(def_id, lint_loc, |lint| {
+                self.lint_const_item_usage(place, def_id, lint_loc, |lint| {
                     let mut lint = lint.build("taking a mutable reference to a `const` item");
                     lint
                         .note("each usage of a `const` item creates a new temporary")

diff --git a/src/test/ui/lint/lint-const-item-mutation.rs b/src/test/ui/lint/lint-const-item-mutation.rs
@@ -29,6 +29,7 @@ const RAW_PTR: *mut u8 = 1 as *mut u8;
 const MUTABLE: Mutable = Mutable { msg: "" };
 const MUTABLE2: Mutable2 = Mutable2 { msg: "", other: String::new() };
 const VEC: Vec<i32> = Vec::new();
+const PTR: *mut () = 1 as *mut _;
 
 fn main() {
     ARRAY[0] = 5; //~ WARN attempting to modify
@@ -50,4 +51,8 @@ fn main() {
     MUTABLE.msg = "wow"; // no warning, because Drop observes the mutation
     MUTABLE2.msg = "wow"; //~ WARN attempting to modify
     VEC.push(0); //~ WARN taking a mutable reference to a `const` item
+
+    // Test that we don't warn when converting a raw pointer
+    // into a mutable reference
+    unsafe { &mut *PTR };
 }
diff --git a/src/test/ui/lint/lint-const-item-mutation.stderr b/src/test/ui/lint/lint-const-item-mutation.stderr
@@ -1,5 +1,5 @@
 warning: attempting to modify a `const` item
-  --> $DIR/lint-const-item-mutation.rs:34:5
+  --> $DIR/lint-const-item-mutation.rs:35:5
    |
 LL |     ARRAY[0] = 5;
    |     ^^^^^^^^^^^^
@@ -13,7 +13,7 @@ LL | const ARRAY: [u8; 1] = [25];
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: attempting to modify a `const` item
-  --> $DIR/lint-const-item-mutation.rs:35:5
+  --> $DIR/lint-const-item-mutation.rs:36:5
    |
 LL |     MY_STRUCT.field = false;
    |     ^^^^^^^^^^^^^^^^^^^^^^^
@@ -26,7 +26,7 @@ LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: attempting to modify a `const` item
-  --> $DIR/lint-const-item-mutation.rs:36:5
+  --> $DIR/lint-const-item-mutation.rs:37:5
    |
 LL |     MY_STRUCT.inner_array[0] = 'b';
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -39,7 +39,7 @@ LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: taking a mutable reference to a `const` item
-  --> $DIR/lint-const-item-mutation.rs:37:5
+  --> $DIR/lint-const-item-mutation.rs:38:5
    |
 LL |     MY_STRUCT.use_mut();
    |     ^^^^^^^^^^^^^^^^^^^
@@ -58,7 +58,7 @@ LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: taking a mutable reference to a `const` item
-  --> $DIR/lint-const-item-mutation.rs:38:5
+  --> $DIR/lint-const-item-mutation.rs:39:5
    |
 LL |     &mut MY_STRUCT;
    |     ^^^^^^^^^^^^^^
@@ -72,7 +72,7 @@ LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: taking a mutable reference to a `const` item
-  --> $DIR/lint-const-item-mutation.rs:39:5
+  --> $DIR/lint-const-item-mutation.rs:40:5
    |
 LL |     (&mut MY_STRUCT).use_mut();
    |     ^^^^^^^^^^^^^^^^
@@ -86,7 +86,7 @@ LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: attempting to modify a `const` item
-  --> $DIR/lint-const-item-mutation.rs:51:5
+  --> $DIR/lint-const-item-mutation.rs:52:5
    |
 LL |     MUTABLE2.msg = "wow";
    |     ^^^^^^^^^^^^^^^^^^^^
@@ -99,7 +99,7 @@ LL | const MUTABLE2: Mutable2 = Mutable2 { msg: "", other: String::new() };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: taking a mutable reference to a `const` item
-  --> $DIR/lint-const-item-mutation.rs:52:5
+  --> $DIR/lint-const-item-mutation.rs:53:5
    |
 LL |     VEC.push(0);
    |     ^^^^^^^^^^^