1 //! Checks for uses of const which the type is not `Freeze` (`Cell`-free).
3 //! This lint is **warn** by default.
7 use clippy_utils
::diagnostics
::span_lint_and_then
;
8 use clippy_utils
::in_constant
;
9 use if_chain
::if_chain
;
10 use rustc_hir
::def
::{DefKind, Res}
;
11 use rustc_hir
::def_id
::DefId
;
13 BodyId
, Expr
, ExprKind
, HirId
, Impl
, ImplItem
, ImplItemKind
, Item
, ItemKind
, Node
, TraitItem
, TraitItemKind
, UnOp
,
15 use rustc_lint
::{LateContext, LateLintPass, Lint}
;
16 use rustc_middle
::mir
::interpret
::{ConstValue, ErrorHandled}
;
17 use rustc_middle
::ty
::adjustment
::Adjust
;
18 use rustc_middle
::ty
::{self, Const, Ty}
;
19 use rustc_session
::{declare_lint_pass, declare_tool_lint}
;
20 use rustc_span
::{InnerSpan, Span, DUMMY_SP}
;
21 use rustc_typeck
::hir_ty_to_ty
;
23 // FIXME: this is a correctness problem but there's no suitable
24 // warn-by-default category.
25 declare_clippy_lint
! {
27 /// Checks for declaration of `const` items which is interior
28 /// mutable (e.g., contains a `Cell`, `Mutex`, `AtomicXxxx`, etc.).
30 /// ### Why is this bad?
31 /// Consts are copied everywhere they are referenced, i.e.,
32 /// every time you refer to the const a fresh instance of the `Cell` or `Mutex`
33 /// or `AtomicXxxx` will be created, which defeats the whole purpose of using
34 /// these types in the first place.
36 /// The `const` should better be replaced by a `static` item if a global
37 /// variable is wanted, or replaced by a `const fn` if a constructor is wanted.
39 /// ### Known problems
40 /// A "non-constant" const item is a legacy way to supply an
41 /// initialized value to downstream `static` items (e.g., the
42 /// `std::sync::ONCE_INIT` constant). In this case the use of `const` is legit,
43 /// and this lint should be suppressed.
45 /// Even though the lint avoids triggering on a constant whose type has enums that have variants
46 /// with interior mutability, and its value uses non interior mutable variants (see
47 /// [#3962](https://github.com/rust-lang/rust-clippy/issues/3962) and
48 /// [#3825](https://github.com/rust-lang/rust-clippy/issues/3825) for examples);
49 /// it complains about associated constants without default values only based on its types;
50 /// which might not be preferable.
51 /// There're other enums plus associated constants cases that the lint cannot handle.
53 /// Types that have underlying or potential interior mutability trigger the lint whether
54 /// the interior mutable field is used or not. See issues
55 /// [#5812](https://github.com/rust-lang/rust-clippy/issues/5812) and
59 /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
62 /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12);
63 /// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged
64 /// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct
67 /// static STATIC_ATOM: AtomicUsize = AtomicUsize::new(15);
68 /// STATIC_ATOM.store(9, SeqCst);
69 /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
71 #[clippy::version = "pre 1.29.0"]
72 pub DECLARE_INTERIOR_MUTABLE_CONST
,
74 "declaring `const` with interior mutability"
77 // FIXME: this is a correctness problem but there's no suitable
78 // warn-by-default category.
79 declare_clippy_lint
! {
81 /// Checks if `const` items which is interior mutable (e.g.,
82 /// contains a `Cell`, `Mutex`, `AtomicXxxx`, etc.) has been borrowed directly.
84 /// ### Why is this bad?
85 /// Consts are copied everywhere they are referenced, i.e.,
86 /// every time you refer to the const a fresh instance of the `Cell` or `Mutex`
87 /// or `AtomicXxxx` will be created, which defeats the whole purpose of using
88 /// these types in the first place.
90 /// The `const` value should be stored inside a `static` item.
92 /// ### Known problems
93 /// When an enum has variants with interior mutability, use of its non
94 /// interior mutable variants can generate false positives. See issue
95 /// [#3962](https://github.com/rust-lang/rust-clippy/issues/3962)
97 /// Types that have underlying or potential interior mutability trigger the lint whether
98 /// the interior mutable field is used or not. See issues
99 /// [#5812](https://github.com/rust-lang/rust-clippy/issues/5812) and
100 /// [#3825](https://github.com/rust-lang/rust-clippy/issues/3825)
104 /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
105 /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12);
108 /// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged
109 /// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct
112 /// static STATIC_ATOM: AtomicUsize = CONST_ATOM;
113 /// STATIC_ATOM.store(9, SeqCst);
114 /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
116 #[clippy::version = "pre 1.29.0"]
117 pub BORROW_INTERIOR_MUTABLE_CONST
,
119 "referencing `const` with interior mutability"
122 fn is_unfrozen
<'tcx
>(cx
: &LateContext
<'tcx
>, ty
: Ty
<'tcx
>) -> bool
{
123 // Ignore types whose layout is unknown since `is_freeze` reports every generic types as `!Freeze`,
124 // making it indistinguishable from `UnsafeCell`. i.e. it isn't a tool to prove a type is
125 // 'unfrozen'. However, this code causes a false negative in which
126 // a type contains a layout-unknown type, but also an unsafe cell like `const CELL: Cell<T>`.
127 // Yet, it's better than `ty.has_type_flags(TypeFlags::HAS_TY_PARAM | TypeFlags::HAS_PROJECTION)`
128 // since it works when a pointer indirection involves (`Cell<*const T>`).
129 // Making up a `ParamEnv` where every generic params and assoc types are `Freeze`is another option;
130 // but I'm not sure whether it's a decent way, if possible.
131 cx
.tcx
.layout_of(cx
.param_env
.and(ty
)).is_ok() && !ty
.is_freeze(cx
.tcx
.at(DUMMY_SP
), cx
.param_env
)
134 fn is_value_unfrozen_raw
<'tcx
>(
135 cx
: &LateContext
<'tcx
>,
136 result
: Result
<ConstValue
<'tcx
>, ErrorHandled
>,
139 fn inner
<'tcx
>(cx
: &LateContext
<'tcx
>, val
: Const
<'tcx
>) -> bool
{
140 match val
.ty().kind() {
141 // the fact that we have to dig into every structs to search enums
142 // leads us to the point checking `UnsafeCell` directly is the only option.
143 ty
::Adt(ty_def
, ..) if Some(ty_def
.did()) == cx
.tcx
.lang_items().unsafe_cell_type() => true,
144 ty
::Array(..) | ty
::Adt(..) | ty
::Tuple(..) => {
145 let val
= cx
.tcx
.destructure_const(cx
.param_env
.and(val
));
146 val
.fields
.iter().any(|field
| inner(cx
, *field
))
154 // Consider `TooGeneric` cases as being unfrozen.
155 // This causes a false positive where an assoc const whose type is unfrozen
156 // have a value that is a frozen variant with a generic param (an example is
157 // `declare_interior_mutable_const::enums::BothOfCellAndGeneric::GENERIC_VARIANT`).
158 // However, it prevents a number of false negatives that is, I think, important:
159 // 1. assoc consts in trait defs referring to consts of themselves
160 // (an example is `declare_interior_mutable_const::traits::ConcreteTypes::ANOTHER_ATOMIC`).
161 // 2. a path expr referring to assoc consts whose type is doesn't have
162 // any frozen variants in trait defs (i.e. without substitute for `Self`).
163 // (e.g. borrowing `borrow_interior_mutable_const::trait::ConcreteTypes::ATOMIC`)
164 // 3. similar to the false positive above;
165 // but the value is an unfrozen variant, or the type has no enums. (An example is
166 // `declare_interior_mutable_const::enums::BothOfCellAndGeneric::UNFROZEN_VARIANT`
167 // and `declare_interior_mutable_const::enums::BothOfCellAndGeneric::NO_ENUM`).
168 // One might be able to prevent these FNs correctly, and replace this with `false`;
169 // e.g. implementing `has_frozen_variant` described above, and not running this function
170 // when the type doesn't have any frozen variants would be the 'correct' way for the 2nd
171 // case (that actually removes another suboptimal behavior (I won't say 'false positive') where,
172 // similar to 2., but with the a frozen variant) (e.g. borrowing
173 // `borrow_interior_mutable_const::enums::AssocConsts::TO_BE_FROZEN_VARIANT`).
174 // I chose this way because unfrozen enums as assoc consts are rare (or, hopefully, none).
175 err
== ErrorHandled
::TooGeneric
177 |val
| inner(cx
, Const
::from_value(cx
.tcx
, val
, ty
)),
181 fn is_value_unfrozen_poly
<'tcx
>(cx
: &LateContext
<'tcx
>, body_id
: BodyId
, ty
: Ty
<'tcx
>) -> bool
{
182 let result
= cx
.tcx
.const_eval_poly(body_id
.hir_id
.owner
.to_def_id());
183 is_value_unfrozen_raw(cx
, result
, ty
)
186 fn is_value_unfrozen_expr
<'tcx
>(cx
: &LateContext
<'tcx
>, hir_id
: HirId
, def_id
: DefId
, ty
: Ty
<'tcx
>) -> bool
{
187 let substs
= cx
.typeck_results().node_substs(hir_id
);
189 let result
= cx
.tcx
.const_eval_resolve(
191 ty
::Unevaluated
::new(ty
::WithOptConstParam
::unknown(def_id
), substs
),
194 is_value_unfrozen_raw(cx
, result
, ty
)
197 #[derive(Copy, Clone)]
200 Assoc { item: Span }
,
206 fn lint(&self) -> (&'
static Lint
, &'
static str, Span
) {
208 Self::Item { item }
| Self::Assoc { item, .. }
=> (
209 DECLARE_INTERIOR_MUTABLE_CONST
,
210 "a `const` item should never be interior mutable",
213 Self::Expr { expr }
=> (
214 BORROW_INTERIOR_MUTABLE_CONST
,
215 "a `const` item with interior mutability should not be borrowed",
222 fn lint(cx
: &LateContext
<'_
>, source
: Source
) {
223 let (lint
, msg
, span
) = source
.lint();
224 span_lint_and_then(cx
, lint
, span
, msg
, |diag
| {
225 if span
.from_expansion() {
226 return; // Don't give suggestions into macros.
229 Source
::Item { .. }
=> {
230 let const_kw_span
= span
.from_inner(InnerSpan
::new(0, 5));
231 diag
.span_label(const_kw_span
, "make this a static item (maybe with lazy_static)");
233 Source
::Assoc { .. }
=> (),
234 Source
::Expr { .. }
=> {
235 diag
.help("assign this const to a local or static variable, and use the variable here");
241 declare_lint_pass
!(NonCopyConst
=> [DECLARE_INTERIOR_MUTABLE_CONST
, BORROW_INTERIOR_MUTABLE_CONST
]);
243 impl<'tcx
> LateLintPass
<'tcx
> for NonCopyConst
{
244 fn check_item(&mut self, cx
: &LateContext
<'tcx
>, it
: &'tcx Item
<'_
>) {
245 if let ItemKind
::Const(hir_ty
, body_id
) = it
.kind
{
246 let ty
= hir_ty_to_ty(cx
.tcx
, hir_ty
);
248 if is_unfrozen(cx
, ty
) && is_value_unfrozen_poly(cx
, body_id
, ty
) {
249 lint(cx
, Source
::Item { item: it.span }
);
254 fn check_trait_item(&mut self, cx
: &LateContext
<'tcx
>, trait_item
: &'tcx TraitItem
<'_
>) {
255 if let TraitItemKind
::Const(hir_ty
, body_id_opt
) = &trait_item
.kind
{
256 let ty
= hir_ty_to_ty(cx
.tcx
, hir_ty
);
258 // Normalize assoc types because ones originated from generic params
259 // bounded other traits could have their bound.
260 let normalized
= cx
.tcx
.normalize_erasing_regions(cx
.param_env
, ty
);
261 if is_unfrozen(cx
, normalized
)
262 // When there's no default value, lint it only according to its type;
263 // in other words, lint consts whose value *could* be unfrozen, not definitely is.
264 // This feels inconsistent with how the lint treats generic types,
265 // which avoids linting types which potentially become unfrozen.
266 // One could check whether an unfrozen type have a *frozen variant*
267 // (like `body_id_opt.map_or_else(|| !has_frozen_variant(...), ...)`),
268 // and do the same as the case of generic types at impl items.
269 // Note that it isn't sufficient to check if it has an enum
270 // since all of that enum's variants can be unfrozen:
271 // i.e. having an enum doesn't necessary mean a type has a frozen variant.
272 // And, implementing it isn't a trivial task; it'll probably end up
273 // re-implementing the trait predicate evaluation specific to `Freeze`.
274 && body_id_opt
.map_or(true, |body_id
| is_value_unfrozen_poly(cx
, body_id
, normalized
))
276 lint(cx
, Source
::Assoc { item: trait_item.span }
);
281 fn check_impl_item(&mut self, cx
: &LateContext
<'tcx
>, impl_item
: &'tcx ImplItem
<'_
>) {
282 if let ImplItemKind
::Const(hir_ty
, body_id
) = &impl_item
.kind
{
283 let item_def_id
= cx
.tcx
.hir().get_parent_item(impl_item
.hir_id());
284 let item
= cx
.tcx
.hir().expect_item(item_def_id
);
287 ItemKind
::Impl(Impl
{
288 of_trait
: Some(of_trait_ref
),
292 // Lint a trait impl item only when the definition is a generic type,
293 // assuming an assoc const is not meant to be an interior mutable type.
294 if let Some(of_trait_def_id
) = of_trait_ref
.trait_def_id();
295 if let Some(of_assoc_item
) = cx
297 .associated_item(impl_item
.def_id
)
301 .layout_of(cx
.tcx
.param_env(of_trait_def_id
).and(
302 // Normalize assoc types because ones originated from generic params
303 // bounded other traits could have their bound at the trait defs;
304 // and, in that case, the definition is *not* generic.
305 cx
.tcx
.normalize_erasing_regions(
306 cx
.tcx
.param_env(of_trait_def_id
),
307 cx
.tcx
.type_of(of_assoc_item
),
311 // If there were a function like `has_frozen_variant` described above,
312 // we should use here as a frozen variant is a potential to be frozen
313 // similar to unknown layouts.
314 // e.g. `layout_of(...).is_err() || has_frozen_variant(...);`
315 let ty
= hir_ty_to_ty(cx
.tcx
, hir_ty
);
316 let normalized
= cx
.tcx
.normalize_erasing_regions(cx
.param_env
, ty
);
317 if is_unfrozen(cx
, normalized
);
318 if is_value_unfrozen_poly(cx
, *body_id
, normalized
);
323 item
: impl_item
.span
,
329 ItemKind
::Impl(Impl { of_trait: None, .. }
) => {
330 let ty
= hir_ty_to_ty(cx
.tcx
, hir_ty
);
331 // Normalize assoc types originated from generic params.
332 let normalized
= cx
.tcx
.normalize_erasing_regions(cx
.param_env
, ty
);
334 if is_unfrozen(cx
, ty
) && is_value_unfrozen_poly(cx
, *body_id
, normalized
) {
335 lint(cx
, Source
::Assoc { item: impl_item.span }
);
343 fn check_expr(&mut self, cx
: &LateContext
<'tcx
>, expr
: &'tcx Expr
<'_
>) {
344 if let ExprKind
::Path(qpath
) = &expr
.kind
{
345 // Only lint if we use the const item inside a function.
346 if in_constant(cx
, expr
.hir_id
) {
350 // Make sure it is a const item.
351 let item_def_id
= match cx
.qpath_res(qpath
, expr
.hir_id
) {
352 Res
::Def(DefKind
::Const
| DefKind
::AssocConst
, did
) => did
,
356 // Climb up to resolve any field access and explicit referencing.
357 let mut cur_expr
= expr
;
358 let mut dereferenced_expr
= expr
;
359 let mut needs_check_adjustment
= true;
361 let parent_id
= cx
.tcx
.hir().get_parent_node(cur_expr
.hir_id
);
362 if parent_id
== cur_expr
.hir_id
{
365 if let Some(Node
::Expr(parent_expr
)) = cx
.tcx
.hir().find(parent_id
) {
366 match &parent_expr
.kind
{
367 ExprKind
::AddrOf(..) => {
368 // `&e` => `e` must be referenced.
369 needs_check_adjustment
= false;
371 ExprKind
::Field(..) => {
372 needs_check_adjustment
= true;
374 // Check whether implicit dereferences happened;
375 // if so, no need to go further up
376 // because of the same reason as the `ExprKind::Unary` case.
379 .expr_adjustments(dereferenced_expr
)
381 .any(|adj
| matches
!(adj
.kind
, Adjust
::Deref(_
)))
386 dereferenced_expr
= parent_expr
;
388 ExprKind
::Index(e
, _
) if ptr
::eq(&**e
, cur_expr
) => {
389 // `e[i]` => desugared to `*Index::index(&e, i)`,
390 // meaning `e` must be referenced.
391 // no need to go further up since a method call is involved now.
392 needs_check_adjustment
= false;
395 ExprKind
::Unary(UnOp
::Deref
, _
) => {
396 // `*e` => desugared to `*Deref::deref(&e)`,
397 // meaning `e` must be referenced.
398 // no need to go further up since a method call is involved now.
399 needs_check_adjustment
= false;
404 cur_expr
= parent_expr
;
410 let ty
= if needs_check_adjustment
{
411 let adjustments
= cx
.typeck_results().expr_adjustments(dereferenced_expr
);
412 if let Some(i
) = adjustments
414 .position(|adj
| matches
!(adj
.kind
, Adjust
::Borrow(_
) | Adjust
::Deref(_
)))
417 cx
.typeck_results().expr_ty(dereferenced_expr
)
419 adjustments
[i
- 1].target
422 // No borrow adjustments means the entire const is moved.
426 cx
.typeck_results().expr_ty(dereferenced_expr
)
429 if is_unfrozen(cx
, ty
) && is_value_unfrozen_expr(cx
, expr
.hir_id
, item_def_id
, ty
) {
430 lint(cx
, Source
::Expr { expr: expr.span }
);