Skip to content

Commit 372bf9d

Browse files
authored
Unrolled build for #148407
Rollup merge of #148407 - Urgau:suspicious_int_mutable_consts, r=JonathanBrouwer Warn against calls which mutate an interior mutable `const`-item ## `const_item_interior_mutations` ~~`interior_mutable_const_item_mutations`~~ ~~`suspicious_mutation_of_interior_mutable_consts`~~ *warn-by-default* The `const_item_interior_mutations` 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"); }); } ``` ```text warning: mutation of an interior mutable `const` item with call to `call_once` --> a.rs:11:5 | 11 | INIT.call_once(|| { | ^--- | | | _____`INIT` is a interior mutable `const` item of type `std::sync::Once` | | 12 | | println!("Once::call_once first call"); 13 | | }); | |______^ | = note: each usage of a `const` item creates a new temporary = note: only the temporaries and never the original `const INIT` will be modified = help: for more details on interior mutability see <https://doc.rust-lang.org/reference/interior-mutability.html> = note: `#[warn(const_item_interior_mutations)]` on by default help: for a shared instance of `INIT`, consider making it a `static` item instead | 6 - const INIT: Once = Once::new(); // using `INIT` will always create a temporary and 6 + static INIT: Once = Once::new(); // using `INIT` will always create a temporary and | ``` ### 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 and may be extended in the future. ---- This PR is an targeted alternative to #132146. It avoids false-positives by adding an internal-only attribute `#[rustc_should_not_be_called_on_const_items]` on methods and functions that mutates an interior mutale type through a shared reference (mutable refrences are already linted by the `const_item_mutation` lint). It should also be noted that this is NOT an uplift of the more general [`clippy::borrow_interior_mutable_const`](https://rust-lang.github.io/rust-clippy/master/index.html#/borrow_interior_mutable_const) lint, which is a much more general lint regarding borrow of interior mutable types, but has false-positives that are completly avoided by this lint. A simple [GitHub Search](https://github.com/search?q=lang%3Arust+%2F%28%3F-i%29const+%5Ba-zA-Z0-9_%5D*%3A+Once%2F&type=code) reveals many instance where the user probably wanted to use a `static`-item instead. ---- ````@rustbot```` labels +I-lang-nominated +T-lang cc ````@traviscross```` r? compiler Fixes [IRLO - Forbidding creation of constant mutexes, etc](https://internals.rust-lang.org/t/forbidding-creation-of-constant-mutexes-etc/19005) Fixes #132028 Fixes #40543
2 parents 4b1b6dd + c48dce1 commit 372bf9d

32 files changed

+2686
-30
lines changed

compiler/rustc_attr_parsing/src/attributes/lint_helpers.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,17 @@ impl<S: Stage> NoArgsAttributeParser<S> for PassByValueParser {
3838
const CREATE: fn(Span) -> AttributeKind = AttributeKind::PassByValue;
3939
}
4040

41+
pub(crate) struct RustcShouldNotBeCalledOnConstItems;
42+
impl<S: Stage> NoArgsAttributeParser<S> for RustcShouldNotBeCalledOnConstItems {
43+
const PATH: &[Symbol] = &[sym::rustc_should_not_be_called_on_const_items];
44+
const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Error;
45+
const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(&[
46+
Allow(Target::Method(MethodKind::Inherent)),
47+
Allow(Target::Method(MethodKind::TraitImpl)),
48+
]);
49+
const CREATE: fn(Span) -> AttributeKind = AttributeKind::RustcShouldNotBeCalledOnConstItems;
50+
}
51+
4152
pub(crate) struct AutomaticallyDerivedParser;
4253
impl<S: Stage> NoArgsAttributeParser<S> for AutomaticallyDerivedParser {
4354
const PATH: &[Symbol] = &[sym::automatically_derived];

compiler/rustc_attr_parsing/src/context.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use crate::attributes::link_attrs::{
3939
};
4040
use crate::attributes::lint_helpers::{
4141
AsPtrParser, AutomaticallyDerivedParser, PassByValueParser, PubTransparentParser,
42+
RustcShouldNotBeCalledOnConstItems,
4243
};
4344
use crate::attributes::loop_match::{ConstContinueParser, LoopMatchParser};
4445
use crate::attributes::macro_attrs::{
@@ -244,6 +245,7 @@ attribute_parsers!(
244245
Single<WithoutArgs<RustcCoherenceIsCoreParser>>,
245246
Single<WithoutArgs<RustcMainParser>>,
246247
Single<WithoutArgs<RustcPassIndirectlyInNonRusticAbisParser>>,
248+
Single<WithoutArgs<RustcShouldNotBeCalledOnConstItems>>,
247249
Single<WithoutArgs<SpecializationTraitParser>>,
248250
Single<WithoutArgs<StdInternalSymbolParser>>,
249251
Single<WithoutArgs<TrackCallerParser>>,

compiler/rustc_feature/src/builtin_attrs.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,6 +1267,11 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
12671267
EncodeCrossCrate::Yes,
12681268
"`#[rustc_as_ptr]` is used to mark functions returning pointers to their inner allocations."
12691269
),
1270+
rustc_attr!(
1271+
rustc_should_not_be_called_on_const_items, Normal, template!(Word), ErrorFollowing,
1272+
EncodeCrossCrate::Yes,
1273+
"`#[rustc_should_not_be_called_on_const_items]` is used to mark methods that don't make sense to be called on interior mutable consts."
1274+
),
12701275
rustc_attr!(
12711276
rustc_pass_by_value, Normal, template!(Word), ErrorFollowing,
12721277
EncodeCrossCrate::Yes,

compiler/rustc_hir/src/attrs/data_structures.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,9 @@ pub enum AttributeKind {
701701
/// Represents `#[rustc_pass_indirectly_in_non_rustic_abis]`
702702
RustcPassIndirectlyInNonRusticAbis(Span),
703703

704+
/// Represents `#[rustc_should_not_be_called_on_const_items]`
705+
RustcShouldNotBeCalledOnConstItems(Span),
706+
704707
/// Represents `#[rustc_simd_monomorphize_lane_limit = "N"]`.
705708
RustcSimdMonomorphizeLaneLimit(Limit),
706709

compiler/rustc_hir/src/attrs/encode_cross_crate.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ impl AttributeKind {
9191
RustcMain => No,
9292
RustcObjectLifetimeDefault => No,
9393
RustcPassIndirectlyInNonRusticAbis(..) => No,
94+
RustcShouldNotBeCalledOnConstItems(..) => Yes,
9495
RustcSimdMonomorphizeLaneLimit(..) => Yes, // Affects layout computation, which needs to work cross-crate
9596
Sanitize { .. } => No,
9697
ShouldPanic { .. } => No,

compiler/rustc_lint/messages.ftl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,14 @@ lint_confusable_identifier_pair = found both `{$existing_sym}` and `{$sym}` as i
193193
.current_use = this identifier can be confused with `{$existing_sym}`
194194
.other_use = other identifier used here
195195
196+
lint_const_item_interior_mutations =
197+
mutation of an interior mutable `const` item with call to `{$method_name}`
198+
.label = `{$const_name}` is a interior mutable `const` item of type `{$const_ty}`
199+
.temporary = each usage of a `const` item creates a new temporary
200+
.never_original = only the temporaries and never the original `const {$const_name}` will be modified
201+
.suggestion_static = for a shared instance of `{$const_name}`, consider making it a `static` item instead
202+
.help = for more details on interior mutability see <https://doc.rust-lang.org/reference/interior-mutability.html>
203+
196204
lint_dangling_pointers_from_locals = {$fn_kind} returns a dangling pointer to dropped local variable `{$local_var_name}`
197205
.ret_ty = return type is `{$ret_ty}`
198206
.local_var = local variable `{$local_var_name}` is dropped at the end of the {$fn_kind}
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
use rustc_hir::attrs::AttributeKind;
2+
use rustc_hir::def::{DefKind, Res};
3+
use rustc_hir::{Expr, ExprKind, ItemKind, Node, find_attr};
4+
use rustc_session::{declare_lint, declare_lint_pass};
5+
6+
use crate::lints::{ConstItemInteriorMutationsDiag, ConstItemInteriorMutationsSuggestionStatic};
7+
use crate::{LateContext, LateLintPass, LintContext};
8+
9+
declare_lint! {
10+
/// The `const_item_interior_mutations` lint checks for calls which
11+
/// mutates an interior mutable const-item.
12+
///
13+
/// ### Example
14+
///
15+
/// ```rust
16+
/// use std::sync::Once;
17+
///
18+
/// const INIT: Once = Once::new(); // using `INIT` will always create a temporary and
19+
/// // never modify it-self on use, should be a `static`
20+
/// // instead for shared use
21+
///
22+
/// fn init() {
23+
/// INIT.call_once(|| {
24+
/// println!("Once::call_once first call");
25+
/// });
26+
/// INIT.call_once(|| { // this second will also print
27+
/// println!("Once::call_once second call"); // as each call to `INIT` creates
28+
/// }); // new temporary
29+
/// }
30+
/// ```
31+
///
32+
/// {{produces}}
33+
///
34+
/// ### Explanation
35+
///
36+
/// Calling a method which mutates an interior mutable type has no effect as const-item
37+
/// are essentially inlined wherever they are used, meaning that they are copied
38+
/// directly into the relevant context when used rendering modification through
39+
/// interior mutability ineffective across usage of that const-item.
40+
///
41+
/// The current implementation of this lint only warns on significant `std` and
42+
/// `core` interior mutable types, like `Once`, `AtomicI32`, ... this is done out
43+
/// of prudence to avoid false-positive and may be extended in the future.
44+
pub CONST_ITEM_INTERIOR_MUTATIONS,
45+
Warn,
46+
"checks for calls which mutates a interior mutable const-item"
47+
}
48+
49+
declare_lint_pass!(InteriorMutableConsts => [CONST_ITEM_INTERIOR_MUTATIONS]);
50+
51+
impl<'tcx> LateLintPass<'tcx> for InteriorMutableConsts {
52+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
53+
let typeck = cx.typeck_results();
54+
55+
let (method_did, receiver) = match expr.kind {
56+
// matching on `<receiver>.method(..)`
57+
ExprKind::MethodCall(_, receiver, _, _) => {
58+
(typeck.type_dependent_def_id(expr.hir_id), receiver)
59+
}
60+
// matching on `function(&<receiver>, ...)`
61+
ExprKind::Call(path, [receiver, ..]) => match receiver.kind {
62+
ExprKind::AddrOf(_, _, receiver) => match path.kind {
63+
ExprKind::Path(ref qpath) => {
64+
(cx.qpath_res(qpath, path.hir_id).opt_def_id(), receiver)
65+
}
66+
_ => return,
67+
},
68+
_ => return,
69+
},
70+
_ => return,
71+
};
72+
73+
let Some(method_did) = method_did else {
74+
return;
75+
};
76+
77+
if let ExprKind::Path(qpath) = &receiver.kind
78+
&& let Res::Def(DefKind::Const | DefKind::AssocConst, const_did) =
79+
typeck.qpath_res(qpath, receiver.hir_id)
80+
// Let's do the attribute check after the other checks for perf reasons
81+
&& find_attr!(
82+
cx.tcx.get_all_attrs(method_did),
83+
AttributeKind::RustcShouldNotBeCalledOnConstItems(_)
84+
)
85+
&& let Some(method_name) = cx.tcx.opt_item_ident(method_did)
86+
&& let Some(const_name) = cx.tcx.opt_item_ident(const_did)
87+
&& let Some(const_ty) = typeck.node_type_opt(receiver.hir_id)
88+
{
89+
// Find the local `const`-item and create the suggestion to use `static` instead
90+
let sugg_static = if let Some(Node::Item(const_item)) =
91+
cx.tcx.hir_get_if_local(const_did)
92+
&& let ItemKind::Const(ident, _generics, _ty, _body_id) = const_item.kind
93+
{
94+
if let Some(vis_span) = const_item.vis_span.find_ancestor_inside(const_item.span)
95+
&& const_item.span.can_be_used_for_suggestions()
96+
&& vis_span.can_be_used_for_suggestions()
97+
{
98+
Some(ConstItemInteriorMutationsSuggestionStatic::Spanful {
99+
const_: const_item.vis_span.between(ident.span),
100+
before: if !vis_span.is_empty() { " " } else { "" },
101+
})
102+
} else {
103+
Some(ConstItemInteriorMutationsSuggestionStatic::Spanless)
104+
}
105+
} else {
106+
None
107+
};
108+
109+
cx.emit_span_lint(
110+
CONST_ITEM_INTERIOR_MUTATIONS,
111+
expr.span,
112+
ConstItemInteriorMutationsDiag {
113+
method_name,
114+
const_name,
115+
const_ty,
116+
receiver_span: receiver.span,
117+
sugg_static,
118+
},
119+
);
120+
}
121+
}
122+
}

compiler/rustc_lint/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ mod foreign_modules;
4848
mod function_cast_as_integer;
4949
mod if_let_rescope;
5050
mod impl_trait_overcaptures;
51+
mod interior_mutable_consts;
5152
mod internal;
5253
mod invalid_from_utf8;
5354
mod late;
@@ -93,6 +94,7 @@ use for_loops_over_fallibles::*;
9394
use function_cast_as_integer::*;
9495
use if_let_rescope::IfLetRescope;
9596
use impl_trait_overcaptures::ImplTraitOvercaptures;
97+
use interior_mutable_consts::*;
9698
use internal::*;
9799
use invalid_from_utf8::*;
98100
use let_underscore::*;
@@ -239,6 +241,7 @@ late_lint_methods!(
239241
AsyncClosureUsage: AsyncClosureUsage,
240242
AsyncFnInTrait: AsyncFnInTrait,
241243
NonLocalDefinitions: NonLocalDefinitions::default(),
244+
InteriorMutableConsts: InteriorMutableConsts,
242245
ImplTraitOvercaptures: ImplTraitOvercaptures,
243246
IfLetRescope: IfLetRescope::default(),
244247
StaticMutRefs: StaticMutRefs,

compiler/rustc_lint/src/lints.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,39 @@ pub(crate) enum InvalidFromUtf8Diag {
784784
},
785785
}
786786

787+
// interior_mutable_consts.rs
788+
#[derive(LintDiagnostic)]
789+
#[diag(lint_const_item_interior_mutations)]
790+
#[note(lint_temporary)]
791+
#[note(lint_never_original)]
792+
#[help]
793+
pub(crate) struct ConstItemInteriorMutationsDiag<'tcx> {
794+
pub method_name: Ident,
795+
pub const_name: Ident,
796+
pub const_ty: Ty<'tcx>,
797+
#[label]
798+
pub receiver_span: Span,
799+
#[subdiagnostic]
800+
pub sugg_static: Option<ConstItemInteriorMutationsSuggestionStatic>,
801+
}
802+
803+
#[derive(Subdiagnostic)]
804+
pub(crate) enum ConstItemInteriorMutationsSuggestionStatic {
805+
#[suggestion(
806+
lint_suggestion_static,
807+
code = "{before}static ",
808+
style = "verbose",
809+
applicability = "maybe-incorrect"
810+
)]
811+
Spanful {
812+
#[primary_span]
813+
const_: Span,
814+
before: &'static str,
815+
},
816+
#[help(lint_suggestion_static)]
817+
Spanless,
818+
}
819+
787820
// reference_casting.rs
788821
#[derive(LintDiagnostic)]
789822
pub(crate) enum InvalidReferenceCastingDiag<'tcx> {

compiler/rustc_passes/src/check_attr.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
254254
| AttributeKind::RustcLayoutScalarValidRangeStart(..)
255255
| AttributeKind::RustcLayoutScalarValidRangeEnd(..)
256256
| AttributeKind::RustcSimdMonomorphizeLaneLimit(..)
257+
| AttributeKind::RustcShouldNotBeCalledOnConstItems(..)
257258
| AttributeKind::ExportStable
258259
| AttributeKind::FfiConst(..)
259260
| AttributeKind::UnstableFeatureBound(..)

0 commit comments

Comments
 (0)