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 b6d57b899ddab..5fd57f471868e 100644 --- a/compiler/rustc_mir/src/transform/check_const_item_mutation.rs +++ b/compiler/rustc_mir/src/transform/check_const_item_mutation.rs @@ -4,7 +4,9 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::*; use rustc_middle::ty::TyCtxt; use rustc_session::lint::builtin::CONST_ITEM_MUTATION; +use rustc_session::lint::Level; use rustc_span::def_id::DefId; +use rustc_span::sym; use crate::transform::{MirPass, MirSource}; @@ -32,31 +34,38 @@ 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. - // - // 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), + // In addition to considering lint levels inside of Body as normal, we also + // look at whether the *definition* of the const being mutated is annotated + // with #[allow(const_item_mutation)]. + fn is_suppressed_at_definition(&self, const_item: DefId) -> bool { + if let Some(local_const_item) = const_item.as_local() { + let const_item_hir_id = self.tcx.hir().local_def_id_to_hir_id(local_const_item); + if let Some(lint_level_at_definition) = self + .tcx + .lint_levels(const_item.krate) + .level_and_source(CONST_ITEM_MUTATION, const_item_hir_id, self.tcx.sess) + { + let (level, _lint_source) = lint_level_at_definition; + level == Level::Allow + } else { + false + } + } else { + let attrs = self.tcx.get_attrs(const_item); + for attr in attrs { + if attr.name_or_empty() != sym::allow { + continue; + } + for meta in attr.meta_item_list().unwrap_or_else(Vec::new) { + if meta + .ident() + .map_or(false, |ident| ident.as_str() == CONST_ITEM_MUTATION.name_lower()) + { + return true; + } + } + } + false } } @@ -66,6 +75,10 @@ impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> { location: Location, decorate: impl for<'b> FnOnce(LintDiagnosticBuilder<'b>) -> DiagnosticBuilder<'b>, ) { + if self.is_suppressed_at_definition(const_item) { + return; + } + let source_info = self.body.source_info(location); let lint_root = self.body.source_scopes[source_info.scope] .local_data @@ -88,7 +101,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_without_destructor(lhs.local) { + if let Some(def_id) = self.is_const_item(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)) { diff --git a/src/test/ui/lint/lint-const-item-mutation.rs b/src/test/ui/lint/lint-const-item-mutation.rs index c49a13f1065b5..bb441573e3699 100644 --- a/src/test/ui/lint/lint-const-item-mutation.rs +++ b/src/test/ui/lint/lint-const-item-mutation.rs @@ -47,7 +47,7 @@ fn main() { *MY_STRUCT.raw_ptr = 0; } - MUTABLE.msg = "wow"; // no warning, because Drop observes the mutation + MUTABLE.msg = "wow"; //~ WARN attempting to modify MUTABLE2.msg = "wow"; //~ WARN attempting to modify 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 11b5124b2d26a..4cfd09df5b49e 100644 --- a/src/test/ui/lint/lint-const-item-mutation.stderr +++ b/src/test/ui/lint/lint-const-item-mutation.stderr @@ -85,6 +85,19 @@ note: `const` item defined here 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:50:5 + | +LL | MUTABLE.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:29:1 + | +LL | const MUTABLE: Mutable = Mutable { msg: "" }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + warning: attempting to modify a `const` item --> $DIR/lint-const-item-mutation.rs:51:5 | @@ -123,5 +136,5 @@ note: `const` item defined here LL | const VEC: Vec = Vec::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -warning: 8 warnings emitted +warning: 9 warnings emitted diff --git a/src/test/ui/lint/sneaky-const-mutation.rs b/src/test/ui/lint/sneaky-const-mutation.rs new file mode 100644 index 0000000000000..e95ab6fa7bf68 --- /dev/null +++ b/src/test/ui/lint/sneaky-const-mutation.rs @@ -0,0 +1,83 @@ +// run-pass +// edition:2018 + +#![feature(once_cell)] +#![deny(const_item_mutation)] + +mod library { + use std::lazy::SyncLazy; + use std::ops::{Deref, DerefMut}; + use std::ptr; + use std::sync::atomic::{AtomicPtr, Ordering}; + use std::sync::Mutex; + + pub struct Options { + pub include_prefix: &'static str, + } + + static INCLUDE_PREFIX: SyncLazy> = SyncLazy::new(Mutex::default); + + impl Options { + fn current() -> Self { + Options { + include_prefix: *INCLUDE_PREFIX.lock().unwrap(), + } + } + } + + pub struct Cfg { + options: AtomicPtr, + } + + #[allow(const_item_mutation)] + pub const CFG: Cfg = Cfg { + options: AtomicPtr::new(ptr::null_mut()), + }; + + impl Deref for Cfg { + type Target = Options; + fn deref(&self) -> &Self::Target { + let options = Box::into_raw(Box::new(Options::current())); + let prev = self + .options + .compare_and_swap(ptr::null_mut(), options, Ordering::Relaxed); + if !prev.is_null() { + // compare_and_swap did nothing. + let _ = unsafe { Box::from_raw(options) }; + panic!(); + } + unsafe { &*options } + } + } + + impl DerefMut for Cfg { + fn deref_mut(&mut self) -> &mut Self::Target { + let options = self.options.get_mut(); + if !options.is_null() { + panic!(); + } + *options = Box::into_raw(Box::new(Options::current())); + unsafe { &mut **options } + } + } + + impl Drop for Cfg { + fn drop(&mut self) { + let options = *self.options.get_mut(); + if let Some(options) = unsafe { options.as_mut() } { + *INCLUDE_PREFIX.lock().unwrap() = options.include_prefix; + let _ = unsafe { Box::from_raw(options) }; + } + } + } +} + +use library::CFG; + +fn main() { + assert_eq!(CFG.include_prefix, ""); + + CFG.include_prefix = "path/to"; + + assert_eq!(CFG.include_prefix, "path/to"); +}