From 61d4c74cba8c5b5cd26f4a41e3e4a83e6924f1bf Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Thu, 25 Sep 2025 11:09:55 +0200 Subject: [PATCH 1/8] clean-up --- clippy_utils/src/attrs.rs | 93 +++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 52 deletions(-) diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 2d42e76dcbc9..169f07f28aef 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -69,51 +69,40 @@ pub fn get_attr<'a, A: AttributeExt + 'a>( name: Symbol, ) -> impl Iterator { attrs.iter().filter(move |attr| { - let Some(attr_segments) = attr.ident_path() else { - return false; - }; - - if attr_segments.len() == 2 && attr_segments[0].name == sym::clippy { - BUILTIN_ATTRIBUTES + if let Some([clippy, segment2]) = attr.ident_path().as_deref() + && clippy.name == sym::clippy + { + let Some((_, deprecation_status)) = BUILTIN_ATTRIBUTES .iter() - .find_map(|(builtin_name, deprecation_status)| { - if attr_segments[1].name == *builtin_name { - Some(deprecation_status) - } else { - None - } - }) - .map_or_else( - || { - sess.dcx().span_err(attr_segments[1].span, "usage of unknown attribute"); - false - }, - |deprecation_status| { - let mut diag = sess - .dcx() - .struct_span_err(attr_segments[1].span, "usage of deprecated attribute"); - match *deprecation_status { - DeprecationStatus::Deprecated => { - diag.emit(); - false - }, - DeprecationStatus::Replaced(new_name) => { - diag.span_suggestion( - attr_segments[1].span, - "consider using", - new_name, - Applicability::MachineApplicable, - ); - diag.emit(); - false - }, - DeprecationStatus::None => { - diag.cancel(); - attr_segments[1].name == name - }, - } - }, - ) + .find(|(builtin_name, _)| segment2.name == *builtin_name) + else { + sess.dcx().span_err(segment2.span, "usage of unknown attribute"); + return false; + }; + + let mut diag = sess + .dcx() + .struct_span_err(segment2.span, "usage of deprecated attribute"); + match deprecation_status { + DeprecationStatus::Deprecated => { + diag.emit(); + false + }, + DeprecationStatus::Replaced(new_name) => { + diag.span_suggestion( + segment2.span, + "consider using", + new_name, + Applicability::MachineApplicable, + ); + diag.emit(); + false + }, + DeprecationStatus::None => { + diag.cancel(); + segment2.name == name + }, + } } else { false } @@ -122,15 +111,15 @@ pub fn get_attr<'a, A: AttributeExt + 'a>( fn parse_attrs(sess: &Session, attrs: &[impl AttributeExt], name: Symbol, mut f: F) { for attr in get_attr(sess, attrs, name) { - if let Some(value) = attr.value_str() { - if let Ok(value) = FromStr::from_str(value.as_str()) { - f(value); - } else { - sess.dcx().span_err(attr.span(), "not a number"); - } - } else { + let Some(value) = attr.value_str() else { sess.dcx().span_err(attr.span(), "bad clippy attribute"); - } + continue; + }; + let Ok(value) = u64::from_str(value.as_str()) else { + sess.dcx().span_err(attr.span(), "not a number"); + continue; + }; + f(value); } } From 9c3076c5961bf80a9c9a64cfbaa83d1d237fe507 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 28 Sep 2025 12:49:12 +0200 Subject: [PATCH 2/8] merge `DeprecationStatus::{Replaced,Deprecated}` Their meanings, and the way they're handled in `get_attr`, are pretty similar --- clippy_utils/src/attrs.rs | 76 ++++++++++++++------------------------- 1 file changed, 27 insertions(+), 49 deletions(-) diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 169f07f28aef..aef5c59e8f10 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -12,29 +12,6 @@ use rustc_session::Session; use rustc_span::{Span, Symbol}; use std::str::FromStr; -/// Deprecation status of attributes known by Clippy. -pub enum DeprecationStatus { - /// Attribute is deprecated - Deprecated, - /// Attribute is deprecated and was replaced by the named attribute - Replaced(&'static str), - None, -} - -#[rustfmt::skip] -pub const BUILTIN_ATTRIBUTES: &[(Symbol, DeprecationStatus)] = &[ - (sym::author, DeprecationStatus::None), - (sym::version, DeprecationStatus::None), - (sym::cognitive_complexity, DeprecationStatus::None), - (sym::cyclomatic_complexity, DeprecationStatus::Replaced("cognitive_complexity")), - (sym::dump, DeprecationStatus::None), - (sym::msrv, DeprecationStatus::None), - // The following attributes are for the 3rd party crate authors. - // See book/src/attribs.md - (sym::has_significant_drop, DeprecationStatus::None), - (sym::format_args, DeprecationStatus::None), -]; - pub struct LimitStack { stack: Vec, } @@ -72,36 +49,37 @@ pub fn get_attr<'a, A: AttributeExt + 'a>( if let Some([clippy, segment2]) = attr.ident_path().as_deref() && clippy.name == sym::clippy { - let Some((_, deprecation_status)) = BUILTIN_ATTRIBUTES - .iter() - .find(|(builtin_name, _)| segment2.name == *builtin_name) - else { - sess.dcx().span_err(segment2.span, "usage of unknown attribute"); - return false; + let new_name = match segment2.name { + sym::cyclomatic_complexity => Some("cognitive_complexity"), + sym::author + | sym::version + | sym::cognitive_complexity + | sym::dump + | sym::msrv + // The following attributes are for the 3rd party crate authors. + // See book/src/attribs.md + | sym::has_significant_drop + | sym::format_args => None, + _ => { + sess.dcx().span_err(segment2.span, "usage of unknown attribute"); + return false; + }, }; - let mut diag = sess - .dcx() - .struct_span_err(segment2.span, "usage of deprecated attribute"); - match deprecation_status { - DeprecationStatus::Deprecated => { - diag.emit(); + match new_name { + Some(new_name) => { + sess.dcx() + .struct_span_err(segment2.span, "usage of deprecated attribute") + .with_span_suggestion( + segment2.span, + "consider using", + new_name, + Applicability::MachineApplicable, + ) + .emit(); false }, - DeprecationStatus::Replaced(new_name) => { - diag.span_suggestion( - segment2.span, - "consider using", - new_name, - Applicability::MachineApplicable, - ); - diag.emit(); - false - }, - DeprecationStatus::None => { - diag.cancel(); - segment2.name == name - }, + None => segment2.name == name, } } else { false From abd5c21503c67964951629e5489868eb3b134be0 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Thu, 25 Sep 2025 21:22:48 +0200 Subject: [PATCH 3/8] overhaul `LimitStack` - Move it and its helper function `parse_attrs` together to the end of the file, because it's surprising to see front-and-center a struct that's only really used in one place (`cognitive_complexity`). - Avoid panic path in `LimitStack::limit` - Replace `assert` with `debug_assert` to avoid panics in release builds --- clippy_utils/src/attrs.rs | 88 ++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index aef5c59e8f10..b4fafc998229 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -12,34 +12,6 @@ use rustc_session::Session; use rustc_span::{Span, Symbol}; use std::str::FromStr; -pub struct LimitStack { - stack: Vec, -} - -impl Drop for LimitStack { - fn drop(&mut self) { - assert_eq!(self.stack.len(), 1); - } -} - -impl LimitStack { - #[must_use] - pub fn new(limit: u64) -> Self { - Self { stack: vec![limit] } - } - pub fn limit(&self) -> u64 { - *self.stack.last().expect("there should always be a value in the stack") - } - pub fn push_attrs(&mut self, sess: &Session, attrs: &[impl AttributeExt], name: Symbol) { - let stack = &mut self.stack; - parse_attrs(sess, attrs, name, |val| stack.push(val)); - } - pub fn pop_attrs(&mut self, sess: &Session, attrs: &[impl AttributeExt], name: Symbol) { - let stack = &mut self.stack; - parse_attrs(sess, attrs, name, |val| assert_eq!(stack.pop(), Some(val))); - } -} - pub fn get_attr<'a, A: AttributeExt + 'a>( sess: &'a Session, attrs: &'a [A], @@ -87,20 +59,6 @@ pub fn get_attr<'a, A: AttributeExt + 'a>( }) } -fn parse_attrs(sess: &Session, attrs: &[impl AttributeExt], name: Symbol, mut f: F) { - for attr in get_attr(sess, attrs, name) { - let Some(value) = attr.value_str() else { - sess.dcx().span_err(attr.span(), "bad clippy attribute"); - continue; - }; - let Ok(value) = u64::from_str(value.as_str()) else { - sess.dcx().span_err(attr.span(), "not a number"); - continue; - }; - f(value); - } -} - pub fn get_unique_attr<'a, A: AttributeExt>(sess: &'a Session, attrs: &'a [A], name: Symbol) -> Option<&'a A> { let mut unique_attr: Option<&A> = None; for attr in get_attr(sess, attrs, name) { @@ -165,3 +123,49 @@ pub fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool { false }) } +pub struct LimitStack { + default: u64, + stack: Vec, +} + +impl Drop for LimitStack { + fn drop(&mut self) { + debug_assert_eq!(self.stack, Vec::::new()); // avoid `.is_empty()`, for a nicer error message + } +} + +impl LimitStack { + #[must_use] + /// Initialize the stack starting with a default value, which usually comes from configuration + pub fn new(limit: u64) -> Self { + Self { + default: limit, + stack: vec![], + } + } + pub fn limit(&self) -> u64 { + self.stack.last().copied().unwrap_or(self.default) + } + pub fn push_attrs(&mut self, sess: &Session, attrs: &[impl AttributeExt], name: Symbol) { + let stack = &mut self.stack; + parse_attrs(sess, attrs, name, |val| stack.push(val)); + } + pub fn pop_attrs(&mut self, sess: &Session, attrs: &[impl AttributeExt], name: Symbol) { + let stack = &mut self.stack; + parse_attrs(sess, attrs, name, |val| debug_assert_eq!(stack.pop(), Some(val))); + } +} + +fn parse_attrs(sess: &Session, attrs: &[impl AttributeExt], name: Symbol, mut f: F) { + for attr in get_attr(sess, attrs, name) { + let Some(value) = attr.value_str() else { + sess.dcx().span_err(attr.span(), "bad clippy attribute"); + continue; + }; + let Ok(value) = u64::from_str(value.as_str()) else { + sess.dcx().span_err(attr.span(), "not a number"); + continue; + }; + f(value); + } +} From 3bb0bf37cb2e98e3ced32bdc23dbcb590cb86dbb Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Thu, 25 Sep 2025 19:54:17 +0200 Subject: [PATCH 4/8] give functions more descriptive names, add docs --- .../matches/significant_drop_in_scrutinee.rs | 4 +-- .../src/significant_drop_tightening.rs | 4 +-- clippy_lints/src/utils/author.rs | 4 +-- clippy_lints/src/utils/dump_hir.rs | 4 +-- clippy_utils/src/attrs.rs | 25 +++++++++++++------ clippy_utils/src/lib.rs | 1 + clippy_utils/src/macros.rs | 4 +-- 7 files changed, 28 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs index 81fecc87256c..bac35e7f8c70 100644 --- a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs +++ b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs @@ -4,7 +4,7 @@ use crate::FxHashSet; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::{first_line_of_span, indent_of, snippet}; use clippy_utils::ty::{for_each_top_level_late_bound_region, is_copy}; -use clippy_utils::{get_attr, is_lint_allowed, sym}; +use clippy_utils::{get_builtin_attr, is_lint_allowed, sym}; use itertools::Itertools; use rustc_ast::Mutability; use rustc_data_structures::fx::FxIndexSet; @@ -183,7 +183,7 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> { fn has_sig_drop_attr_impl(&mut self, ty: Ty<'tcx>) -> bool { if let Some(adt) = ty.ty_adt_def() - && get_attr( + && get_builtin_attr( self.cx.sess(), self.cx.tcx.get_all_attrs(adt.did()), sym::has_significant_drop, diff --git a/clippy_lints/src/significant_drop_tightening.rs b/clippy_lints/src/significant_drop_tightening.rs index dcce90649958..b4e7c771a54f 100644 --- a/clippy_lints/src/significant_drop_tightening.rs +++ b/clippy_lints/src/significant_drop_tightening.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::res::MaybeResPath; use clippy_utils::source::{indent_of, snippet}; -use clippy_utils::{expr_or_init, get_attr, peel_hir_expr_unary, sym}; +use clippy_utils::{expr_or_init, get_builtin_attr, peel_hir_expr_unary, sym}; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; @@ -167,7 +167,7 @@ impl<'cx, 'others, 'tcx> AttrChecker<'cx, 'others, 'tcx> { fn has_sig_drop_attr_uncached(&mut self, ty: Ty<'tcx>, depth: usize) -> bool { if let Some(adt) = ty.ty_adt_def() { - let mut iter = get_attr( + let mut iter = get_builtin_attr( self.cx.sess(), self.cx.tcx.get_all_attrs(adt.did()), sym::has_significant_drop, diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 68e51dace2db..4b157dc41142 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -1,5 +1,5 @@ use clippy_utils::res::MaybeQPath; -use clippy_utils::{get_attr, higher, sym}; +use clippy_utils::{get_builtin_attr, higher, sym}; use itertools::Itertools; use rustc_ast::LitIntType; use rustc_ast::ast::{LitFloatType, LitKind}; @@ -856,5 +856,5 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> { fn has_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool { let attrs = cx.tcx.hir_attrs(hir_id); - get_attr(cx.sess(), attrs, sym::author).count() > 0 + get_builtin_attr(cx.sess(), attrs, sym::author).count() > 0 } diff --git a/clippy_lints/src/utils/dump_hir.rs b/clippy_lints/src/utils/dump_hir.rs index d6cf07fdaf3f..b490866f0a11 100644 --- a/clippy_lints/src/utils/dump_hir.rs +++ b/clippy_lints/src/utils/dump_hir.rs @@ -1,4 +1,4 @@ -use clippy_utils::{get_attr, sym}; +use clippy_utils::{get_builtin_attr, sym}; use hir::TraitItem; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -60,5 +60,5 @@ impl<'tcx> LateLintPass<'tcx> for DumpHir { fn has_attr(cx: &LateContext<'_>, hir_id: hir::HirId) -> bool { let attrs = cx.tcx.hir_attrs(hir_id); - get_attr(cx.sess(), attrs, sym::dump).count() > 0 + get_builtin_attr(cx.sess(), attrs, sym::dump).count() > 0 } diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index b4fafc998229..671b266ba008 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -1,3 +1,5 @@ +//! Utility functions for attributes, including Clippy's built-in ones + use crate::source::SpanRangeExt; use crate::{sym, tokenize_with_text}; use rustc_ast::attr; @@ -12,7 +14,8 @@ use rustc_session::Session; use rustc_span::{Span, Symbol}; use std::str::FromStr; -pub fn get_attr<'a, A: AttributeExt + 'a>( +/// Given `attrs`, extract all the instances of a built-in Clippy attribute called `name` +pub fn get_builtin_attr<'a, A: AttributeExt + 'a>( sess: &'a Session, attrs: &'a [A], name: Symbol, @@ -59,9 +62,11 @@ pub fn get_attr<'a, A: AttributeExt + 'a>( }) } -pub fn get_unique_attr<'a, A: AttributeExt>(sess: &'a Session, attrs: &'a [A], name: Symbol) -> Option<&'a A> { +/// If `attrs` contain exactly one instance of a built-in Clippy attribute called `name`, +/// returns that attribute, and `None` otherwise +pub fn get_unique_builtin_attr<'a, A: AttributeExt>(sess: &'a Session, attrs: &'a [A], name: Symbol) -> Option<&'a A> { let mut unique_attr: Option<&A> = None; - for attr in get_attr(sess, attrs, name) { + for attr in get_builtin_attr(sess, attrs, name) { if let Some(duplicate) = unique_attr { sess.dcx() .struct_span_err(attr.span(), format!("`{name}` is defined multiple times")) @@ -74,13 +79,13 @@ pub fn get_unique_attr<'a, A: AttributeExt>(sess: &'a Session, attrs: &'a [A], n unique_attr } -/// Returns true if the attributes contain any of `proc_macro`, -/// `proc_macro_derive` or `proc_macro_attribute`, false otherwise +/// Checks whether `attrs` contain any of `proc_macro`, `proc_macro_derive` or +/// `proc_macro_attribute` pub fn is_proc_macro(attrs: &[impl AttributeExt]) -> bool { attrs.iter().any(AttributeExt::is_proc_macro_attr) } -/// Returns true if the attributes contain `#[doc(hidden)]` +/// Checks whether `attrs` contain `#[doc(hidden)]` pub fn is_doc_hidden(attrs: &[impl AttributeExt]) -> bool { attrs .iter() @@ -89,6 +94,7 @@ pub fn is_doc_hidden(attrs: &[impl AttributeExt]) -> bool { .any(|l| attr::list_contains_name(&l, sym::hidden)) } +/// Checks whether the given ADT, or any of its fields/variants, are marked as `#[non_exhaustive]` pub fn has_non_exhaustive_attr(tcx: TyCtxt<'_>, adt: AdtDef<'_>) -> bool { adt.is_variant_list_non_exhaustive() || find_attr!(tcx.get_all_attrs(adt.did()), AttributeKind::NonExhaustive(..)) @@ -101,7 +107,7 @@ pub fn has_non_exhaustive_attr(tcx: TyCtxt<'_>, adt: AdtDef<'_>) -> bool { .any(|field_def| find_attr!(tcx.get_all_attrs(field_def.did), AttributeKind::NonExhaustive(..))) } -/// Checks if the given span contains a `#[cfg(..)]` attribute +/// Checks whether the given span contains a `#[cfg(..)]` attribute pub fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool { s.check_source_text(cx, |src| { let mut iter = tokenize_with_text(src); @@ -123,6 +129,8 @@ pub fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool { false }) } + +/// Currently used to keep track of the current value of `#[clippy::cognitive_complexity(N)]` pub struct LimitStack { default: u64, stack: Vec, @@ -134,6 +142,7 @@ impl Drop for LimitStack { } } +#[expect(missing_docs, reason = "they're all trivial...")] impl LimitStack { #[must_use] /// Initialize the stack starting with a default value, which usually comes from configuration @@ -157,7 +166,7 @@ impl LimitStack { } fn parse_attrs(sess: &Session, attrs: &[impl AttributeExt], name: Symbol, mut f: F) { - for attr in get_attr(sess, attrs, name) { + for attr in get_builtin_attr(sess, attrs, name) { let Some(value) = attr.value_str() else { sess.dcx().span_err(attr.span(), "bad clippy attribute"); continue; diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 8250104b0ca2..58aa82675aae 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -52,6 +52,7 @@ extern crate rustc_trait_selection; extern crate smallvec; pub mod ast_utils; +#[deny(missing_docs)] pub mod attrs; mod check_proc_macro; pub mod comparisons; diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index 7cd5a16f5b46..4e06f010bd59 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -3,7 +3,7 @@ use std::sync::{Arc, OnceLock}; use crate::visitors::{Descend, for_each_expr_without_closures}; -use crate::{get_unique_attr, sym}; +use crate::{get_unique_builtin_attr, sym}; use arrayvec::ArrayVec; use rustc_ast::{FormatArgs, FormatArgument, FormatPlaceholder}; @@ -42,7 +42,7 @@ pub fn is_format_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool { } else { // Allow users to tag any macro as being format!-like // TODO: consider deleting FORMAT_MACRO_DIAG_ITEMS and using just this method - get_unique_attr(cx.sess(), cx.tcx.get_all_attrs(macro_def_id), sym::format_args).is_some() + get_unique_builtin_attr(cx.sess(), cx.tcx.get_all_attrs(macro_def_id), sym::format_args).is_some() } } From a1dc025bbbcde12d26db0976e36d8fd2fb576f99 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Wed, 8 Oct 2025 00:03:50 +0200 Subject: [PATCH 5/8] clean-up --- clippy_lints/src/mut_key.rs | 19 +++++++++---------- clippy_utils/src/ty/mod.rs | 28 ++++++++++++---------------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index 98a9a98d281a..550684f082cb 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -131,17 +131,16 @@ impl<'tcx> MutableKeyType<'tcx> { cx.tcx.get_diagnostic_name(def.did()), Some(sym::HashMap | sym::BTreeMap | sym::HashSet | sym::BTreeSet) ) + && let subst_ty = args.type_at(0) + && let Some(chain) = self.interior_mut.interior_mut_ty_chain(cx, subst_ty) { - let subst_ty = args.type_at(0); - if let Some(chain) = self.interior_mut.interior_mut_ty_chain(cx, subst_ty) { - span_lint_and_then(cx, MUTABLE_KEY_TYPE, span, "mutable key type", |diag| { - for ty in chain.iter().rev() { - diag.note(with_forced_trimmed_paths!(format!( - "... because it contains `{ty}`, which has interior mutability" - ))); - } - }); - } + span_lint_and_then(cx, MUTABLE_KEY_TYPE, span, "mutable key type", |diag| { + for ty in chain.iter().rev() { + diag.note(with_forced_trimmed_paths!(format!( + "... because it contains `{ty}`, which has interior mutability" + ))); + } + }); } } } diff --git a/clippy_utils/src/ty/mod.rs b/clippy_utils/src/ty/mod.rs index bb2ef9fa4692..554033f92f48 100644 --- a/clippy_utils/src/ty/mod.rs +++ b/clippy_utils/src/ty/mod.rs @@ -26,7 +26,7 @@ use rustc_middle::ty::{ TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, UintTy, Upcast, VariantDef, VariantDiscr, }; use rustc_span::symbol::Ident; -use rustc_span::{DUMMY_SP, Span, Symbol, sym}; +use rustc_span::{DUMMY_SP, Span, Symbol}; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; use rustc_trait_selection::traits::query::normalize::QueryNormalizeExt; use rustc_trait_selection::traits::{Obligation, ObligationCause}; @@ -36,6 +36,7 @@ use std::{iter, mem}; use crate::paths::{PathNS, lookup_path_str}; use crate::res::{MaybeDef, MaybeQPath}; +use crate::sym; mod type_certainty; pub use type_certainty::expr_type_is_certain; @@ -1067,7 +1068,7 @@ impl<'tcx> InteriorMut<'tcx> { /// Check if given type has interior mutability such as [`std::cell::Cell`] or /// [`std::cell::RefCell`] etc. and if it does, returns a chain of types that causes - /// this type to be interior mutable. False negatives may be expected for infinitely recursive + /// this type to be interior mutable. False negatives may be expected for infinitely recursive /// types, and `None` will be returned there. pub fn interior_mut_ty_chain(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'tcx ty::List>> { self.interior_mut_ty_chain_inner(cx, ty, 0) @@ -1163,13 +1164,11 @@ pub fn make_normalized_projection_with_regions<'tcx>( .enumerate() .find(|(_, arg)| arg.has_escaping_bound_vars()) { - debug_assert!( - false, + panic!( "args contain late-bound region at index `{i}` which can't be normalized.\n\ use `TyCtxt::instantiate_bound_regions_with_erased`\n\ note: arg is `{arg:#?}`", ); - return None; } let cause = ObligationCause::dummy(); let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env); @@ -1219,17 +1218,14 @@ pub fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl /// This does not look for impls in the type's `Deref::Target` type. /// If you need this, you should wrap this call in `clippy_utils::ty::deref_chain().any(...)`. pub fn get_adt_inherent_method<'a>(cx: &'a LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> Option<&'a AssocItem> { - if let Some(ty_did) = ty.ty_adt_def().map(AdtDef::did) { - cx.tcx.inherent_impls(ty_did).iter().find_map(|&did| { - cx.tcx - .associated_items(did) - .filter_by_name_unhygienic(method_name) - .next() - .filter(|item| item.as_tag() == AssocTag::Fn) - }) - } else { - None - } + let ty_did = ty.ty_adt_def().map(AdtDef::did)?; + cx.tcx.inherent_impls(ty_did).iter().find_map(|&did| { + cx.tcx + .associated_items(did) + .filter_by_name_unhygienic(method_name) + .next() + .filter(|item| item.as_tag() == AssocTag::Fn) + }) } /// Gets the type of a field by name. From f17af45f48bc74b5c93063e66d06adc69d4d61b4 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Wed, 8 Oct 2025 00:50:42 +0200 Subject: [PATCH 6/8] non_copy_const: pass `cx` instead of `tcx` everywhere This will be required for the next commit -- kept as a separate commit to reduce noise --- clippy_lints/src/non_copy_const.rs | 166 ++++++++++++++--------------- 1 file changed, 83 insertions(+), 83 deletions(-) diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 2fffc4244a73..4f4c57d87a0b 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -271,13 +271,13 @@ impl<'tcx> NonCopyConst<'tcx> { } /// Checks if a value of the given type is `Freeze`, or may be depending on the value. - fn is_ty_freeze(&mut self, tcx: TyCtxt<'tcx>, typing_env: TypingEnv<'tcx>, ty: Ty<'tcx>) -> IsFreeze { - let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); + fn is_ty_freeze(&mut self, cx: &LateContext<'tcx>, typing_env: TypingEnv<'tcx>, ty: Ty<'tcx>) -> IsFreeze { + let ty = cx.tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); match self.freeze_tys.entry(ty) { Entry::Occupied(e) => *e.get(), Entry::Vacant(e) => { let e = e.insert(IsFreeze::Yes); - if ty.is_freeze(tcx, typing_env) { + if ty.is_freeze(cx.tcx, typing_env) { return IsFreeze::Yes; } let is_freeze = match *ty.kind() { @@ -290,14 +290,15 @@ impl<'tcx> NonCopyConst<'tcx> { IsFreeze::from_fields( v.fields .iter() - .map(|f| self.is_ty_freeze(tcx, typing_env, f.ty(tcx, args))), + .map(|f| self.is_ty_freeze(cx, typing_env, f.ty(cx.tcx, args))), ) })), // Workaround for `ManuallyDrop`-like unions. ty::Adt(adt, args) if adt.is_union() && adt.non_enum_variant().fields.iter().any(|f| { - tcx.layout_of(typing_env.as_query_input(f.ty(tcx, args))) + cx.tcx + .layout_of(typing_env.as_query_input(f.ty(cx.tcx, args))) .is_ok_and(|l| l.layout.size().bytes() == 0) }) => { @@ -309,12 +310,10 @@ impl<'tcx> NonCopyConst<'tcx> { adt.non_enum_variant() .fields .iter() - .map(|f| self.is_ty_freeze(tcx, typing_env, f.ty(tcx, args))), + .map(|f| self.is_ty_freeze(cx, typing_env, f.ty(cx.tcx, args))), ), - ty::Array(ty, _) | ty::Pat(ty, _) => self.is_ty_freeze(tcx, typing_env, ty), - ty::Tuple(tys) => { - IsFreeze::from_fields(tys.iter().map(|ty| self.is_ty_freeze(tcx, typing_env, ty))) - }, + ty::Array(ty, _) | ty::Pat(ty, _) => self.is_ty_freeze(cx, typing_env, ty), + ty::Tuple(tys) => IsFreeze::from_fields(tys.iter().map(|ty| self.is_ty_freeze(cx, typing_env, ty))), // Treat type parameters as though they were `Freeze`. ty::Param(_) | ty::Alias(..) => return IsFreeze::Yes, // TODO: check other types. @@ -335,21 +334,22 @@ impl<'tcx> NonCopyConst<'tcx> { /// cannot be read, but the result depends on the value. fn is_value_freeze( &mut self, - tcx: TyCtxt<'tcx>, + cx: &LateContext<'tcx>, typing_env: TypingEnv<'tcx>, ty: Ty<'tcx>, val: ConstValue, ) -> Result { - let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); - match self.is_ty_freeze(tcx, typing_env, ty) { + let ty = cx.tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); + match self.is_ty_freeze(cx, typing_env, ty) { IsFreeze::Yes => Ok(true), IsFreeze::Maybe if matches!(ty.kind(), ty::Adt(..) | ty::Array(..) | ty::Tuple(..)) => { - for &(val, ty) in tcx + for &(val, ty) in cx + .tcx .try_destructure_mir_constant_for_user_output(val, ty) .ok_or(())? .fields { - if !self.is_value_freeze(tcx, typing_env, ty, val)? { + if !self.is_value_freeze(cx, typing_env, ty, val)? { return Ok(false); } } @@ -368,16 +368,16 @@ impl<'tcx> NonCopyConst<'tcx> { /// `typeck` and `e` are from the constant's definition site. fn is_init_expr_freeze( &mut self, - tcx: TyCtxt<'tcx>, + cx: &LateContext<'tcx>, typing_env: TypingEnv<'tcx>, typeck: &'tcx TypeckResults<'tcx>, gen_args: GenericArgsRef<'tcx>, e: &'tcx Expr<'tcx>, ) -> bool { // Make sure to instantiate all types coming from `typeck` with `gen_args`. - let ty = EarlyBinder::bind(typeck.expr_ty(e)).instantiate(tcx, gen_args); - let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); - match self.is_ty_freeze(tcx, typing_env, ty) { + let ty = EarlyBinder::bind(typeck.expr_ty(e)).instantiate(cx.tcx, gen_args); + let ty = cx.tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); + match self.is_ty_freeze(cx, typing_env, ty) { IsFreeze::Yes => true, IsFreeze::No => false, IsFreeze::Maybe => match e.kind { @@ -386,23 +386,25 @@ impl<'tcx> NonCopyConst<'tcx> { && b.stmts.is_empty() && let Some(e) = b.expr => { - self.is_init_expr_freeze(tcx, typing_env, typeck, gen_args, e) + self.is_init_expr_freeze(cx, typing_env, typeck, gen_args, e) }, ExprKind::Path(ref p) => { let res = typeck.qpath_res(p, e.hir_id); - let gen_args = EarlyBinder::bind(typeck.node_args(e.hir_id)).instantiate(tcx, gen_args); + let gen_args = EarlyBinder::bind(typeck.node_args(e.hir_id)).instantiate(cx.tcx, gen_args); match res { Res::Def(DefKind::Const | DefKind::AssocConst, did) - if let Ok(val) = - tcx.const_eval_resolve(typing_env, UnevaluatedConst::new(did, gen_args), DUMMY_SP) - && let Ok(is_freeze) = self.is_value_freeze(tcx, typing_env, ty, val) => + if let Ok(val) = cx.tcx.const_eval_resolve( + typing_env, + UnevaluatedConst::new(did, gen_args), + DUMMY_SP, + ) && let Ok(is_freeze) = self.is_value_freeze(cx, typing_env, ty, val) => { is_freeze }, Res::Def(DefKind::Const | DefKind::AssocConst, did) - if let Some((typeck, init)) = get_const_hir_value(tcx, typing_env, did, gen_args) => + if let Some((typeck, init)) = get_const_hir_value(cx.tcx, typing_env, did, gen_args) => { - self.is_init_expr_freeze(tcx, typing_env, typeck, gen_args, init) + self.is_init_expr_freeze(cx, typing_env, typeck, gen_args, init) }, // Either this is a unit constructor, or some unknown value. // In either case we consider the value to be `Freeze`. @@ -415,15 +417,15 @@ impl<'tcx> NonCopyConst<'tcx> { && matches!(res, Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(_)) => { args.iter() - .all(|e| self.is_init_expr_freeze(tcx, typing_env, typeck, gen_args, e)) + .all(|e| self.is_init_expr_freeze(cx, typing_env, typeck, gen_args, e)) }, ExprKind::Struct(_, fields, StructTailExpr::None) => fields .iter() - .all(|f| self.is_init_expr_freeze(tcx, typing_env, typeck, gen_args, f.expr)), + .all(|f| self.is_init_expr_freeze(cx, typing_env, typeck, gen_args, f.expr)), ExprKind::Tup(exprs) | ExprKind::Array(exprs) => exprs .iter() - .all(|e| self.is_init_expr_freeze(tcx, typing_env, typeck, gen_args, e)), - ExprKind::Repeat(e, _) => self.is_init_expr_freeze(tcx, typing_env, typeck, gen_args, e), + .all(|e| self.is_init_expr_freeze(cx, typing_env, typeck, gen_args, e)), + ExprKind::Repeat(e, _) => self.is_init_expr_freeze(cx, typing_env, typeck, gen_args, e), _ => true, }, } @@ -433,24 +435,24 @@ impl<'tcx> NonCopyConst<'tcx> { /// definitely a non-`Freeze` type. fn is_non_freeze_expr_borrowed( &mut self, - tcx: TyCtxt<'tcx>, + cx: &LateContext<'tcx>, typing_env: TypingEnv<'tcx>, typeck: &'tcx TypeckResults<'tcx>, mut src_expr: &'tcx Expr<'tcx>, ) -> Option> { - let mut parents = tcx.hir_parent_iter(src_expr.hir_id); + let mut parents = cx.tcx.hir_parent_iter(src_expr.hir_id); loop { let ty = typeck.expr_ty(src_expr); // Normalized as we need to check if this is an array later. - let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); - let is_freeze = self.is_ty_freeze(tcx, typing_env, ty); + let ty = cx.tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); + let is_freeze = self.is_ty_freeze(cx, typing_env, ty); if is_freeze.is_freeze() { return None; } if let [adjust, ..] = typeck.expr_adjustments(src_expr) { return does_adjust_borrow(adjust) .filter(|_| is_freeze.is_not_freeze()) - .map(|cause| BorrowSource::new(tcx, src_expr, cause)); + .map(|cause| BorrowSource::new(cx.tcx, src_expr, cause)); } let Some((_, Node::Expr(use_expr))) = parents.next() else { return None; @@ -459,7 +461,7 @@ impl<'tcx> NonCopyConst<'tcx> { ExprKind::Field(..) => {}, ExprKind::Index(..) if ty.is_array() => {}, ExprKind::AddrOf(..) if is_freeze.is_not_freeze() => { - return Some(BorrowSource::new(tcx, use_expr, BorrowCause::Borrow)); + return Some(BorrowSource::new(cx.tcx, use_expr, BorrowCause::Borrow)); }, // All other expressions use the value. _ => return None, @@ -473,29 +475,29 @@ impl<'tcx> NonCopyConst<'tcx> { /// result depends on the value. fn is_non_freeze_val_borrowed( &mut self, - tcx: TyCtxt<'tcx>, + cx: &LateContext<'tcx>, typing_env: TypingEnv<'tcx>, typeck: &'tcx TypeckResults<'tcx>, mut src_expr: &'tcx Expr<'tcx>, mut val: ConstValue, ) -> Result>, ()> { - let mut parents = tcx.hir_parent_iter(src_expr.hir_id); + let mut parents = cx.tcx.hir_parent_iter(src_expr.hir_id); let mut ty = typeck.expr_ty(src_expr); loop { // Normalized as we need to check if this is an array later. - ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); + ty = cx.tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); if let [adjust, ..] = typeck.expr_adjustments(src_expr) { let res = if let Some(cause) = does_adjust_borrow(adjust) - && !self.is_value_freeze(tcx, typing_env, ty, val)? + && !self.is_value_freeze(cx, typing_env, ty, val)? { - Some(BorrowSource::new(tcx, src_expr, cause)) + Some(BorrowSource::new(cx.tcx, src_expr, cause)) } else { None }; return Ok(res); } // Check only the type here as the result gets cached for each type. - if self.is_ty_freeze(tcx, typing_env, ty).is_freeze() { + if self.is_ty_freeze(cx, typing_env, ty).is_freeze() { return Ok(None); } let Some((_, Node::Expr(use_expr))) = parents.next() else { @@ -504,7 +506,8 @@ impl<'tcx> NonCopyConst<'tcx> { let next_val = match use_expr.kind { ExprKind::Field(_, name) => { if let Some(idx) = get_field_idx_by_name(ty, name.name) { - tcx.try_destructure_mir_constant_for_user_output(val, ty) + cx.tcx + .try_destructure_mir_constant_for_user_output(val, ty) .ok_or(())? .fields .get(idx) @@ -513,23 +516,21 @@ impl<'tcx> NonCopyConst<'tcx> { } }, ExprKind::Index(_, idx, _) if ty.is_array() => { - let val = tcx.try_destructure_mir_constant_for_user_output(val, ty).ok_or(())?; - if let Some(Constant::Int(idx)) = ConstEvalCtxt::with_env(tcx, typing_env, typeck).eval(idx) { + let val = cx.tcx.try_destructure_mir_constant_for_user_output(val, ty).ok_or(())?; + if let Some(Constant::Int(idx)) = ConstEvalCtxt::with_env(cx.tcx, typing_env, typeck).eval(idx) { val.fields.get(idx as usize) } else { // It's some value in the array so check all of them. for &(val, _) in val.fields { - if let Some(src) = - self.is_non_freeze_val_borrowed(tcx, typing_env, typeck, use_expr, val)? - { + if let Some(src) = self.is_non_freeze_val_borrowed(cx, typing_env, typeck, use_expr, val)? { return Ok(Some(src)); } } return Ok(None); } }, - ExprKind::AddrOf(..) if !self.is_value_freeze(tcx, typing_env, ty, val)? => { - return Ok(Some(BorrowSource::new(tcx, use_expr, BorrowCause::Borrow))); + ExprKind::AddrOf(..) if !self.is_value_freeze(cx, typing_env, ty, val)? => { + return Ok(Some(BorrowSource::new(cx.tcx, use_expr, BorrowCause::Borrow))); }, // All other expressions use the value. _ => return Ok(None), @@ -552,7 +553,7 @@ impl<'tcx> NonCopyConst<'tcx> { #[expect(clippy::too_many_arguments, clippy::too_many_lines)] fn is_non_freeze_init_borrowed( &mut self, - tcx: TyCtxt<'tcx>, + cx: &LateContext<'tcx>, typing_env: TypingEnv<'tcx>, typeck: &'tcx TypeckResults<'tcx>, mut src_expr: &'tcx Expr<'tcx>, @@ -561,13 +562,13 @@ impl<'tcx> NonCopyConst<'tcx> { mut init_expr: &'tcx Expr<'tcx>, ) -> Option> { // Make sure to instantiate all types coming from `init_typeck` with `init_args`. - let mut parents = tcx.hir_parent_iter(src_expr.hir_id); + let mut parents = cx.tcx.hir_parent_iter(src_expr.hir_id); loop { // First handle any adjustments since they are cheap to check. if let [adjust, ..] = typeck.expr_adjustments(src_expr) { return does_adjust_borrow(adjust) - .filter(|_| !self.is_init_expr_freeze(tcx, typing_env, init_typeck, init_args, init_expr)) - .map(|cause| BorrowSource::new(tcx, src_expr, cause)); + .filter(|_| !self.is_init_expr_freeze(cx, typing_env, init_typeck, init_args, init_expr)) + .map(|cause| BorrowSource::new(cx.tcx, src_expr, cause)); } // Then read through constants and blocks on the init expression before @@ -583,22 +584,22 @@ impl<'tcx> NonCopyConst<'tcx> { }, ExprKind::Path(ref init_path) => { let next_init_args = - EarlyBinder::bind(init_typeck.node_args(init_expr.hir_id)).instantiate(tcx, init_args); + EarlyBinder::bind(init_typeck.node_args(init_expr.hir_id)).instantiate(cx.tcx, init_args); match init_typeck.qpath_res(init_path, init_expr.hir_id) { Res::Def(DefKind::Ctor(..), _) => return None, Res::Def(DefKind::Const | DefKind::AssocConst, did) - if let Ok(val) = tcx.const_eval_resolve( + if let Ok(val) = cx.tcx.const_eval_resolve( typing_env, UnevaluatedConst::new(did, next_init_args), DUMMY_SP, ) && let Ok(res) = - self.is_non_freeze_val_borrowed(tcx, typing_env, init_typeck, src_expr, val) => + self.is_non_freeze_val_borrowed(cx, typing_env, init_typeck, src_expr, val) => { return res; }, Res::Def(DefKind::Const | DefKind::AssocConst, did) if let Some((next_typeck, value)) = - get_const_hir_value(tcx, typing_env, did, next_init_args) => + get_const_hir_value(cx.tcx, typing_env, did, next_init_args) => { init_typeck = next_typeck; init_args = next_init_args; @@ -607,7 +608,7 @@ impl<'tcx> NonCopyConst<'tcx> { // There's no more that we can read from the init expression. Switch to a // type based check. _ => { - return self.is_non_freeze_expr_borrowed(tcx, typing_env, typeck, src_expr); + return self.is_non_freeze_expr_borrowed(cx, typing_env, typeck, src_expr); }, } }, @@ -619,8 +620,8 @@ impl<'tcx> NonCopyConst<'tcx> { // gets cached. let ty = typeck.expr_ty(src_expr); // Normalized as we need to check if this is an array later. - let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); - if self.is_ty_freeze(tcx, typing_env, ty).is_freeze() { + let ty = cx.tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); + if self.is_ty_freeze(cx, typing_env, ty).is_freeze() { return None; } @@ -653,11 +654,12 @@ impl<'tcx> NonCopyConst<'tcx> { arg }, // Revert to a type based check as we don't know the field's value. - _ => return self.is_non_freeze_expr_borrowed(tcx, typing_env, typeck, use_expr), + _ => return self.is_non_freeze_expr_borrowed(cx, typing_env, typeck, use_expr), }, ExprKind::Index(_, idx, _) if ty.is_array() => match init_expr.kind { ExprKind::Array(fields) => { - if let Some(Constant::Int(idx)) = ConstEvalCtxt::with_env(tcx, typing_env, typeck).eval(idx) { + if let Some(Constant::Int(idx)) = ConstEvalCtxt::with_env(cx.tcx, typing_env, typeck).eval(idx) + { // If the index is out of bounds it means the code // unconditionally panics. In that case there is no borrow. fields.get(idx as usize)? @@ -665,7 +667,7 @@ impl<'tcx> NonCopyConst<'tcx> { // Unknown index, just run the check for all values. return fields.iter().find_map(|f| { self.is_non_freeze_init_borrowed( - tcx, + cx, typing_env, typeck, use_expr, @@ -678,12 +680,12 @@ impl<'tcx> NonCopyConst<'tcx> { }, // Just assume the index expression doesn't panic here. ExprKind::Repeat(field, _) => field, - _ => return self.is_non_freeze_expr_borrowed(tcx, typing_env, typeck, use_expr), + _ => return self.is_non_freeze_expr_borrowed(cx, typing_env, typeck, use_expr), }, ExprKind::AddrOf(..) - if !self.is_init_expr_freeze(tcx, typing_env, init_typeck, init_args, init_expr) => + if !self.is_init_expr_freeze(cx, typing_env, init_typeck, init_args, init_expr) => { - return Some(BorrowSource::new(tcx, use_expr, BorrowCause::Borrow)); + return Some(BorrowSource::new(cx.tcx, use_expr, BorrowCause::Borrow)); }, // All other expressions use the value. _ => return None, @@ -698,13 +700,13 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> { if let ItemKind::Const(ident, .., body_id) = item.kind && !ident.is_special() && let ty = cx.tcx.type_of(item.owner_id).instantiate_identity() - && match self.is_ty_freeze(cx.tcx, cx.typing_env(), ty) { + && match self.is_ty_freeze(cx, cx.typing_env(), ty) { IsFreeze::No => true, IsFreeze::Yes => false, IsFreeze::Maybe => match cx.tcx.const_eval_poly(item.owner_id.to_def_id()) { - Ok(val) if let Ok(is_freeze) = self.is_value_freeze(cx.tcx, cx.typing_env(), ty, val) => !is_freeze, + Ok(val) if let Ok(is_freeze) = self.is_value_freeze(cx, cx.typing_env(), ty, val) => !is_freeze, _ => !self.is_init_expr_freeze( - cx.tcx, + cx, cx.typing_env(), cx.tcx.typeck(item.owner_id), GenericArgs::identity_for_item(cx.tcx, item.owner_id), @@ -738,15 +740,13 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> { fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) { if let TraitItemKind::Const(_, body_id_opt) = item.kind && let ty = cx.tcx.type_of(item.owner_id).instantiate_identity() - && match self.is_ty_freeze(cx.tcx, cx.typing_env(), ty) { + && match self.is_ty_freeze(cx, cx.typing_env(), ty) { IsFreeze::No => true, IsFreeze::Maybe if let Some(body_id) = body_id_opt => { match cx.tcx.const_eval_poly(item.owner_id.to_def_id()) { - Ok(val) if let Ok(is_freeze) = self.is_value_freeze(cx.tcx, cx.typing_env(), ty, val) => { - !is_freeze - }, + Ok(val) if let Ok(is_freeze) = self.is_value_freeze(cx, cx.typing_env(), ty, val) => !is_freeze, _ => !self.is_init_expr_freeze( - cx.tcx, + cx, cx.typing_env(), cx.tcx.typeck(item.owner_id), GenericArgs::identity_for_item(cx.tcx, item.owner_id), @@ -770,7 +770,7 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { if let ImplItemKind::Const(_, body_id) = item.kind && let ty = cx.tcx.type_of(item.owner_id).instantiate_identity() - && match self.is_ty_freeze(cx.tcx, cx.typing_env(), ty) { + && match self.is_ty_freeze(cx, cx.typing_env(), ty) { IsFreeze::Yes => false, IsFreeze::No => { // If this is a trait impl, check if the trait definition is the source @@ -790,7 +790,7 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> { }) .fold_ty(cx.tcx.type_of(item.owner_id).instantiate_identity()); // `ty` may not be normalizable, but that should be fine. - !self.is_ty_freeze(cx.tcx, cx.typing_env(), ty).is_not_freeze() + !self.is_ty_freeze(cx, cx.typing_env(), ty).is_not_freeze() } else { true } @@ -798,9 +798,9 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> { // Even if this is from a trait, there are values which don't have // interior mutability. IsFreeze::Maybe => match cx.tcx.const_eval_poly(item.owner_id.to_def_id()) { - Ok(val) if let Ok(is_freeze) = self.is_value_freeze(cx.tcx, cx.typing_env(), ty, val) => !is_freeze, + Ok(val) if let Ok(is_freeze) = self.is_value_freeze(cx, cx.typing_env(), ty, val) => !is_freeze, _ => !self.is_init_expr_freeze( - cx.tcx, + cx, cx.typing_env(), cx.tcx.typeck(item.owner_id), GenericArgs::identity_for_item(cx.tcx, item.owner_id), @@ -825,22 +825,22 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> { && let Res::Def(DefKind::Const | DefKind::AssocConst, did) = typeck.qpath_res(qpath, e.hir_id) // As of `1.80` constant contexts can't borrow any type with interior mutability && !is_in_const_context(cx) - && !self.is_ty_freeze(cx.tcx, cx.typing_env(), typeck.expr_ty(e)).is_freeze() + && !self.is_ty_freeze(cx, cx.typing_env(), typeck.expr_ty(e)).is_freeze() && let Some(borrow_src) = { // The extra block helps formatting a lot. if let Ok(val) = cx.tcx.const_eval_resolve( cx.typing_env(), UnevaluatedConst::new(did, typeck.node_args(e.hir_id)), DUMMY_SP, - ) && let Ok(src) = self.is_non_freeze_val_borrowed(cx.tcx, cx.typing_env(), typeck, e, val) + ) && let Ok(src) = self.is_non_freeze_val_borrowed(cx, cx.typing_env(), typeck, e, val) { src } else if let init_args = typeck.node_args(e.hir_id) && let Some((init_typeck, init)) = get_const_hir_value(cx.tcx, cx.typing_env(), did, init_args) { - self.is_non_freeze_init_borrowed(cx.tcx, cx.typing_env(), typeck, e, init_typeck, init_args, init) + self.is_non_freeze_init_borrowed(cx, cx.typing_env(), typeck, e, init_typeck, init_args, init) } else { - self.is_non_freeze_expr_borrowed(cx.tcx, cx.typing_env(), typeck, e) + self.is_non_freeze_expr_borrowed(cx, cx.typing_env(), typeck, e) } } && !borrow_src.expr.span.in_external_macro(cx.sess().source_map()) From 969e771c4c372fbb3a3cccbca3a165f99e74302e Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Wed, 8 Oct 2025 00:51:09 +0200 Subject: [PATCH 7/8] non_copy_const: use `InteriorMut` --- clippy_lints/src/non_copy_const.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 4f4c57d87a0b..4158afc22f16 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -22,11 +22,10 @@ use clippy_utils::consts::{ConstEvalCtxt, Constant}; use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; use clippy_utils::is_in_const_context; use clippy_utils::macros::macro_backtrace; -use clippy_utils::paths::{PathNS, lookup_path_str}; -use clippy_utils::ty::{get_field_idx_by_name, implements_trait}; +use clippy_utils::ty::{InteriorMut, get_field_idx_by_name, implements_trait}; use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::{DefId, DefIdSet}; +use rustc_hir::def_id::DefId; use rustc_hir::{ Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, Node, StructTailExpr, TraitItem, TraitItemKind, UnOp, }; @@ -250,7 +249,7 @@ impl<'tcx> BorrowSource<'tcx> { } pub struct NonCopyConst<'tcx> { - ignore_tys: DefIdSet, + interior_mut: InteriorMut<'tcx>, // Cache checked types. We can recurse through a type multiple times so this // can be hit quite frequently. freeze_tys: FxHashMap, IsFreeze>, @@ -261,11 +260,7 @@ impl_lint_pass!(NonCopyConst<'_> => [DECLARE_INTERIOR_MUTABLE_CONST, BORROW_INTE impl<'tcx> NonCopyConst<'tcx> { pub fn new(tcx: TyCtxt<'tcx>, conf: &'static Conf) -> Self { Self { - ignore_tys: conf - .ignore_interior_mutability - .iter() - .flat_map(|ignored_ty| lookup_path_str(tcx, PathNS::Type, ignored_ty)) - .collect(), + interior_mut: InteriorMut::new(tcx, &conf.ignore_interior_mutability), freeze_tys: FxHashMap::default(), } } @@ -285,7 +280,7 @@ impl<'tcx> NonCopyConst<'tcx> { *e = IsFreeze::No; return IsFreeze::No; }, - ty::Adt(adt, _) if self.ignore_tys.contains(&adt.did()) => return IsFreeze::Yes, + ty::Adt(_, _) if !self.interior_mut.is_interior_mut_ty(cx, ty) => return IsFreeze::Yes, ty::Adt(adt, args) if adt.is_enum() => IsFreeze::from_variants(adt.variants().iter().map(|v| { IsFreeze::from_fields( v.fields From 108709583550dbbf73ba7402ca82057604cc6205 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Wed, 8 Oct 2025 00:50:06 +0200 Subject: [PATCH 8/8] introduce `#[clippy::ignore_interior_mutability]` --- clippy_lints/src/non_copy_const.rs | 10 ++++--- clippy_utils/src/attrs.rs | 1 + clippy_utils/src/sym.rs | 1 + clippy_utils/src/ty/mod.rs | 15 ++++++++-- tests/ui/borrow_interior_mutable_const.rs | 7 +++++ tests/ui/declare_interior_mutable_const.rs | 6 ++++ tests/ui/ifs_same_cond.rs | 34 +++++++++++++++++++++- tests/ui/ifs_same_cond.stderr | 21 +++++++++---- tests/ui/mut_key.rs | 11 +++++++ 9 files changed, 93 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 4158afc22f16..994787c693c2 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -70,8 +70,9 @@ declare_clippy_lint! { /// no way to both create a value as a constant and modify any mutable field using the /// type's public interface (e.g. `bytes::Bytes`). As there is no reasonable way to /// scan a crate's interface to see if this is the case, all such types will be linted. - /// If this happens use the `ignore-interior-mutability` configuration option to allow - /// the type. + /// If this happens, use the `ignore-interior-mutability` configuration option or put + /// the `#[clippy::ignore_interior_mutability]` attribute onto a type to allow the + /// type. /// /// ### Example /// ```no_run @@ -131,8 +132,9 @@ declare_clippy_lint! { /// no way to both create a value as a constant and modify any mutable field using the /// type's public interface (e.g. `bytes::Bytes`). As there is no reasonable way to /// scan a crate's interface to see if this is the case, all such types will be linted. - /// If this happens use the `ignore-interior-mutability` configuration option to allow - /// the type. + /// If this happens, use the `ignore-interior-mutability` configuration option or put + /// the `#[clippy::ignore_interior_mutability]` attribute onto a type to allow the + /// type. /// /// ### Example /// ```no_run diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 671b266ba008..052bafcf620e 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -28,6 +28,7 @@ pub fn get_builtin_attr<'a, A: AttributeExt + 'a>( sym::cyclomatic_complexity => Some("cognitive_complexity"), sym::author | sym::version + | sym::ignore_interior_mutability | sym::cognitive_complexity | sym::dump | sym::msrv diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index 2b22f344e8c0..a9c6ce2cd376 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -175,6 +175,7 @@ generate! { has_significant_drop, hidden_glob_reexports, hygiene, + ignore_interior_mutability, insert, insert_str, inspect, diff --git a/clippy_utils/src/ty/mod.rs b/clippy_utils/src/ty/mod.rs index 554033f92f48..28f08008912d 100644 --- a/clippy_utils/src/ty/mod.rs +++ b/clippy_utils/src/ty/mod.rs @@ -14,7 +14,7 @@ use rustc_hir::def_id::DefId; use rustc_hir::{Expr, FnDecl, LangItem, find_attr}; use rustc_hir_analysis::lower_ty; use rustc_infer::infer::TyCtxtInferExt; -use rustc_lint::LateContext; +use rustc_lint::{LateContext, LintContext}; use rustc_middle::mir::ConstValue; use rustc_middle::mir::interpret::Scalar; use rustc_middle::traits::EvaluationResult; @@ -36,7 +36,7 @@ use std::{iter, mem}; use crate::paths::{PathNS, lookup_path_str}; use crate::res::{MaybeDef, MaybeQPath}; -use crate::sym; +use crate::{get_unique_builtin_attr, sym}; mod type_certainty; pub use type_certainty::expr_type_is_certain; @@ -1071,6 +1071,17 @@ impl<'tcx> InteriorMut<'tcx> { /// this type to be interior mutable. False negatives may be expected for infinitely recursive /// types, and `None` will be returned there. pub fn interior_mut_ty_chain(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'tcx ty::List>> { + // Check if given type has a `#[clippy::ignore_interior_mutability]` attribute + if let Some(did) = ty.ty_adt_def().map(AdtDef::did) + && !self.ignored_def_ids.contains(&did) + && let attrs = cx.tcx.get_all_attrs(did) + && get_unique_builtin_attr(cx.sess(), attrs, sym::ignore_interior_mutability).is_some() + { + self.ignored_def_ids.insert(did); + // Small optimization: since we know that we're going to ignore interior mutability for + // this type anyway, running `self.interior_mut_ty_chain_inner` is unnecessary + return None; + } self.interior_mut_ty_chain_inner(cx, ty, 0) } diff --git a/tests/ui/borrow_interior_mutable_const.rs b/tests/ui/borrow_interior_mutable_const.rs index 674450a73ad2..1769f31d3959 100644 --- a/tests/ui/borrow_interior_mutable_const.rs +++ b/tests/ui/borrow_interior_mutable_const.rs @@ -234,4 +234,11 @@ fn main() { } } } + fn issue13865() { + #[clippy::ignore_interior_mutability] + struct Foo(Cell); + + const FOO: Foo = Foo(Cell::new(0)); + let _ = &FOO; + } } diff --git a/tests/ui/declare_interior_mutable_const.rs b/tests/ui/declare_interior_mutable_const.rs index c65df275038d..f9f7502573c4 100644 --- a/tests/ui/declare_interior_mutable_const.rs +++ b/tests/ui/declare_interior_mutable_const.rs @@ -198,3 +198,9 @@ impl WithGenericAssocCell for i32 { thread_local!(static THREAD_LOCAL_CELL: Cell = const { Cell::new(0) }); thread_local!(static THREAD_LOCAL_CELL2: Cell = Cell::new(0)); + +fn issue13865() { + #[clippy::ignore_interior_mutability] + struct IgnoreInteriorMut(Cell); + const IIM: IgnoreInteriorMut = IgnoreInteriorMut(Cell::new(0)); +} diff --git a/tests/ui/ifs_same_cond.rs b/tests/ui/ifs_same_cond.rs index 7067434953d6..81b641af262a 100644 --- a/tests/ui/ifs_same_cond.rs +++ b/tests/ui/ifs_same_cond.rs @@ -1,6 +1,8 @@ #![warn(clippy::ifs_same_cond)] #![allow(clippy::if_same_then_else, clippy::needless_if, clippy::needless_else)] // all empty blocks +use std::cell::Cell; + fn ifs_same_cond() { let a = 0; let b = false; @@ -72,7 +74,7 @@ fn issue10272() { } else { } - let x = std::cell::Cell::new(true); + let x = Cell::new(true); if x.get() { } else if !x.take() { } else if x.get() { @@ -81,4 +83,34 @@ fn issue10272() { } } +fn issue13865() { + #[clippy::ignore_interior_mutability] + struct X(Cell); + + // delegate some methods so that the test case works + impl X { + fn get(&self) -> T + where + T: Copy, + { + self.0.get() + } + fn take(&self) -> T + where + T: Default, + { + self.0.take() + } + } + + let x = X(Cell::new(true)); + if x.get() { + //~^ ifs_same_cond + // x is interior mutable type, but that should be ignored, so we lint + } else if !x.take() { + } else if x.get() { + } else { + } +} + fn main() {} diff --git a/tests/ui/ifs_same_cond.stderr b/tests/ui/ifs_same_cond.stderr index 7acbc1a6399b..bf41ed9bae1f 100644 --- a/tests/ui/ifs_same_cond.stderr +++ b/tests/ui/ifs_same_cond.stderr @@ -1,5 +1,5 @@ error: these `if` branches have the same condition - --> tests/ui/ifs_same_cond.rs:8:8 + --> tests/ui/ifs_same_cond.rs:10:8 | LL | if b { | ^ @@ -11,7 +11,7 @@ LL | } else if b { = help: to override `-D warnings` add `#[allow(clippy::ifs_same_cond)]` error: these `if` branches have the same condition - --> tests/ui/ifs_same_cond.rs:13:8 + --> tests/ui/ifs_same_cond.rs:15:8 | LL | if b { | ^ @@ -22,7 +22,7 @@ LL | } else if b { | ^ error: these `if` branches have the same condition - --> tests/ui/ifs_same_cond.rs:19:8 + --> tests/ui/ifs_same_cond.rs:21:8 | LL | if a == 1 { | ^^^^^^ @@ -31,7 +31,7 @@ LL | } else if a == 1 { | ^^^^^^ error: these `if` branches have the same condition - --> tests/ui/ifs_same_cond.rs:24:8 + --> tests/ui/ifs_same_cond.rs:26:8 | LL | if 2 * a == 1 { | ^^^^^^^^^^ @@ -40,7 +40,7 @@ LL | } else if 2 * a == 1 { | ^^^^^^^^^^ error: these `if` branches have the same condition - --> tests/ui/ifs_same_cond.rs:58:8 + --> tests/ui/ifs_same_cond.rs:60:8 | LL | if a.contains("ah") { | ^^^^^^^^^^^^^^^^ @@ -48,5 +48,14 @@ LL | LL | } else if a.contains("ah") { | ^^^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: these `if` branches have the same condition + --> tests/ui/ifs_same_cond.rs:107:8 + | +LL | if x.get() { + | ^^^^^^^ +... +LL | } else if x.get() { + | ^^^^^^^ + +error: aborting due to 6 previous errors diff --git a/tests/ui/mut_key.rs b/tests/ui/mut_key.rs index 29dc2d020132..fcf2b6faf110 100644 --- a/tests/ui/mut_key.rs +++ b/tests/ui/mut_key.rs @@ -113,3 +113,14 @@ fn main() { let _map = HashMap::<&mut usize, usize>::new(); let _map = HashMap::, usize>::new(); } + +fn issue13865() { + #[clippy::ignore_interior_mutability] + struct IgnoreMut { + cell: std::cell::Cell<&'static [u32]>, + } + + fn main() { + let a: HashSet = HashSet::new(); + } +}