From e4e9bb4a244464049651c66b408639f60fcdcc58 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 7 Nov 2020 20:11:53 -0500 Subject: [PATCH] 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. --- .../transform/check_const_item_mutation.rs | 51 +++++++++++-------- src/test/ui/lint/lint-const-item-mutation.rs | 5 ++ .../ui/lint/lint-const-item-mutation.stderr | 16 +++--- 3 files changed, 43 insertions(+), 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 index fb89b36060a28..74fe589e4a946 100644 --- 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 index c49a13f1065b5..ef55f31593b63 100644 --- 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 = 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 index 11b5124b2d26a..d9195a2319f54 100644 --- 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); | ^^^^^^^^^^^