From a19dfe64f8b21da37c19dddea722d9a92632acc4 Mon Sep 17 00:00:00 2001 From: John Kelly Date: Fri, 28 Apr 2023 14:48:37 +0100 Subject: [PATCH 1/5] Initial impl of `lint_duplicate_constraints` --- compiler/rustc_lint/messages.ftl | 4 ++ compiler/rustc_lint/src/duplicate_trait.rs | 65 +++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 9 +++ .../lint/duplicate-trait/duplicate-trait.rs | 15 +++++ 5 files changed, 96 insertions(+) create mode 100644 compiler/rustc_lint/src/duplicate_trait.rs create mode 100644 tests/ui/lint/duplicate-trait/duplicate-trait.rs diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 3c6dbb466db7a..89c59f184e3a3 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -192,6 +192,10 @@ lint_redundant_semicolons = *[false] this semicolon } +lint_duplicate_trait = + trait `{$trait_name}` was already specified + .suggestion = remove duplicate trait + lint_drop_trait_constraints = bounds on `{$predicate}` are most likely incorrect, consider instead using `{$needs_drop}` to detect whether a type can be trivially dropped diff --git a/compiler/rustc_lint/src/duplicate_trait.rs b/compiler/rustc_lint/src/duplicate_trait.rs new file mode 100644 index 0000000000000..c70145b4d42ba --- /dev/null +++ b/compiler/rustc_lint/src/duplicate_trait.rs @@ -0,0 +1,65 @@ +use crate::hir; + +use crate::{lints::DuplicateTraitDiag, LateContext, LateLintPass}; + +declare_lint! { + /// The `lint_duplicate_trait` lints repetition of traits. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// fn foo(_: &(dyn MyTrait + Send + Send>) {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Duplicate trait `Send` in trait object. + pub DUPLICATE_TRAIT, + Warn, + "duplicate trait constraint in trait object" +} + +declare_lint_pass!(DuplicateTrait => [DUPLICATE_TRAIT]); + +impl<'tcx> LateLintPass<'tcx> for DuplicateTrait { + fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx hir::Ty<'tcx>) { + let hir::TyKind::Ref( + .., + hir::MutTy { + ty: hir::Ty { + kind: hir::TyKind::TraitObject(bounds, ..), + .. + }, + .. + } + ) = ty.kind else { return; }; + + let mut bounds = (*bounds).to_owned(); + + bounds.sort_unstable_by_key(|b| b.trait_ref.trait_def_id()); + + if bounds.len() < 2 { + return; + } + + let mut last_bound = &bounds[0]; + for bound in bounds.iter().skip(1) { + if last_bound.trait_ref.trait_def_id() == bound.trait_ref.trait_def_id() + && let Some(def_id) = last_bound.trait_ref.trait_def_id() { + cx.tcx.emit_spanned_lint( + DUPLICATE_TRAIT, + bound.trait_ref.hir_ref_id, // is this correct? + bound.span, + DuplicateTraitDiag { + trait_name: cx.tcx.item_name(def_id), + suggestion: bound.span + }, + ) + } + + last_bound = bound; + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 319eb2ea445ed..5c41657906884 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -77,6 +77,7 @@ mod redundant_semicolon; mod traits; mod types; mod unused; +mod duplicate_trait; pub use array_into_iter::ARRAY_INTO_ITER; @@ -125,6 +126,7 @@ pub use passes::{EarlyLintPass, LateLintPass}; pub use rustc_session::lint::Level::{self, *}; pub use rustc_session::lint::{BufferedEarlyLint, FutureIncompatibleInfo, Lint, LintId}; pub use rustc_session::lint::{LintArray, LintPass}; +use duplicate_trait::DuplicateTrait; fluent_messages! { "../messages.ftl" } @@ -242,6 +244,7 @@ late_lint_methods!( OpaqueHiddenInferredBound: OpaqueHiddenInferredBound, MultipleSupertraitUpcastable: MultipleSupertraitUpcastable, MapUnitFn: MapUnitFn, + DuplicateTrait: DuplicateTrait, ] ] ); diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 848f6a9ecb532..ab3f70b994086 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1168,6 +1168,15 @@ pub struct RedundantSemicolonsDiag { pub suggestion: Span, } +// duplicate_trait.rs +#[derive(LintDiagnostic)] +#[diag(lint_duplicate_trait)] +pub struct DuplicateTraitDiag { + pub trait_name: Symbol, + #[suggestion(code = "", applicability = "maybe-incorrect")] + pub suggestion: Span, +} + // traits.rs pub struct DropTraitConstraintsDiag<'a> { pub predicate: Predicate<'a>, diff --git a/tests/ui/lint/duplicate-trait/duplicate-trait.rs b/tests/ui/lint/duplicate-trait/duplicate-trait.rs new file mode 100644 index 0000000000000..8b86cee5e36e3 --- /dev/null +++ b/tests/ui/lint/duplicate-trait/duplicate-trait.rs @@ -0,0 +1,15 @@ +#![warn(duplicate_trait)] + +use std::any::Any; + +fn main() {} + +fn fine(_a: &dyn Any + Send) {} + +fn duplicate_once(_a: &(dyn Any + Send + Send)) {} + +fn duplicate_twice(_a: &(dyn Any + Send + Send + Send)) {} + +fn duplicate_out_of_order(_a: &(dyn Any + Send + Sync + Send)) {} + +fn duplicate_multiple(_a: &(dyn Any + Send + Sync + Send + Sync)) {} From afc50bdb12b24feeedf6d258221f6caf52f1d622 Mon Sep 17 00:00:00 2001 From: John Kelly Date: Sat, 29 Apr 2023 12:02:19 +0100 Subject: [PATCH 2/5] tests --- compiler/rustc_lint/src/duplicate_trait.rs | 2 +- .../lint/duplicate-trait/duplicate-trait.rs | 11 ++--- .../duplicate-trait/duplicate-trait.stderr | 43 +++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 tests/ui/lint/duplicate-trait/duplicate-trait.stderr diff --git a/compiler/rustc_lint/src/duplicate_trait.rs b/compiler/rustc_lint/src/duplicate_trait.rs index c70145b4d42ba..0650641e0f244 100644 --- a/compiler/rustc_lint/src/duplicate_trait.rs +++ b/compiler/rustc_lint/src/duplicate_trait.rs @@ -47,7 +47,7 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateTrait { let mut last_bound = &bounds[0]; for bound in bounds.iter().skip(1) { if last_bound.trait_ref.trait_def_id() == bound.trait_ref.trait_def_id() - && let Some(def_id) = last_bound.trait_ref.trait_def_id() { + && let Some(def_id) = bound.trait_ref.trait_def_id() { cx.tcx.emit_spanned_lint( DUPLICATE_TRAIT, bound.trait_ref.hir_ref_id, // is this correct? diff --git a/tests/ui/lint/duplicate-trait/duplicate-trait.rs b/tests/ui/lint/duplicate-trait/duplicate-trait.rs index 8b86cee5e36e3..16b3efe553ece 100644 --- a/tests/ui/lint/duplicate-trait/duplicate-trait.rs +++ b/tests/ui/lint/duplicate-trait/duplicate-trait.rs @@ -1,15 +1,16 @@ +// check-fail #![warn(duplicate_trait)] use std::any::Any; fn main() {} -fn fine(_a: &dyn Any + Send) {} +fn fine(_a: &(dyn Any + Send)) {} -fn duplicate_once(_a: &(dyn Any + Send + Send)) {} +fn duplicate_once(_a: &(dyn Any + Send + Send)) {} //~WARNING duplicate trait -fn duplicate_twice(_a: &(dyn Any + Send + Send + Send)) {} +fn duplicate_twice(_a: &(dyn Any + Send + Send + Send)) {} //~WARNING duplicate trait -fn duplicate_out_of_order(_a: &(dyn Any + Send + Sync + Send)) {} +fn duplicate_out_of_order(_a: &(dyn Any + Send + Sync + Send)) {} //~WARNING duplicate trait -fn duplicate_multiple(_a: &(dyn Any + Send + Sync + Send + Sync)) {} +fn duplicate_multiple(_a: &(dyn Any + Send + Sync + Send + Sync)) {} //~WARNING duplicate trait diff --git a/tests/ui/lint/duplicate-trait/duplicate-trait.stderr b/tests/ui/lint/duplicate-trait/duplicate-trait.stderr new file mode 100644 index 0000000000000..906d123b5b053 --- /dev/null +++ b/tests/ui/lint/duplicate-trait/duplicate-trait.stderr @@ -0,0 +1,43 @@ +warning: trait `Send` was already specified + --> $DIR/duplicate-trait.rs:9:42 + | +LL | fn duplicate_once(_a: &(dyn Any + Send + Send)) {} + | ^^^^ help: remove duplicate trait + | +note: the lint level is defined here + --> $DIR/duplicate-trait.rs:1:9 + | +LL | #![warn(duplicate_trait)] + | ^^^^^^^^^^^^^^^ + +warning: trait `Send` was already specified + --> $DIR/duplicate-trait.rs:11:43 + | +LL | fn duplicate_twice(_a: &(dyn Any + Send + Send + Send)) {} + | ^^^^ help: remove duplicate trait + +warning: trait `Send` was already specified + --> $DIR/duplicate-trait.rs:11:50 + | +LL | fn duplicate_twice(_a: &(dyn Any + Send + Send + Send)) {} + | ^^^^ help: remove duplicate trait + +warning: trait `Send` was already specified + --> $DIR/duplicate-trait.rs:13:57 + | +LL | fn duplicate_out_of_order(_a: &(dyn Any + Send + Sync + Send)) {} + | ^^^^ help: remove duplicate trait + +warning: trait `Send` was already specified + --> $DIR/duplicate-trait.rs:15:53 + | +LL | fn duplicate_multiple(_a: &(dyn Any + Send + Sync + Send + Sync)) {} + | ^^^^ help: remove duplicate trait + +warning: trait `Sync` was already specified + --> $DIR/duplicate-trait.rs:15:60 + | +LL | fn duplicate_multiple(_a: &(dyn Any + Send + Sync + Send + Sync)) {} + | ^^^^ help: remove duplicate trait + +warning: 6 warnings emitted From 495cfca6900d61cc3de84de6f5957e184793c967 Mon Sep 17 00:00:00 2001 From: John Kelly Date: Sat, 29 Apr 2023 13:04:17 +0100 Subject: [PATCH 3/5] fmt --- compiler/rustc_lint/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 5c41657906884..86e9a5c6a0354 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -52,6 +52,7 @@ mod array_into_iter; pub mod builtin; mod context; mod deref_into_dyn_supertrait; +mod duplicate_trait; mod early; mod enum_intrinsics_non_enums; mod errors; @@ -77,7 +78,6 @@ mod redundant_semicolon; mod traits; mod types; mod unused; -mod duplicate_trait; pub use array_into_iter::ARRAY_INTO_ITER; @@ -120,13 +120,13 @@ use unused::*; pub use builtin::SoftLints; pub use context::{CheckLintNameResult, FindLintError, LintStore}; pub use context::{EarlyContext, LateContext, LintContext}; +use duplicate_trait::DuplicateTrait; pub use early::{check_ast_node, EarlyCheckNode}; pub use late::{check_crate, unerased_lint_store}; pub use passes::{EarlyLintPass, LateLintPass}; pub use rustc_session::lint::Level::{self, *}; pub use rustc_session::lint::{BufferedEarlyLint, FutureIncompatibleInfo, Lint, LintId}; pub use rustc_session::lint::{LintArray, LintPass}; -use duplicate_trait::DuplicateTrait; fluent_messages! { "../messages.ftl" } From 0549704a750732b715b98eef6b6a1c62db645e2e Mon Sep 17 00:00:00 2001 From: John Kelly Date: Sat, 29 Apr 2023 13:44:30 +0100 Subject: [PATCH 4/5] Refactor to use FxHashSet --- compiler/rustc_lint/src/duplicate_trait.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_lint/src/duplicate_trait.rs b/compiler/rustc_lint/src/duplicate_trait.rs index 0650641e0f244..ac2a7bcd6a7a6 100644 --- a/compiler/rustc_lint/src/duplicate_trait.rs +++ b/compiler/rustc_lint/src/duplicate_trait.rs @@ -1,3 +1,5 @@ +use rustc_data_structures::fx::FxHashSet; + use crate::hir; use crate::{lints::DuplicateTraitDiag, LateContext, LateLintPass}; @@ -36,18 +38,18 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateTrait { } ) = ty.kind else { return; }; - let mut bounds = (*bounds).to_owned(); - - bounds.sort_unstable_by_key(|b| b.trait_ref.trait_def_id()); - if bounds.len() < 2 { return; } - let mut last_bound = &bounds[0]; - for bound in bounds.iter().skip(1) { - if last_bound.trait_ref.trait_def_id() == bound.trait_ref.trait_def_id() - && let Some(def_id) = bound.trait_ref.trait_def_id() { + let mut seen_def_ids = FxHashSet::default(); + + for bound in bounds.iter() { + let Some(def_id) = bound.trait_ref.trait_def_id() else { continue; }; + + let already_seen = !seen_def_ids.insert(def_id); + + if already_seen { cx.tcx.emit_spanned_lint( DUPLICATE_TRAIT, bound.trait_ref.hir_ref_id, // is this correct? @@ -58,8 +60,6 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateTrait { }, ) } - - last_bound = bound; } } } From d7af3be8c57a64ed43da8040a006a21ad25f6e6a Mon Sep 17 00:00:00 2001 From: John Kelly Date: Sat, 29 Apr 2023 13:53:33 +0100 Subject: [PATCH 5/5] fmt --- compiler/rustc_lint/src/duplicate_trait.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/duplicate_trait.rs b/compiler/rustc_lint/src/duplicate_trait.rs index ac2a7bcd6a7a6..22b0b70d863b8 100644 --- a/compiler/rustc_lint/src/duplicate_trait.rs +++ b/compiler/rustc_lint/src/duplicate_trait.rs @@ -56,7 +56,7 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateTrait { bound.span, DuplicateTraitDiag { trait_name: cx.tcx.item_name(def_id), - suggestion: bound.span + suggestion: bound.span, }, ) }