Add suspicious_mutation_of_interior_mutable_consts lint

Author: Urgau

compiler/rustc_lint/messages.ftl

@@ -774,6 +774,13 @@ lint_suspicious_double_ref_clone =
 lint_suspicious_double_ref_deref =
     using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type
 
+lint_suspicious_mutation_of_interior_mutable_consts =
+    suspicious call to `{$method_name}` which mutates an interior mutable `const`
+    .label = `{$const_name}` is a interior mutable `const` of type `{$const_ty}`
+    .temporary = each usage of a `const` creates a new temporary
+    .never_original = only the temporaries and never the original `const {$const_name}` will be modified
+    .suggestion_static = for a shared instance of `{$const_name}`, consider making it a `static` item instead
+
 lint_symbol_intern_string_literal = using `Symbol::intern` on a string literal
     .help = consider adding the symbol to `compiler/rustc_span/src/symbol.rs`
 

compiler/rustc_lint/src/interior_mutable_consts.rs

@@ -0,0 +1,125 @@
+use rustc_hir::attrs::AttributeKind;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Expr, ExprKind, ItemKind, Node, find_attr};
+use rustc_session::{declare_lint, declare_lint_pass};
+
+use crate::lints::{
+    SuspiciousMutationOfInteriorMutableConstsDiag,
+    SuspiciousMutationOfInteriorMutableConstsSuggestionStatic,
+};
+use crate::{LateContext, LateLintPass, LintContext};
+
+declare_lint! {
+    /// The `suspicious_mutation_of_interior_mutable_consts` lint checks for calls which
+    /// mutates an interior mutable const-item.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// use std::sync::Once;
+    ///
+    /// const INIT: Once = Once::new(); // using `INIT` will always create a temporary and
+    ///                                 // never modify it-self on use, should be a `static`
+    ///                                 // instead for shared use
+    ///
+    /// fn init() {
+    ///     INIT.call_once(|| {
+    ///         println!("Once::call_once first call");
+    ///     });
+    ///     INIT.call_once(|| {                          // this second will also print
+    ///         println!("Once::call_once second call"); // as each call to `INIT` creates
+    ///     });                                          // new temporary
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Calling a method which mutates an interior mutable type has no effect as const-item
+    /// are essentially inlined wherever they are used, meaning that they are copied
+    /// directly into the relevant context when used rendering modification through
+    /// interior mutability ineffective across usage of that const-item.
+    ///
+    /// The current implementation of this lint only warns on significant `std` and
+    /// `core` interior mutable types, like `Once`, `AtomicI32`, ... this is done out
+    /// of prudence to avoid false-positive and may be extended in the future.
+    pub SUSPICIOUS_MUTATION_OF_INTERIOR_MUTABLE_CONSTS,
+    Warn,
+    "checks for calls which mutates a interior mutable const-item"
+}
+
+declare_lint_pass!(InteriorMutableConsts => [SUSPICIOUS_MUTATION_OF_INTERIOR_MUTABLE_CONSTS]);
+
+impl<'tcx> LateLintPass<'tcx> for InteriorMutableConsts {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        let typeck = cx.typeck_results();
+
+        let (method_did, receiver) = match expr.kind {
+            // matching on `<receiver>.method(..)`
+            ExprKind::MethodCall(_, receiver, _, _) => {
+                (typeck.type_dependent_def_id(expr.hir_id), receiver)
+            }
+            // matching on `function(&<receiver>, ...)`
+            ExprKind::Call(path, [receiver, ..]) => match receiver.kind {
+                ExprKind::AddrOf(_, _, receiver) => match path.kind {
+                    ExprKind::Path(ref qpath) => {
+                        (cx.qpath_res(qpath, path.hir_id).opt_def_id(), receiver)
+                    }
+                    _ => return,
+                },
+                _ => return,
+            },
+            _ => return,
+        };
+
+        let Some(method_did) = method_did else {
+            return;
+        };
+
+        if let ExprKind::Path(qpath) = &receiver.kind
+            && let Res::Def(DefKind::Const | DefKind::AssocConst, const_did) =
+                typeck.qpath_res(qpath, receiver.hir_id)
+            // Let's do the attribute check after the other checks for perf reasons
+            && find_attr!(
+                cx.tcx.get_all_attrs(method_did),
+                AttributeKind::RustcMustNotCallOnInteriorMutableConsts(_)
+            )
+            && let Some(method_name) = cx.tcx.opt_item_ident(method_did)
+            && let Some(const_name) = cx.tcx.opt_item_ident(const_did)
+            && let Some(const_ty) = typeck.node_type_opt(receiver.hir_id)
+        {
+            // Find the local `const`-item and create the suggestion to use `static` instead
+            let sugg_static = if let Some(Node::Item(const_item)) =
+                cx.tcx.hir_get_if_local(const_did)
+                && let ItemKind::Const(ident, _generics, _ty, _body_id) = const_item.kind
+            {
+                if let Some(vis_span) = const_item.vis_span.find_ancestor_inside(const_item.span)
+                    && const_item.span.can_be_used_for_suggestions()
+                    && vis_span.can_be_used_for_suggestions()
+                {
+                    Some(SuspiciousMutationOfInteriorMutableConstsSuggestionStatic::Spanful {
+                        const_: const_item.vis_span.between(ident.span),
+                        before: if !vis_span.is_empty() { " " } else { "" },
+                    })
+                } else {
+                    Some(SuspiciousMutationOfInteriorMutableConstsSuggestionStatic::Spanless)
+                }
+            } else {
+                None
+            };
+
+            cx.emit_span_lint(
+                SUSPICIOUS_MUTATION_OF_INTERIOR_MUTABLE_CONSTS,
+                expr.span,
+                SuspiciousMutationOfInteriorMutableConstsDiag {
+                    method_name,
+                    const_name,
+                    const_ty,
+                    receiver_span: receiver.span,
+                    sugg_static,
+                },
+            );
+        }
+    }
+}

compiler/rustc_lint/src/lib.rs

@@ -50,6 +50,7 @@ mod for_loops_over_fallibles;
 mod foreign_modules;
 mod if_let_rescope;
 mod impl_trait_overcaptures;
+mod interior_mutable_consts;
 mod internal;
 mod invalid_from_utf8;
 mod late;
@@ -94,6 +95,7 @@ use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
 use for_loops_over_fallibles::*;
 use if_let_rescope::IfLetRescope;
 use impl_trait_overcaptures::ImplTraitOvercaptures;
+use interior_mutable_consts::*;
 use internal::*;
 use invalid_from_utf8::*;
 use let_underscore::*;
@@ -240,6 +242,7 @@ late_lint_methods!(
             AsyncClosureUsage: AsyncClosureUsage,
             AsyncFnInTrait: AsyncFnInTrait,
             NonLocalDefinitions: NonLocalDefinitions::default(),
+            InteriorMutableConsts: InteriorMutableConsts,
             ImplTraitOvercaptures: ImplTraitOvercaptures,
             IfLetRescope: IfLetRescope::default(),
             StaticMutRefs: StaticMutRefs,

compiler/rustc_lint/src/lints.rs

@@ -784,6 +784,38 @@ pub(crate) enum InvalidFromUtf8Diag {
     },
 }
 
+// interior_mutable_consts.rs
+#[derive(LintDiagnostic)]
+#[diag(lint_suspicious_mutation_of_interior_mutable_consts)]
+#[note(lint_temporary)]
+#[note(lint_never_original)]
+pub(crate) struct SuspiciousMutationOfInteriorMutableConstsDiag<'tcx> {
+    pub method_name: Ident,
+    pub const_name: Ident,
+    pub const_ty: Ty<'tcx>,
+    #[label]
+    pub receiver_span: Span,
+    #[subdiagnostic]
+    pub sugg_static: Option<SuspiciousMutationOfInteriorMutableConstsSuggestionStatic>,
+}
+
+#[derive(Subdiagnostic)]
+pub(crate) enum SuspiciousMutationOfInteriorMutableConstsSuggestionStatic {
+    #[suggestion(
+        lint_suggestion_static,
+        code = "{before}static ",
+        style = "verbose",
+        applicability = "maybe-incorrect"
+    )]
+    Spanful {
+        #[primary_span]
+        const_: Span,
+        before: &'static str,
+    },
+    #[help(lint_suggestion_static)]
+    Spanless,
+}
+
 // reference_casting.rs
 #[derive(LintDiagnostic)]
 pub(crate) enum InvalidReferenceCastingDiag<'tcx> {

tests/ui/lint/suspicious-call-int-mutable-const-atomics.fixed

@@ -0,0 +1,173 @@
+//@ check-pass
+//@ run-rustfix
+
+#![allow(deprecated)]
+#![allow(dead_code)]
+#![feature(atomic_try_update)]
+
+use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU32, Ordering};
+
+fn atomic_bool() {
+    static A: AtomicBool = AtomicBool::new(false);
+
+    let _a = A.load(Ordering::SeqCst);
+    //~^ WARN suspicious call to `load`
+
+    let _a = A.store(true, Ordering::SeqCst);
+    //~^ WARN suspicious call to `store`
+
+    let _a = A.swap(true, Ordering::SeqCst);
+    //~^ WARN suspicious call to `swap`
+
+    let _a = A.compare_and_swap(false, true, Ordering::SeqCst);
+    //~^ WARN suspicious call to `compare_and_swap`
+
+    let _a = A.compare_exchange(false, true, Ordering::SeqCst, Ordering::Relaxed);
+    //~^ WARN suspicious call to `compare_exchange`
+
+    let _a = A.compare_exchange_weak(false, true, Ordering::SeqCst, Ordering::Relaxed);
+    //~^ WARN suspicious call to `compare_exchange_weak`
+
+    let _a = A.fetch_and(true, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_and`
+
+    let _a = A.fetch_nand(true, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_nand`
+
+    let _a = A.fetch_or(true, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_or`
+
+    let _a = A.fetch_xor(true, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_xor`
+
+    let _a = A.fetch_not(Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_not`
+
+    let _a = A.fetch_update(Ordering::SeqCst, Ordering::Relaxed, |_| Some(true));
+    //~^ WARN suspicious call to `fetch_update`
+
+    let _a = A.try_update(Ordering::SeqCst, Ordering::Relaxed, |_| Some(false));
+    //~^ WARN suspicious call to `try_update`
+
+    let _a = A.update(Ordering::SeqCst, Ordering::Relaxed, |_| true);
+    //~^ WARN suspicious call to `update`
+}
+
+fn atomic_ptr() {
+    static A: AtomicPtr<i32> = AtomicPtr::new(std::ptr::null_mut());
+
+    let _a = A.load(Ordering::SeqCst);
+    //~^ WARN suspicious call to `load`
+
+    let _a = A.store(std::ptr::null_mut(), Ordering::SeqCst);
+    //~^ WARN suspicious call to `store`
+
+    let _a = A.swap(std::ptr::null_mut(), Ordering::SeqCst);
+    //~^ WARN suspicious call to `swap`
+
+    let _a = A.compare_and_swap(std::ptr::null_mut(), std::ptr::null_mut(), Ordering::SeqCst);
+    //~^ WARN suspicious call to `compare_and_swap`
+
+    let _a = A.compare_exchange(
+        //~^ WARN suspicious call to `compare_exchange`
+        std::ptr::null_mut(),
+        std::ptr::null_mut(),
+        Ordering::SeqCst,
+        Ordering::Relaxed,
+    );
+
+    let _a = A.compare_exchange_weak(
+        //~^ WARN suspicious call to `compare_exchange_weak`
+        std::ptr::null_mut(),
+        std::ptr::null_mut(),
+        Ordering::SeqCst,
+        Ordering::Relaxed,
+    );
+
+    let _a = A.fetch_update(Ordering::SeqCst, Ordering::Relaxed, |_| Some(std::ptr::null_mut()));
+    //~^ WARN suspicious call to `fetch_update`
+
+    let _a = A.try_update(Ordering::SeqCst, Ordering::Relaxed, |_| Some(std::ptr::null_mut()));
+    //~^ WARN suspicious call to `try_update`
+
+    let _a = A.update(Ordering::SeqCst, Ordering::Relaxed, |_| std::ptr::null_mut());
+    //~^ WARN suspicious call to `update`
+
+    let _a = A.fetch_ptr_add(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_ptr_add`
+
+    let _a = A.fetch_ptr_sub(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_ptr_sub`
+
+    let _a = A.fetch_byte_add(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_byte_add`
+
+    let _a = A.fetch_byte_sub(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_byte_sub`
+
+    let _a = A.fetch_and(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_and`
+
+    let _a = A.fetch_or(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_or`
+
+    let _a = A.fetch_xor(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_xor`
+}
+
+fn atomic_u32() {
+    static A: AtomicU32 = AtomicU32::new(0);
+
+    let _a = A.load(Ordering::SeqCst);
+    //~^ WARN suspicious call to `load`
+
+    let _a = A.store(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `store`
+
+    let _a = A.swap(2, Ordering::SeqCst);
+    //~^ WARN suspicious call to `swap`
+
+    let _a = A.compare_and_swap(2, 3, Ordering::SeqCst);
+    //~^ WARN suspicious call to `compare_and_swap`
+
+    let _a = A.compare_exchange(3, 4, Ordering::SeqCst, Ordering::Relaxed);
+    //~^ WARN suspicious call to `compare_exchange`
+
+    let _a = A.compare_exchange_weak(4, 5, Ordering::SeqCst, Ordering::Relaxed);
+    //~^ WARN suspicious call to `compare_exchange_weak`
+
+    let _a = A.fetch_add(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_add`
+
+    let _a = A.fetch_sub(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_sub`
+
+    let _a = A.fetch_add(2, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_add`
+
+    let _a = A.fetch_nand(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_nand`
+
+    let _a = A.fetch_or(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_or`
+
+    let _a = A.fetch_xor(1, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_xor`
+
+    let _a = A.fetch_update(Ordering::SeqCst, Ordering::Relaxed, |_| Some(10));
+    //~^ WARN suspicious call to `fetch_update`
+
+    let _a = A.try_update(Ordering::SeqCst, Ordering::Relaxed, |_| Some(11));
+    //~^ WARN suspicious call to `try_update`
+
+    let _a = A.update(Ordering::SeqCst, Ordering::Relaxed, |_| 12);
+    //~^ WARN suspicious call to `update`
+
+    let _a = A.fetch_max(20, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_max`
+
+    let _a = A.fetch_min(5, Ordering::SeqCst);
+    //~^ WARN suspicious call to `fetch_min`
+}
+
+fn main() {}