Fix clippy

When mgca is enabled, const rhs's that are paths may have false
negatives with the lints in non_copy_const.rs. But these should probably
be using the trait solver anyway, and it only happens under mgca.
This commit is contained in:
Noah Lev 2025-11-01 13:52:11 -04:00
parent f532622aa8
commit 91054e8042
14 changed files with 157 additions and 75 deletions

View file

@ -18,7 +18,7 @@
// definitely contains interior mutability.
use clippy_config::Conf;
use clippy_utils::consts::{ConstEvalCtxt, Constant};
use clippy_utils::consts::{ConstEvalCtxt, Constant, const_item_rhs_to_expr};
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::is_in_const_context;
use clippy_utils::macros::macro_backtrace;
@ -28,7 +28,8 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, DefIdSet};
use rustc_hir::{
Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, Node, StructTailExpr, TraitItem, TraitItemKind, UnOp,
ConstArgKind, ConstItemRhs, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, Node, StructTailExpr,
TraitItem, TraitItemKind, UnOp,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::mir::{ConstValue, UnevaluatedConst};
@ -272,6 +273,7 @@ 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 {
// FIXME: this should probably be using the trait solver
let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty);
match self.freeze_tys.entry(ty) {
Entry::Occupied(e) => *e.get(),
@ -695,7 +697,7 @@ impl<'tcx> NonCopyConst<'tcx> {
impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
if let ItemKind::Const(ident, .., body_id) = item.kind
if let ItemKind::Const(ident, .., ct_rhs) = 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) {
@ -703,13 +705,14 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> {
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,
_ => !self.is_init_expr_freeze(
// FIXME: we just assume mgca rhs's are freeze
_ => const_item_rhs_to_expr(cx.tcx, ct_rhs).map_or(false, |e| !self.is_init_expr_freeze(
cx.tcx,
cx.typing_env(),
cx.tcx.typeck(item.owner_id),
GenericArgs::identity_for_item(cx.tcx, item.owner_id),
cx.tcx.hir_body(body_id).value,
),
e
)),
},
}
&& !item.span.in_external_macro(cx.sess().source_map())
@ -736,22 +739,25 @@ 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
if let TraitItemKind::Const(_, ct_rhs_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) {
IsFreeze::No => true,
IsFreeze::Maybe if let Some(body_id) = body_id_opt => {
IsFreeze::Maybe if let Some(ct_rhs) = ct_rhs_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
},
_ => !self.is_init_expr_freeze(
cx.tcx,
cx.typing_env(),
cx.tcx.typeck(item.owner_id),
GenericArgs::identity_for_item(cx.tcx, item.owner_id),
cx.tcx.hir_body(body_id).value,
),
// FIXME: we just assume mgca rhs's are freeze
_ => const_item_rhs_to_expr(cx.tcx, ct_rhs).map_or(false, |e| {
!self.is_init_expr_freeze(
cx.tcx,
cx.typing_env(),
cx.tcx.typeck(item.owner_id),
GenericArgs::identity_for_item(cx.tcx, item.owner_id),
e,
)
}),
}
},
IsFreeze::Yes | IsFreeze::Maybe => false,
@ -768,7 +774,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
if let ImplItemKind::Const(_, ct_rhs) = item.kind
&& let ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
&& match self.is_ty_freeze(cx.tcx, cx.typing_env(), ty) {
IsFreeze::Yes => false,
@ -799,13 +805,16 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> {
// 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,
_ => !self.is_init_expr_freeze(
cx.tcx,
cx.typing_env(),
cx.tcx.typeck(item.owner_id),
GenericArgs::identity_for_item(cx.tcx, item.owner_id),
cx.tcx.hir_body(body_id).value,
),
// FIXME: we just assume mgca rhs's are freeze
_ => const_item_rhs_to_expr(cx.tcx, ct_rhs).map_or(false, |e| {
!self.is_init_expr_freeze(
cx.tcx,
cx.typing_env(),
cx.tcx.typeck(item.owner_id),
GenericArgs::identity_for_item(cx.tcx, item.owner_id),
e,
)
}),
},
}
&& !item.span.in_external_macro(cx.sess().source_map())
@ -913,20 +922,26 @@ fn get_const_hir_value<'tcx>(
args: GenericArgsRef<'tcx>,
) -> Option<(&'tcx TypeckResults<'tcx>, &'tcx Expr<'tcx>)> {
let did = did.as_local()?;
let (did, body_id) = match tcx.hir_node(tcx.local_def_id_to_hir_id(did)) {
Node::Item(item) if let ItemKind::Const(.., body_id) = item.kind => (did, body_id),
Node::ImplItem(item) if let ImplItemKind::Const(.., body_id) = item.kind => (did, body_id),
let (did, ct_rhs) = match tcx.hir_node(tcx.local_def_id_to_hir_id(did)) {
Node::Item(item) if let ItemKind::Const(.., ct_rhs) = item.kind => (did, ct_rhs),
Node::ImplItem(item) if let ImplItemKind::Const(.., ct_rhs) = item.kind => (did, ct_rhs),
Node::TraitItem(_)
if let Ok(Some(inst)) = Instance::try_resolve(tcx, typing_env, did.into(), args)
&& let Some(did) = inst.def_id().as_local() =>
{
match tcx.hir_node(tcx.local_def_id_to_hir_id(did)) {
Node::ImplItem(item) if let ImplItemKind::Const(.., body_id) = item.kind => (did, body_id),
Node::TraitItem(item) if let TraitItemKind::Const(.., Some(body_id)) = item.kind => (did, body_id),
Node::ImplItem(item) if let ImplItemKind::Const(.., ct_rhs) = item.kind => (did, ct_rhs),
Node::TraitItem(item) if let TraitItemKind::Const(.., Some(ct_rhs)) = item.kind => (did, ct_rhs),
_ => return None,
}
},
_ => return None,
};
Some((tcx.typeck(did), tcx.hir_body(body_id).value))
match ct_rhs {
ConstItemRhs::Body(body_id) => Some((tcx.typeck(did), tcx.hir_body(body_id).value)),
ConstItemRhs::TypeConst(ct_arg) => match ct_arg.kind {
ConstArgKind::Anon(anon_const) => Some((tcx.typeck(did), tcx.hir_body(anon_const.body).value)),
_ => None,
},
}
}

View file

@ -2,6 +2,7 @@ use std::ops::ControlFlow;
use std::sync::Arc;
use clippy_config::Conf;
use clippy_utils::consts::const_item_rhs_to_expr;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_lint_allowed;
use clippy_utils::source::walk_span_to_context;
@ -184,7 +185,7 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
}
}
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &hir::Item<'tcx>) {
if item.span.in_external_macro(cx.tcx.sess.source_map()) {
return;
}
@ -214,7 +215,7 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
}
}
fn check_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>, (span, help_span): (Span, Span), is_doc: bool) {
fn check_has_safety_comment<'tcx>(cx: &LateContext<'tcx>, item: &hir::Item<'tcx>, (span, help_span): (Span, Span), is_doc: bool) {
match &item.kind {
ItemKind::Impl(Impl {
of_trait: Some(of_trait),
@ -234,7 +235,29 @@ fn check_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>, (span, h
},
ItemKind::Impl(_) => {},
// const and static items only need a safety comment if their body is an unsafe block, lint otherwise
&ItemKind::Const(.., body) | &ItemKind::Static(.., body) => {
&ItemKind::Const(.., ct_rhs) => {
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, ct_rhs.hir_id()) {
let expr = const_item_rhs_to_expr(cx.tcx, ct_rhs);
if let Some(expr) = expr && !matches!(
expr.kind, hir::ExprKind::Block(block, _)
if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
) {
span_lint_and_then(
cx,
UNNECESSARY_SAFETY_COMMENT,
span,
format!(
"{} has unnecessary safety comment",
cx.tcx.def_descr(item.owner_id.to_def_id()),
),
|diag| {
diag.span_help(help_span, "consider removing the safety comment");
},
);
}
}
}
&ItemKind::Static(.., body) => {
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, body.hir_id) {
let body = cx.tcx.hir_body(body);
if !matches!(

View file

@ -320,6 +320,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
self.body(field!(anon_const.body));
},
ConstArgKind::Infer(..) => chain!(self, "let ConstArgKind::Infer(..) = {const_arg}.kind"),
ConstArgKind::Error(..) => chain!(self, "let ConstArgKind::Error(..) = {const_arg}.kind"),
}
}

View file

@ -374,7 +374,7 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool {
&& eq_id(*li, *ri)
&& eq_generics(lg, rg)
&& eq_ty(lt, rt)
&& both(lb.as_deref(), rb.as_deref(), |l, r| eq_anon_const(l, r))
&& both(lb.as_ref(), rb.as_ref(), |l, r| eq_const_item_rhs(l, r))
},
(
Fn(box ast::Fn {
@ -628,7 +628,7 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool {
&& eq_id(*li, *ri)
&& eq_generics(lg, rg)
&& eq_ty(lt, rt)
&& both(lb.as_deref(), rb.as_deref(), |l, r| eq_anon_const(l, r))
&& both(lb.as_ref(), rb.as_ref(), |l, r| eq_const_item_rhs(l, r))
},
(
Fn(box ast::Fn {
@ -786,6 +786,15 @@ pub fn eq_anon_const(l: &AnonConst, r: &AnonConst) -> bool {
eq_expr(&l.value, &r.value)
}
pub fn eq_const_item_rhs(l: &ConstItemRhs, r: &ConstItemRhs) -> bool {
use ConstItemRhs::*;
match (l, r) {
(TypeConst(l), TypeConst(r)) => eq_anon_const(l, r),
(Body(l), Body(r)) => eq_expr(l, r),
(TypeConst(..), Body(..)) | (Body(..), TypeConst(..)) => false,
}
}
pub fn eq_use_tree_kind(l: &UseTreeKind, r: &UseTreeKind) -> bool {
use UseTreeKind::*;
match (l, r) {

View file

@ -13,7 +13,10 @@ use rustc_apfloat::Float;
use rustc_apfloat::ieee::{Half, Quad};
use rustc_ast::ast::{LitFloatType, LitKind};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BinOpKind, Block, ConstBlock, Expr, ExprKind, HirId, PatExpr, PatExprKind, QPath, TyKind, UnOp};
use rustc_hir::{
BinOpKind, Block, ConstArgKind, ConstBlock, ConstItemRhs, Expr, ExprKind, HirId, PatExpr, PatExprKind, QPath,
TyKind, UnOp,
};
use rustc_lexer::{FrontmatterAllowed, tokenize};
use rustc_lint::LateContext;
use rustc_middle::mir::ConstValue;
@ -1130,3 +1133,14 @@ pub fn integer_const(cx: &LateContext<'_>, expr: &Expr<'_>, ctxt: SyntaxContext)
pub fn is_zero_integer_const(cx: &LateContext<'_>, expr: &Expr<'_>, ctxt: SyntaxContext) -> bool {
integer_const(cx, expr, ctxt) == Some(0)
}
pub fn const_item_rhs_to_expr<'tcx>(tcx: TyCtxt<'tcx>, ct_rhs: ConstItemRhs<'tcx>) -> Option<&'tcx Expr<'tcx>> {
match ct_rhs {
ConstItemRhs::Body(body_id) => Some(tcx.hir_body(body_id).value),
ConstItemRhs::TypeConst(const_arg) => match const_arg.kind {
ConstArgKind::Path(_) => None,
ConstArgKind::Anon(anon) => Some(tcx.hir_body(anon.body).value),
ConstArgKind::Error(..) | ConstArgKind::Infer(..) => None,
},
}
}

View file

@ -481,7 +481,9 @@ impl HirEqInterExpr<'_, '_, '_> {
(ConstArgKind::Path(..), ConstArgKind::Anon(..))
| (ConstArgKind::Anon(..), ConstArgKind::Path(..))
| (ConstArgKind::Infer(..), _)
| (_, ConstArgKind::Infer(..)) => false,
| (_, ConstArgKind::Infer(..))
| (ConstArgKind::Error(..), _)
| (_, ConstArgKind::Error(..)) => false,
}
}
@ -1330,7 +1332,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
match &const_arg.kind {
ConstArgKind::Path(path) => self.hash_qpath(path),
ConstArgKind::Anon(anon) => self.hash_body(anon.body),
ConstArgKind::Infer(..) => {},
ConstArgKind::Infer(..) | ConstArgKind::Error(..) => {},
}
}

View file

@ -192,12 +192,12 @@ fn parse_attrs(sess: &Session, attrs: &[impl AttributeExt]) -> Option<RustcVersi
let msrv_attr = msrv_attrs.next()?;
if let Some(duplicate) = msrv_attrs.next_back() {
sess.dcx()
.struct_span_err(duplicate.span(), "`clippy::msrv` is defined multiple times")
.with_span_note(msrv_attr.span(), "first definition found here")
.emit();
}
if let Some(duplicate) = msrv_attrs.next_back() {
sess.dcx()
.struct_span_err(duplicate.span(), "`clippy::msrv` is defined multiple times")
.with_span_note(msrv_attr.span(), "first definition found here")
.emit();
}
let Some(msrv) = msrv_attr.value_str() else {
sess.dcx().span_err(msrv_attr.span(), "bad clippy attribute");
@ -205,8 +205,8 @@ fn parse_attrs(sess: &Session, attrs: &[impl AttributeExt]) -> Option<RustcVersi
};
let Some(version) = parse_version(msrv) else {
sess.dcx()
.span_err(msrv_attr.span(), format!("`{msrv}` is not a valid Rust version"));
sess.dcx()
.span_err(msrv_attr.span(), format!("`{msrv}` is not a valid Rust version"));
return None;
};

View file

@ -9,7 +9,7 @@
/// unimplemented!();
/// }
/// ```
///
///
/// With an explicit return type it should lint too
/// ```edition2015
/// fn main() -> () {
@ -17,7 +17,7 @@
/// unimplemented!();
/// }
/// ```
///
///
/// This should, too.
/// ```rust
/// fn main() {
@ -25,7 +25,7 @@
/// unimplemented!();
/// }
/// ```
///
///
/// This one too.
/// ```no_run
/// // the fn is not always the first line

View file

@ -1,6 +1,6 @@
#![deny(clippy::trait_duplication_in_bounds)]
#![allow(unused)]
#![feature(associated_const_equality, const_trait_impl)]
#![feature(const_trait_impl)]
use std::any::Any;
@ -194,12 +194,3 @@ where
T: Iterator<Item: Clone> + Iterator<Item: Clone>,
{
}
trait AssocConstTrait {
const ASSOC: usize;
}
fn assoc_const_args<T>()
where
T: AssocConstTrait<ASSOC = 0>,
//~^ trait_duplication_in_bounds
{
}

View file

@ -1,6 +1,6 @@
#![deny(clippy::trait_duplication_in_bounds)]
#![allow(unused)]
#![feature(associated_const_equality, const_trait_impl)]
#![feature(const_trait_impl)]
use std::any::Any;
@ -194,12 +194,3 @@ where
T: Iterator<Item: Clone> + Iterator<Item: Clone>,
{
}
trait AssocConstTrait {
const ASSOC: usize;
}
fn assoc_const_args<T>()
where
T: AssocConstTrait<ASSOC = 0> + AssocConstTrait<ASSOC = 0>,
//~^ trait_duplication_in_bounds
{
}

View file

@ -70,11 +70,5 @@ error: these where clauses contain repeated elements
LL | T: IntoIterator<Item = U::Owned> + IntoIterator<Item = U::Owned>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `IntoIterator<Item = U::Owned>`
error: these where clauses contain repeated elements
--> tests/ui/trait_duplication_in_bounds.rs:202:8
|
LL | T: AssocConstTrait<ASSOC = 0> + AssocConstTrait<ASSOC = 0>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `AssocConstTrait<ASSOC = 0>`
error: aborting due to 12 previous errors
error: aborting due to 11 previous errors

View file

@ -0,0 +1,14 @@
#![deny(clippy::trait_duplication_in_bounds)]
#![expect(incomplete_features)]
#![feature(associated_const_equality, min_generic_const_args)]
trait AssocConstTrait {
#[type_const]
const ASSOC: usize;
}
fn assoc_const_args<T>()
where
T: AssocConstTrait<ASSOC = 0>,
//~^ trait_duplication_in_bounds
{
}

View file

@ -0,0 +1,14 @@
#![deny(clippy::trait_duplication_in_bounds)]
#![expect(incomplete_features)]
#![feature(associated_const_equality, min_generic_const_args)]
trait AssocConstTrait {
#[type_const]
const ASSOC: usize;
}
fn assoc_const_args<T>()
where
T: AssocConstTrait<ASSOC = 0> + AssocConstTrait<ASSOC = 0>,
//~^ trait_duplication_in_bounds
{
}

View file

@ -0,0 +1,14 @@
error: these where clauses contain repeated elements
--> tests/ui/trait_duplication_in_bounds_assoc_const_eq.rs:11:8
|
LL | T: AssocConstTrait<ASSOC = 0> + AssocConstTrait<ASSOC = 0>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `AssocConstTrait<ASSOC = 0>`
|
note: the lint level is defined here
--> tests/ui/trait_duplication_in_bounds_assoc_const_eq.rs:1:9
|
LL | #![deny(clippy::trait_duplication_in_bounds)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 1 previous error