Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bypass const_item_mutation if const's type has Drop impl #77251

Merged
merged 9 commits into from
Oct 3, 2020
31 changes: 30 additions & 1 deletion compiler/rustc_mir/src/transform/check_const_item_mutation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,35 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> {
None
}
}

fn is_const_item_without_destructor(&self, local: Local) -> Option<DefId> {
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you open an issue to track removing this (conditional on having a stable attribute to suppress the lint for a particular const item), and add a FIXME?

// Drop impl. The Drop logic observes the mutation which was performed.
//
// 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),
}
}

fn lint_const_item_usage(
&self,
const_item: DefId,
Expand Down Expand Up @@ -59,7 +88,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)) {
Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/lint/lint-const-item-mutation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,26 @@ impl MyStruct {
fn use_mut(&mut self) {}
}

struct Mutable {
msg: &'static str,
}
impl Drop for Mutable {
fn drop(&mut self) {
println!("{}", self.msg);
}
}

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() };
const VEC: Vec<i32> = Vec::new();

fn main() {
ARRAY[0] = 5; //~ WARN attempting to modify
Expand All @@ -29,4 +46,8 @@ fn main() {
*RAW_PTR = 0;
*MY_STRUCT.raw_ptr = 0;
}

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
}
64 changes: 51 additions & 13 deletions src/test/ui/lint/lint-const-item-mutation.stderr
Original file line number Diff line number Diff line change
@@ -1,45 +1,45 @@
warning: attempting to modify a `const` item
--> $DIR/lint-const-item-mutation.rs:17:5
--> $DIR/lint-const-item-mutation.rs:34:5
|
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:26: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:35: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: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:19:5
--> $DIR/lint-const-item-mutation.rs:36: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: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:20:5
--> $DIR/lint-const-item-mutation.rs:37:5
|
LL | MY_STRUCT.use_mut();
| ^^^^^^^^^^^^^^^^^^^
Expand All @@ -52,38 +52,76 @@ 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: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:21:5
--> $DIR/lint-const-item-mutation.rs:38:5
|
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: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:22:5
--> $DIR/lint-const-item-mutation.rs:39:5
|
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: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:51: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: 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<i32> = Vec::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: 8 warnings emitted