Auto merge of #96770 - flip1995:fix-trait-type-in-bounds, r=cjgillot

Track if a where bound comes from a impl Trait desugar With #93803 `impl Trait` function arguments get desugared to hidden where bounds. However, Clippy needs to know if a bound was originally a `impl Trait` or an actual bound. This adds a field to the `WhereBoundPredicate` struct to keep track of this information during AST->HIR lowering. r? `@cjgillot` cc `@estebank` (as the reviewer of #93803)
rust-lang · May 8, 2022 · ed3164b · ed3164b
2 parents 8fbd92d + dd1ff40
commit ed3164b
Show file tree

Hide file tree

Showing 11 changed files with 40 additions and 34 deletions.
diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
@@ -11,6 +11,7 @@ use rustc_errors::struct_span_err;
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID};
+use rustc_hir::PredicateOrigin;
 use rustc_index::vec::{Idx, IndexVec};
 use rustc_session::utils::NtToTokenstream;
 use rustc_session::Session;
@@ -1346,7 +1347,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let mut predicates = SmallVec::new();
         predicates.extend(generics.params.iter().filter_map(|param| {
             let bounds = self.lower_param_bounds(&param.bounds, itctx.reborrow());
-            self.lower_generic_bound_predicate(param.ident, param.id, &param.kind, bounds)
+            self.lower_generic_bound_predicate(
+                param.ident,
+                param.id,
+                &param.kind,
+                bounds,
+                PredicateOrigin::GenericParam,
+            )
         }));
         predicates.extend(
             generics
@@ -1380,6 +1387,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         id: NodeId,
         kind: &GenericParamKind,
         bounds: &'hir [hir::GenericBound<'hir>],
+        origin: PredicateOrigin,
     ) -> Option<hir::WherePredicate<'hir>> {
         // Do not create a clause if we do not have anything inside it.
         if bounds.is_empty() {
@@ -1419,7 +1427,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     bounds,
                     span,
                     bound_generic_params: &[],
-                    in_where_clause: false,
+                    origin,
                 }))
             }
             GenericParamKind::Lifetime => {
@@ -1458,7 +1466,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     )
                 })),
                 span: self.lower_span(span),
-                in_where_clause: true,
+                origin: PredicateOrigin::WhereClause,
             }),
             WherePredicate::RegionPredicate(WhereRegionPredicate {
                 ref lifetime,

diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
@@ -1298,6 +1298,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                             def_node_id,
                             &GenericParamKind::Type { default: None },
                             hir_bounds,
+                            hir::PredicateOrigin::ImplTrait,
                         ) {
                             in_band_ty_bounds.push(preds)
                         }

diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
@@ -706,7 +706,7 @@ impl<'hir> WherePredicate<'hir> {
 
     pub fn in_where_clause(&self) -> bool {
         match self {
-            WherePredicate::BoundPredicate(p) => p.in_where_clause,
+            WherePredicate::BoundPredicate(p) => p.origin == PredicateOrigin::WhereClause,
             WherePredicate::RegionPredicate(p) => p.in_where_clause,
             WherePredicate::EqPredicate(_) => false,
         }
@@ -721,11 +721,19 @@ impl<'hir> WherePredicate<'hir> {
     }
 }
 
+#[derive(Debug, HashStable_Generic, PartialEq, Eq)]
+pub enum PredicateOrigin {
+    WhereClause,
+    GenericParam,
+    ImplTrait,
+}
+
 /// A type bound (e.g., `for<'c> Foo: Send + Clone + 'c`).
 #[derive(Debug, HashStable_Generic)]
 pub struct WhereBoundPredicate<'hir> {
     pub span: Span,
-    pub in_where_clause: bool,
+    /// Origin of the predicate.
+    pub origin: PredicateOrigin,
     /// Any generics from a `for` binding.
     pub bound_generic_params: &'hir [GenericParam<'hir>],
     /// The type being bounded.

diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
@@ -36,7 +36,7 @@ use rustc_feature::{deprecated_attributes, AttributeGate, BuiltinAttribute, Gate
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID};
-use rustc_hir::{ForeignItemKind, GenericParamKind, HirId, PatKind};
+use rustc_hir::{ForeignItemKind, GenericParamKind, HirId, PatKind, PredicateOrigin};
 use rustc_index::vec::Idx;
 use rustc_middle::lint::LintDiagnosticBuilder;
 use rustc_middle::ty::layout::{LayoutError, LayoutOf};
@@ -2226,7 +2226,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
                                     Self::lifetimes_outliving_type(inferred_outlives, index),
                                     &predicate.bounds,
                                     predicate.span,
-                                    predicate.in_where_clause,
+                                    predicate.origin == PredicateOrigin::WhereClause,
                                 )
                             }
                             _ => {

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -16,6 +16,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, DefKind, Res};
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
+use rustc_hir::PredicateOrigin;
 use rustc_infer::infer::region_constraints::{Constraint, RegionConstraintData};
 use rustc_middle::middle::resolve_lifetime as rl;
 use rustc_middle::ty::fold::TypeFolder;
@@ -493,7 +494,7 @@ fn clean_generic_param(
             let bounds = if let Some(generics) = generics {
                 generics
                     .bounds_for_param(did)
-                    .filter(|bp| !bp.in_where_clause)
+                    .filter(|bp| bp.origin != PredicateOrigin::WhereClause)
                     .flat_map(|bp| bp.bounds)
                     .filter_map(|x| x.clean(cx))
                     .collect()

diff --git a/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs b/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs
@@ -29,8 +29,6 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
     LintId::of(strings::STRING_LIT_AS_BYTES),
     LintId::of(suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS),
     LintId::of(trailing_empty_array::TRAILING_EMPTY_ARRAY),
-    LintId::of(trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS),
-    LintId::of(trait_bounds::TYPE_REPETITION_IN_BOUNDS),
     LintId::of(transmute::TRANSMUTE_UNDEFINED_REPR),
     LintId::of(transmute::USELESS_TRANSMUTE),
     LintId::of(use_self::USE_SELF),

diff --git a/src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs b/src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs
@@ -84,6 +84,8 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED),
     LintId::of(stable_sort_primitive::STABLE_SORT_PRIMITIVE),
     LintId::of(strings::STRING_ADD_ASSIGN),
+    LintId::of(trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS),
+    LintId::of(trait_bounds::TYPE_REPETITION_IN_BOUNDS),
     LintId::of(transmute::TRANSMUTE_PTR_TO_PTR),
     LintId::of(types::LINKEDLIST),
     LintId::of(types::OPTION_OPTION),

diff --git a/src/tools/clippy/clippy_lints/src/lifetimes.rs b/src/tools/clippy/clippy_lints/src/lifetimes.rs
@@ -9,8 +9,8 @@ use rustc_hir::intravisit::{
 use rustc_hir::FnRetTy::Return;
 use rustc_hir::{
     BareFnTy, BodyId, FnDecl, GenericArg, GenericBound, GenericParam, GenericParamKind, Generics, Impl, ImplItem,
-    ImplItemKind, Item, ItemKind, LangItem, Lifetime, LifetimeName, ParamName, PolyTraitRef, TraitBoundModifier,
-    TraitFn, TraitItem, TraitItemKind, Ty, TyKind, WherePredicate,
+    ImplItemKind, Item, ItemKind, LangItem, Lifetime, LifetimeName, ParamName, PolyTraitRef, PredicateOrigin,
+    TraitBoundModifier, TraitFn, TraitItem, TraitItemKind, Ty, TyKind, WherePredicate,
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::nested_filter as middle_nested_filter;
@@ -145,7 +145,7 @@ fn check_fn_inner<'tcx>(
         .filter(|param| matches!(param.kind, GenericParamKind::Type { .. }));
     for typ in types {
         for pred in generics.bounds_for_param(cx.tcx.hir().local_def_id(typ.hir_id)) {
-            if pred.in_where_clause {
+            if pred.origin == PredicateOrigin::WhereClause {
                 // has_where_lifetimes checked that this predicate contains no lifetime.
                 continue;
             }

diff --git a/src/tools/clippy/clippy_lints/src/trait_bounds.rs b/src/tools/clippy/clippy_lints/src/trait_bounds.rs
@@ -8,7 +8,8 @@ use rustc_data_structures::unhash::UnhashMap;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
 use rustc_hir::{
-    GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, QPath, TraitItem, Ty, TyKind, WherePredicate,
+    GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, PredicateOrigin, QPath, TraitItem, Ty, TyKind,
+    WherePredicate,
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
@@ -35,7 +36,7 @@ declare_clippy_lint! {
     /// ```
     #[clippy::version = "1.38.0"]
     pub TYPE_REPETITION_IN_BOUNDS,
-    nursery,
+    pedantic,
     "Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`"
 }
 
@@ -65,7 +66,7 @@ declare_clippy_lint! {
     /// ```
     #[clippy::version = "1.47.0"]
     pub TRAIT_DUPLICATION_IN_BOUNDS,
-    nursery,
+    pedantic,
     "Check if the same trait bounds are specified twice during a function declaration"
 }
 
@@ -95,6 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
         for predicate in item.generics.predicates {
             if_chain! {
                 if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate;
+                if bound_predicate.origin != PredicateOrigin::ImplTrait;
                 if !bound_predicate.span.from_expansion();
                 if let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = bound_predicate.bounded_ty.kind;
                 if let Some(PathSegment {
@@ -168,6 +170,7 @@ impl TraitBounds {
         for bound in gen.predicates {
             if_chain! {
                 if let WherePredicate::BoundPredicate(ref p) = bound;
+                if p.origin != PredicateOrigin::ImplTrait;
                 if p.bounds.len() as u64 <= self.max_trait_bounds;
                 if !p.span.from_expansion();
                 if let Some(ref v) = map.insert(
@@ -223,6 +226,7 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) {
     for predicate in gen.predicates {
         if_chain! {
             if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate;
+            if bound_predicate.origin != PredicateOrigin::ImplTrait;
             if !bound_predicate.span.from_expansion();
             if let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = bound_predicate.bounded_ty.kind;
             if let Some(segment) = segments.first();

diff --git a/src/tools/clippy/tests/ui/trait_duplication_in_bounds.stderr b/src/tools/clippy/tests/ui/trait_duplication_in_bounds.stderr
@@ -67,13 +67,5 @@ LL |         Self: Iterator<Item = Foo>,
    |
    = help: consider removing this trait bound
 
-error: this trait bound is already specified in the where clause
-  --> $DIR/trait_duplication_in_bounds.rs:99:23
-   |
-LL | fn impl_trait(_: impl AsRef<str>, _: impl AsRef<str>) {}
-   |                       ^^^^^^^^^^
-   |
-   = help: consider removing this trait bound
-
-error: aborting due to 9 previous errors
+error: aborting due to 8 previous errors
 
diff --git a/src/tools/clippy/tests/ui/type_repetition_in_bounds.stderr b/src/tools/clippy/tests/ui/type_repetition_in_bounds.stderr
@@ -19,13 +19,5 @@ LL |     Self: Copy + Default + Ord,
    |
    = help: consider combining the bounds: `Self: Clone + Copy + Default + Ord`
 
-error: this type has already been used as a bound predicate
-  --> $DIR/type_repetition_in_bounds.rs:83:43
-   |
-LL | fn impl_trait(_: impl AsRef<str>, _: impl AsRef<str>) {}
-   |                                           ^^^^^^^^^^
-   |
-   = help: consider combining the bounds: `impl AsRef<str>: AsRef<str> + AsRef<str>`
-
-error: aborting due to 3 previous errors
+error: aborting due to 2 previous errors