From 17db1cb5d5a864d611e17d6e2466731c3b50a794 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 26 Sep 2020 21:14:31 -0400 Subject: [PATCH 1/9] Bypass const_item_mutation if const's type has Drop impl --- .../src/transform/check_const_item_mutation.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 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 0281c478a6ca0..b556e2d217d6d 100644 --- a/compiler/rustc_mir/src/transform/check_const_item_mutation.rs +++ b/compiler/rustc_mir/src/transform/check_const_item_mutation.rs @@ -31,6 +31,15 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> { None } } + + fn is_const_item_without_destructor(&self, local: Local) -> Option { + let def_id = self.is_const_item(local)?; + match self.tcx.adt_def(def_id).destructor(self.tcx) { + Some(_) => None, + None => Some(def_id), + } + } + fn lint_const_item_usage( &self, const_item: DefId, @@ -59,7 +68,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ConstMutationChecker<'a, 'tcx> { // Assigning directly to a constant (e.g. `FOO = true;`) is a hard error, // so emitting a lint would be redundant. if !lhs.projection.is_empty() { - if let Some(def_id) = self.is_const_item(lhs.local) { + 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)) { @@ -89,7 +98,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ConstMutationChecker<'a, 'tcx> { fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, loc: Location) { if let Rvalue::Ref(_, BorrowKind::Mut { .. }, place) = rvalue { let local = place.local; - if let Some(def_id) = self.is_const_item(local) { + if let Some(def_id) = self.is_const_item_without_destructor(local) { // If this Rvalue is being used as the right-hand side of a // `StatementKind::Assign`, see if it ends up getting used as // the `self` parameter of a method call (as the terminator of our current From 0dfe7b6434f6aabbe9673a891ba74c7c7922661d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 26 Sep 2020 21:19:08 -0400 Subject: [PATCH 2/9] Add justification of the destructor filter --- .../src/transform/check_const_item_mutation.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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 b556e2d217d6d..aae6794994594 100644 --- a/compiler/rustc_mir/src/transform/check_const_item_mutation.rs +++ b/compiler/rustc_mir/src/transform/check_const_item_mutation.rs @@ -34,6 +34,18 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> { fn is_const_item_without_destructor(&self, local: Local) -> Option { let def_id = self.is_const_item(local)?; + + // We avoid linting mutation of a const item if the const's type has a + // Drop impl. The Drop logic observes the mutation which was performed. + // + // struct Log { msg: &'static str } + // const LOG: Log = Log { msg: "" }; + // impl Drop for Log { + // fn drop(&mut self) { println!("{}", self.msg); } + // } + // + // LOG.msg = "wow"; // prints "wow" + // match self.tcx.adt_def(def_id).destructor(self.tcx) { Some(_) => None, None => Some(def_id), From bb760b53bf7f9593c09a03e5dea28ef5f31065d7 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 26 Sep 2020 18:36:01 -0700 Subject: [PATCH 3/9] Simplify defid destructor check --- compiler/rustc_mir/src/transform/check_const_item_mutation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 aae6794994594..112c25a312930 100644 --- a/compiler/rustc_mir/src/transform/check_const_item_mutation.rs +++ b/compiler/rustc_mir/src/transform/check_const_item_mutation.rs @@ -46,7 +46,7 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> { // // LOG.msg = "wow"; // prints "wow" // - match self.tcx.adt_def(def_id).destructor(self.tcx) { + match self.tcx.adt_destructor(def_id) { Some(_) => None, None => Some(def_id), } From 0f6284c88d521904bb0ada34325a0372a1ea26f1 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 26 Sep 2020 18:42:19 -0700 Subject: [PATCH 4/9] Add test of const item mutation with Drop impl --- src/test/ui/lint/lint-const-item-mutation.rs | 12 ++++++++++ .../ui/lint/lint-const-item-mutation.stderr | 24 +++++++++---------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/test/ui/lint/lint-const-item-mutation.rs b/src/test/ui/lint/lint-const-item-mutation.rs index 43371560e02c1..0fb97367737d6 100644 --- a/src/test/ui/lint/lint-const-item-mutation.rs +++ b/src/test/ui/lint/lint-const-item-mutation.rs @@ -9,9 +9,19 @@ impl MyStruct { fn use_mut(&mut self) {} } +struct Mutable { + msg: &'static str, +} +impl Drop for Mutable { + fn drop(&mut self) { + println!("{}", self.msg); + } +} + const ARRAY: [u8; 1] = [25]; const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; const RAW_PTR: *mut u8 = 1 as *mut u8; +const MUTABLE: Mutable = Mutable { msg: "" }; fn main() { ARRAY[0] = 5; //~ WARN attempting to modify @@ -29,4 +39,6 @@ fn main() { *RAW_PTR = 0; *MY_STRUCT.raw_ptr = 0; } + + MUTABLE.msg = "wow"; // no warning, because Drop observes the mutation } diff --git a/src/test/ui/lint/lint-const-item-mutation.stderr b/src/test/ui/lint/lint-const-item-mutation.stderr index c5a221128ffab..6ede7b31666c8 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:17:5 + --> $DIR/lint-const-item-mutation.rs:27:5 | LL | ARRAY[0] = 5; | ^^^^^^^^^^^^ @@ -7,39 +7,39 @@ LL | ARRAY[0] = 5; = note: `#[warn(const_item_mutation)]` on by default = note: each usage of a `const` item creates a new temporary - the original `const` item will not be modified note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:12:1 + --> $DIR/lint-const-item-mutation.rs:21:1 | LL | const ARRAY: [u8; 1] = [25]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: attempting to modify a `const` item - --> $DIR/lint-const-item-mutation.rs:18:5 + --> $DIR/lint-const-item-mutation.rs:28:5 | LL | MY_STRUCT.field = false; | ^^^^^^^^^^^^^^^^^^^^^^^ | = note: each usage of a `const` item creates a new temporary - the original `const` item will not be modified note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:13:1 + --> $DIR/lint-const-item-mutation.rs:22:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: attempting to modify a `const` item - --> $DIR/lint-const-item-mutation.rs:19:5 + --> $DIR/lint-const-item-mutation.rs:29:5 | LL | MY_STRUCT.inner_array[0] = 'b'; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: each usage of a `const` item creates a new temporary - the original `const` item will not be modified note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:13:1 + --> $DIR/lint-const-item-mutation.rs:22:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: taking a mutable reference to a `const` item - --> $DIR/lint-const-item-mutation.rs:20:5 + --> $DIR/lint-const-item-mutation.rs:30:5 | LL | MY_STRUCT.use_mut(); | ^^^^^^^^^^^^^^^^^^^ @@ -52,13 +52,13 @@ note: mutable reference created due to call to this method LL | fn use_mut(&mut self) {} | ^^^^^^^^^^^^^^^^^^^^^ note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:13:1 + --> $DIR/lint-const-item-mutation.rs:22:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: taking a mutable reference to a `const` item - --> $DIR/lint-const-item-mutation.rs:21:5 + --> $DIR/lint-const-item-mutation.rs:31:5 | LL | &mut MY_STRUCT; | ^^^^^^^^^^^^^^ @@ -66,13 +66,13 @@ LL | &mut MY_STRUCT; = note: each usage of a `const` item creates a new temporary = note: the mutable reference will refer to this temporary, not the original `const` item note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:13:1 + --> $DIR/lint-const-item-mutation.rs:22:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: taking a mutable reference to a `const` item - --> $DIR/lint-const-item-mutation.rs:22:5 + --> $DIR/lint-const-item-mutation.rs:32:5 | LL | (&mut MY_STRUCT).use_mut(); | ^^^^^^^^^^^^^^^^ @@ -80,7 +80,7 @@ LL | (&mut MY_STRUCT).use_mut(); = note: each usage of a `const` item creates a new temporary = note: the mutable reference will refer to this temporary, not the original `const` item note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:13:1 + --> $DIR/lint-const-item-mutation.rs:22:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 352ce8b29990d62465675b46b42a695a4206d5be Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 26 Sep 2020 21:19:46 -0700 Subject: [PATCH 5/9] Test a type with drop glue but no Drop impl --- src/test/ui/lint/lint-const-item-mutation.rs | 7 ++++ .../ui/lint/lint-const-item-mutation.stderr | 39 ++++++++++++------- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/test/ui/lint/lint-const-item-mutation.rs b/src/test/ui/lint/lint-const-item-mutation.rs index 0fb97367737d6..37e6420fc9832 100644 --- a/src/test/ui/lint/lint-const-item-mutation.rs +++ b/src/test/ui/lint/lint-const-item-mutation.rs @@ -18,10 +18,16 @@ impl Drop for Mutable { } } +struct Mutable2 { // this one has drop glue but not a Drop impl + msg: &'static str, + other: String, +} + const ARRAY: [u8; 1] = [25]; const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; const RAW_PTR: *mut u8 = 1 as *mut u8; const MUTABLE: Mutable = Mutable { msg: "" }; +const MUTABLE2: Mutable2 = Mutable2 { msg: "", other: String::new() }; fn main() { ARRAY[0] = 5; //~ WARN attempting to modify @@ -41,4 +47,5 @@ fn main() { } MUTABLE.msg = "wow"; // no warning, because Drop observes the mutation + MUTABLE2.msg = "wow"; //~ WARN attempting to modify } diff --git a/src/test/ui/lint/lint-const-item-mutation.stderr b/src/test/ui/lint/lint-const-item-mutation.stderr index 6ede7b31666c8..20a8ed0d78386 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:27:5 + --> $DIR/lint-const-item-mutation.rs:33:5 | LL | ARRAY[0] = 5; | ^^^^^^^^^^^^ @@ -7,39 +7,39 @@ LL | ARRAY[0] = 5; = note: `#[warn(const_item_mutation)]` on by default = note: each usage of a `const` item creates a new temporary - the original `const` item will not be modified note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:21:1 + --> $DIR/lint-const-item-mutation.rs:26:1 | LL | const ARRAY: [u8; 1] = [25]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: attempting to modify a `const` item - --> $DIR/lint-const-item-mutation.rs:28:5 + --> $DIR/lint-const-item-mutation.rs:34:5 | LL | MY_STRUCT.field = false; | ^^^^^^^^^^^^^^^^^^^^^^^ | = note: each usage of a `const` item creates a new temporary - the original `const` item will not be modified note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:22:1 + --> $DIR/lint-const-item-mutation.rs:27:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: attempting to modify a `const` item - --> $DIR/lint-const-item-mutation.rs:29:5 + --> $DIR/lint-const-item-mutation.rs:35:5 | LL | MY_STRUCT.inner_array[0] = 'b'; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: each usage of a `const` item creates a new temporary - the original `const` item will not be modified note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:22:1 + --> $DIR/lint-const-item-mutation.rs:27:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: taking a mutable reference to a `const` item - --> $DIR/lint-const-item-mutation.rs:30:5 + --> $DIR/lint-const-item-mutation.rs:36:5 | LL | MY_STRUCT.use_mut(); | ^^^^^^^^^^^^^^^^^^^ @@ -52,13 +52,13 @@ note: mutable reference created due to call to this method LL | fn use_mut(&mut self) {} | ^^^^^^^^^^^^^^^^^^^^^ note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:22:1 + --> $DIR/lint-const-item-mutation.rs:27:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: taking a mutable reference to a `const` item - --> $DIR/lint-const-item-mutation.rs:31:5 + --> $DIR/lint-const-item-mutation.rs:37:5 | LL | &mut MY_STRUCT; | ^^^^^^^^^^^^^^ @@ -66,13 +66,13 @@ LL | &mut MY_STRUCT; = note: each usage of a `const` item creates a new temporary = note: the mutable reference will refer to this temporary, not the original `const` item note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:22:1 + --> $DIR/lint-const-item-mutation.rs:27:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: taking a mutable reference to a `const` item - --> $DIR/lint-const-item-mutation.rs:32:5 + --> $DIR/lint-const-item-mutation.rs:38:5 | LL | (&mut MY_STRUCT).use_mut(); | ^^^^^^^^^^^^^^^^ @@ -80,10 +80,23 @@ LL | (&mut MY_STRUCT).use_mut(); = note: each usage of a `const` item creates a new temporary = note: the mutable reference will refer to this temporary, not the original `const` item note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:22:1 + --> $DIR/lint-const-item-mutation.rs:27:1 | LL | const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 2 as *mut u8 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -warning: 6 warnings emitted +warning: attempting to modify a `const` item + --> $DIR/lint-const-item-mutation.rs:50:5 + | +LL | MUTABLE2.msg = "wow"; + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: each usage of a `const` item creates a new temporary - the original `const` item will not be modified +note: `const` item defined here + --> $DIR/lint-const-item-mutation.rs:30:1 + | +LL | const MUTABLE2: Mutable2 = Mutable2 { msg: "", other: String::new() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: 7 warnings emitted From 41baa090ad1c40a7fc96f96664543ce4c26746c2 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 26 Sep 2020 18:43:19 -0700 Subject: [PATCH 6/9] Skip dropck::check_drop_impl in is_const_item_without_destructor adt_destructor by default also validates the Drop impl using dropck::check_drop_impl, which contains an expect_local(). This leads to ICE in check_const_item_mutation if the const's type is not a local type. thread 'rustc' panicked at 'DefId::expect_local: `DefId(5:4805 ~ alloc[d7e9]::vec::{impl#50})` isn't local', compiler/rustc_span/src/def_id.rs:174:43 stack backtrace: 0: rust_begin_unwind 1: rustc_span::def_id::DefId::expect_local::{{closure}} 2: rustc_typeck::check::dropck::check_drop_impl 3: rustc_middle::ty::util::::calculate_dtor::{{closure}} 4: rustc_middle::ty::trait_def::::for_each_relevant_impl 5: rustc_middle::ty::util::::calculate_dtor 6: rustc_typeck::check::adt_destructor 7: rustc_middle::ty::query:: for rustc_middle::ty::query::queries::adt_destructor>::compute 8: rustc_query_system::dep_graph::graph::DepGraph::with_task_impl 9: rustc_query_system::query::plumbing::get_query_impl 10: rustc_mir::transform::check_const_item_mutation::ConstMutationChecker::is_const_item_without_destructor --- compiler/rustc_mir/src/transform/check_const_item_mutation.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 112c25a312930..98477b032372e 100644 --- a/compiler/rustc_mir/src/transform/check_const_item_mutation.rs +++ b/compiler/rustc_mir/src/transform/check_const_item_mutation.rs @@ -34,6 +34,7 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> { fn is_const_item_without_destructor(&self, local: Local) -> Option { let def_id = self.is_const_item(local)?; + let mut any_dtor = |_tcx, _def_id| Ok(()); // We avoid linting mutation of a const item if the const's type has a // Drop impl. The Drop logic observes the mutation which was performed. @@ -46,7 +47,7 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> { // // LOG.msg = "wow"; // prints "wow" // - match self.tcx.adt_destructor(def_id) { + match self.tcx.calculate_dtor(def_id, &mut any_dtor) { Some(_) => None, None => Some(def_id), } From eef5104c53114bf9a074fe8d83cfd782bd78652d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 30 Sep 2020 22:13:33 -0700 Subject: [PATCH 7/9] Add test of VEC.push on a const --- src/test/ui/lint/lint-const-item-mutation.rs | 2 ++ src/test/ui/lint/lint-const-item-mutation.stderr | 14 +++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/test/ui/lint/lint-const-item-mutation.rs b/src/test/ui/lint/lint-const-item-mutation.rs index 37e6420fc9832..9f6a4e5908858 100644 --- a/src/test/ui/lint/lint-const-item-mutation.rs +++ b/src/test/ui/lint/lint-const-item-mutation.rs @@ -28,6 +28,7 @@ const MY_STRUCT: MyStruct = MyStruct { field: true, inner_array: ['a'], raw_ptr: 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(); fn main() { ARRAY[0] = 5; //~ WARN attempting to modify @@ -48,4 +49,5 @@ fn main() { MUTABLE.msg = "wow"; // no warning, because Drop observes the mutation MUTABLE2.msg = "wow"; //~ WARN attempting to modify + VEC.push(0); // no warning } diff --git a/src/test/ui/lint/lint-const-item-mutation.stderr b/src/test/ui/lint/lint-const-item-mutation.stderr index 20a8ed0d78386..0e02ba928bb35 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:33:5 + --> $DIR/lint-const-item-mutation.rs:34: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:34:5 + --> $DIR/lint-const-item-mutation.rs:35: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:35:5 + --> $DIR/lint-const-item-mutation.rs:36: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:36:5 + --> $DIR/lint-const-item-mutation.rs:37: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:37:5 + --> $DIR/lint-const-item-mutation.rs:38: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:38:5 + --> $DIR/lint-const-item-mutation.rs:39: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:50:5 + --> $DIR/lint-const-item-mutation.rs:51:5 | LL | MUTABLE2.msg = "wow"; | ^^^^^^^^^^^^^^^^^^^^ From 75c2fdf34a5384627db9ba240c5dcc0723aea763 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 30 Sep 2020 22:16:32 -0700 Subject: [PATCH 8/9] Warn on method call mutating const, even if it has destructor --- .../transform/check_const_item_mutation.rs | 2 +- src/test/ui/lint/lint-const-item-mutation.rs | 2 +- .../ui/lint/lint-const-item-mutation.stderr | 27 ++++++++++++++++++- 3 files changed, 28 insertions(+), 3 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 98477b032372e..a0edb3772439a 100644 --- a/compiler/rustc_mir/src/transform/check_const_item_mutation.rs +++ b/compiler/rustc_mir/src/transform/check_const_item_mutation.rs @@ -111,7 +111,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ConstMutationChecker<'a, 'tcx> { fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, loc: Location) { if let Rvalue::Ref(_, BorrowKind::Mut { .. }, place) = rvalue { let local = place.local; - if let Some(def_id) = self.is_const_item_without_destructor(local) { + if let Some(def_id) = self.is_const_item(local) { // If this Rvalue is being used as the right-hand side of a // `StatementKind::Assign`, see if it ends up getting used as // the `self` parameter of a method call (as the terminator of our current diff --git a/src/test/ui/lint/lint-const-item-mutation.rs b/src/test/ui/lint/lint-const-item-mutation.rs index 9f6a4e5908858..c49a13f1065b5 100644 --- a/src/test/ui/lint/lint-const-item-mutation.rs +++ b/src/test/ui/lint/lint-const-item-mutation.rs @@ -49,5 +49,5 @@ fn main() { MUTABLE.msg = "wow"; // no warning, because Drop observes the mutation MUTABLE2.msg = "wow"; //~ WARN attempting to modify - VEC.push(0); // no warning + VEC.push(0); //~ WARN taking a mutable reference to a `const` item } diff --git a/src/test/ui/lint/lint-const-item-mutation.stderr b/src/test/ui/lint/lint-const-item-mutation.stderr index 0e02ba928bb35..11b5124b2d26a 100644 --- a/src/test/ui/lint/lint-const-item-mutation.stderr +++ b/src/test/ui/lint/lint-const-item-mutation.stderr @@ -98,5 +98,30 @@ note: `const` item defined here LL | const MUTABLE2: Mutable2 = Mutable2 { msg: "", other: String::new() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -warning: 7 warnings emitted +warning: taking a mutable reference to a `const` item + --> $DIR/lint-const-item-mutation.rs:52:5 + | +LL | VEC.push(0); + | ^^^^^^^^^^^ + | + = note: each usage of a `const` item creates a new temporary + = note: the mutable reference will refer to this temporary, not the original `const` item +note: mutable reference created due to call to this method + --> $SRC_DIR/alloc/src/vec.rs:LL:COL + | +LL | / pub fn push(&mut self, value: T) { +LL | | // This will panic or abort if we would allocate > isize::MAX bytes +LL | | // or if the length increment would overflow for zero-sized types. +LL | | if self.len == self.buf.capacity() { +... | +LL | | } +LL | | } + | |_____^ +note: `const` item defined here + --> $DIR/lint-const-item-mutation.rs:31:1 + | +LL | const VEC: Vec = Vec::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: 8 warnings emitted From 804d159c6276025e26ade90a6ef013bda33207c3 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 1 Oct 2020 13:41:29 -0700 Subject: [PATCH 9/9] Fixme with link for re-enabling const mutation lint for Drop consts --- .../src/transform/check_const_item_mutation.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 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 a0edb3772439a..b6d57b899ddab 100644 --- a/compiler/rustc_mir/src/transform/check_const_item_mutation.rs +++ b/compiler/rustc_mir/src/transform/check_const_item_mutation.rs @@ -39,14 +39,21 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> { // We avoid linting mutation of a const item if the const's type has a // Drop impl. The Drop logic observes the mutation which was performed. // - // struct Log { msg: &'static str } - // const LOG: Log = Log { msg: "" }; + // pub struct Log { msg: &'static str } + // pub const LOG: Log = Log { msg: "" }; // impl Drop for Log { // fn drop(&mut self) { println!("{}", self.msg); } // } // // LOG.msg = "wow"; // prints "wow" // + // FIXME(https://github.com/rust-lang/rust/issues/77425): + // Drop this exception once there is a stable attribute to suppress the + // const item mutation lint for a single specific const only. Something + // equivalent to: + // + // #[const_mutation_allowed] + // pub const LOG: Log = Log { msg: "" }; match self.tcx.calculate_dtor(def_id, &mut any_dtor) { Some(_) => None, None => Some(def_id),