From 940498dad7cb35a753a7cce104ab510b414e84de Mon Sep 17 00:00:00 2001 From: Rachit2323 Date: Mon, 24 Aug 2026 12:25:29 +0530 Subject: [PATCH] fix const_item_mutation lint to use needs_drop instead of has_dtor --- .../src/check_const_item_mutation.rs | 40 ++++++++++------- tests/ui/lint/lint-const-item-mutation.rs | 10 +++-- tests/ui/lint/lint-const-item-mutation.stderr | 43 +++++++------------ 3 files changed, 47 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_mir_transform/src/check_const_item_mutation.rs b/compiler/rustc_mir_transform/src/check_const_item_mutation.rs index 5b25bdc01117b..e8ff3c3a08b79 100644 --- a/compiler/rustc_mir_transform/src/check_const_item_mutation.rs +++ b/compiler/rustc_mir_transform/src/check_const_item_mutation.rs @@ -2,7 +2,7 @@ use rustc_hir::HirId; use rustc_lint_defs::builtin::CONST_ITEM_MUTATION; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::*; -use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt}; use rustc_span::Span; use rustc_span::def_id::DefId; @@ -35,8 +35,9 @@ impl<'tcx> ConstMutationChecker<'_, '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. + // We avoid linting mutation of a const item if the const's type needs + // drop. Any drop logic (including that of fields) may observe the + // mutation which was performed. // // pub struct Log { msg: &'static str } // pub const LOG: Log = Log { msg: "" }; @@ -46,21 +47,30 @@ impl<'tcx> ConstMutationChecker<'_, 'tcx> { // // LOG.msg = "wow"; // prints "wow" // + // Likewise, if a field of the const type has its own Drop impl, that + // drop logic may also observe the mutation: + // + // struct Inner { val: u32 } + // impl Drop for Inner { fn drop(&mut self) { println!("{}", self.val); } } + // struct Outer { inner: Inner } + // const O: Outer = Outer { inner: Inner { val: 0 } }; + // + // O.inner.val = 42; // Inner::drop prints "42" + // // 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: "" }; - // FIXME: this should not be checking for `Drop` impls, - // but whether it or any field has a Drop impl (`needs_drop`) - // as fields' Drop impls may make this observable, too. - match self.tcx.type_of(def_id).skip_binder().ty_adt_def().map(|adt| adt.has_dtor(self.tcx)) - { - Some(true) => None, - Some(false) | None => Some(def_id), + // const item mutation lint for a single specific const only. + let ty = self.tcx.type_of(def_id).instantiate_identity().skip_norm_wip(); + // `needs_drop` is overly conservative for types that contain type + // parameters (e.g. `Self` in a trait associated const): it always + // returns `true` because the parameter *might* implement Drop, even + // when the concrete type at the call site does not. In that case we + // cannot suppress the lint, so fall through and warn. + if ty.has_param() { + return Some(def_id); } + let typing_env = ty::TypingEnv::non_body_analysis(self.tcx, def_id); + if ty.needs_drop(self.tcx, typing_env) { None } else { Some(def_id) } } /// If we should lint on this usage, return the [`HirId`], source [`Span`] diff --git a/tests/ui/lint/lint-const-item-mutation.rs b/tests/ui/lint/lint-const-item-mutation.rs index d51d3c394937c..877455e7bb869 100644 --- a/tests/ui/lint/lint-const-item-mutation.rs +++ b/tests/ui/lint/lint-const-item-mutation.rs @@ -18,16 +18,19 @@ impl Drop for Mutable { } } -struct Mutable2 { // this one has drop glue but not a Drop impl +struct Mutable2 { // this one has drop glue but not a direct Drop impl msg: &'static str, other: String, } +struct WithFieldDrop { inner: Mutable } // no Drop on this type, but Mutable has one + 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 WFD: WithFieldDrop = WithFieldDrop { inner: Mutable { msg: "" } }; const VEC: Vec = Vec::new(); const PTR: *mut () = 1 as *mut _; const PTR_TO_ARRAY: *mut [u32; 4] = 0x12345678 as _; @@ -50,8 +53,9 @@ fn main() { *MY_STRUCT.raw_ptr = 0; } - MUTABLE.msg = "wow"; // no warning, because Drop observes the mutation - MUTABLE2.msg = "wow"; //~ WARN attempting to modify + MUTABLE.msg = "wow"; // no warning — Drop impl observes the mutation + MUTABLE2.msg = "wow"; // no warning — field String has drop glue (needs_drop = true) + WFD.inner.msg = "observed"; // no warning — Mutable's Drop observes the field mutation VEC.push(0); //~ WARN taking a mutable reference to a `const` item // Test that we don't warn when converting a raw pointer diff --git a/tests/ui/lint/lint-const-item-mutation.stderr b/tests/ui/lint/lint-const-item-mutation.stderr index 0e405c306fe46..84f5e78953e60 100644 --- a/tests/ui/lint/lint-const-item-mutation.stderr +++ b/tests/ui/lint/lint-const-item-mutation.stderr @@ -1,45 +1,45 @@ warning: attempting to modify a `const` item - --> $DIR/lint-const-item-mutation.rs:37:5 + --> $DIR/lint-const-item-mutation.rs:40:5 | LL | ARRAY[0] = 5; | ^^^^^^^^^^^^ | = 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:26:1 + --> $DIR/lint-const-item-mutation.rs:28:1 | LL | const ARRAY: [u8; 1] = [25]; | ^^^^^^^^^^^^^^^^^^^^ = note: `#[warn(const_item_mutation)]` on by default warning: attempting to modify a `const` item - --> $DIR/lint-const-item-mutation.rs:38:5 + --> $DIR/lint-const-item-mutation.rs:41: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:27:1 + --> $DIR/lint-const-item-mutation.rs:29: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:39:5 + --> $DIR/lint-const-item-mutation.rs:42: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:27:1 + --> $DIR/lint-const-item-mutation.rs:29: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:40:5 + --> $DIR/lint-const-item-mutation.rs:43: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:27:1 + --> $DIR/lint-const-item-mutation.rs:29: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:41:5 + --> $DIR/lint-const-item-mutation.rs:44: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:27:1 + --> $DIR/lint-const-item-mutation.rs:29: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:42:5 + --> $DIR/lint-const-item-mutation.rs:45:5 | LL | (&mut MY_STRUCT).use_mut(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -85,26 +85,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:27:1 + --> $DIR/lint-const-item-mutation.rs:29: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:54: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:55:5 + --> $DIR/lint-const-item-mutation.rs:59:5 | LL | VEC.push(0); | ^^^^^^^^^^^ @@ -114,10 +101,10 @@ LL | VEC.push(0); note: mutable reference created due to call to this method --> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL note: `const` item defined here - --> $DIR/lint-const-item-mutation.rs:31:1 + --> $DIR/lint-const-item-mutation.rs:34:1 | LL | const VEC: Vec = Vec::new(); | ^^^^^^^^^^^^^^^^^^^ -warning: 8 warnings emitted +warning: 7 warnings emitted