Skip to content

Commit

Permalink
Auto merge of #96770 - flip1995:fix-trait-type-in-bounds, r=cjgillot
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
bors committed May 8, 2022
2 parents 8fbd92d + dd1ff40 commit ed3164b
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 34 deletions.
14 changes: 11 additions & 3 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -1419,7 +1427,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
bounds,
span,
bound_generic_params: &[],
in_where_clause: false,
origin,
}))
}
GenericParamKind::Lifetime => {
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
)
}
_ => {
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 0 additions & 2 deletions src/tools/clippy/clippy_lints/src/lib.register_nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
10 changes: 7 additions & 3 deletions src/tools/clippy/clippy_lints/src/trait_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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: _`"
}

Expand Down Expand Up @@ -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"
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 1 addition & 9 deletions src/tools/clippy/tests/ui/trait_duplication_in_bounds.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

10 changes: 1 addition & 9 deletions src/tools/clippy/tests/ui/type_repetition_in_bounds.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit ed3164b

Please sign in to comment.