Skip to content

Commit

Permalink
Rollup merge of #126054 - veera-sivarajan:bugfix-113073-bound-on-gene…
Browse files Browse the repository at this point in the history
…rics-2, r=fee1-dead

`E0229`: Suggest Moving Type Constraints to Type Parameter Declaration

Fixes #113073

This PR suggests  `impl<T: Bound> Trait<T> for Foo` when finding `impl Trait<T: Bound> for Foo`. Tangentially, it also improves a handful of other error messages.

It accomplishes this in two steps:
1. Check if constrained arguments and parameter names appear in the same order and delay emitting "incorrect number of generic arguments" error because it can be confusing for the programmer to see `0 generic arguments provided` when there are `n` constrained generic arguments.

2. Inside `E0229`, suggest declaring the type parameter right after the `impl` keyword by finding the relevant impl block's span for type parameter declaration. This also handles lifetime declarations correctly.

Also, the multi part suggestion doesn't use the fluent error mechanism because translating all the errors to fluent style feels outside the scope of this PR. I will handle it in a separate PR if this gets approved.
  • Loading branch information
matthiaskrgr authored Jun 14, 2024
2 parents 20ca54b + 5da1b41 commit bfe0323
Show file tree
Hide file tree
Showing 14 changed files with 263 additions and 101 deletions.
24 changes: 24 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2471,6 +2471,15 @@ pub enum AssocItemConstraintKind<'hir> {
Bound { bounds: &'hir [GenericBound<'hir>] },
}

impl<'hir> AssocItemConstraintKind<'hir> {
pub fn descr(&self) -> &'static str {
match self {
AssocItemConstraintKind::Equality { .. } => "binding",
AssocItemConstraintKind::Bound { .. } => "constraint",
}
}
}

#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub struct Ty<'hir> {
pub hir_id: HirId,
Expand Down Expand Up @@ -3763,6 +3772,21 @@ impl<'hir> Node<'hir> {
}
}

/// Get a `hir::Impl` if the node is an impl block for the given `trait_def_id`.
pub fn impl_block_of_trait(self, trait_def_id: DefId) -> Option<&'hir Impl<'hir>> {
match self {
Node::Item(Item { kind: ItemKind::Impl(impl_block), .. })
if impl_block
.of_trait
.and_then(|trait_ref| trait_ref.trait_def_id())
.is_some_and(|trait_id| trait_id == trait_def_id) =>
{
Some(impl_block)
}
_ => None,
}
}

pub fn fn_sig(self) -> Option<&'hir FnSig<'hir>> {
match self {
Node::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
Expand Down
71 changes: 62 additions & 9 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,6 @@ pub fn prohibit_assoc_item_constraint(
// otherwise suggest the removal of the binding.
if let Some((def_id, segment, _)) = segment
&& segment.args().parenthesized == hir::GenericArgsParentheses::No
&& let hir::AssocItemConstraintKind::Equality { term } = constraint.kind
{
// Suggests removal of the offending binding
let suggest_removal = |e: &mut Diag<'_>| {
Expand Down Expand Up @@ -1263,7 +1262,7 @@ pub fn prohibit_assoc_item_constraint(
if let Ok(suggestion) = tcx.sess.source_map().span_to_snippet(removal_span) {
e.span_suggestion_verbose(
removal_span,
"consider removing this associated item binding",
format!("consider removing this associated item {}", constraint.kind.descr()),
suggestion,
Applicability::MaybeIncorrect,
);
Expand All @@ -1286,19 +1285,73 @@ pub fn prohibit_assoc_item_constraint(
// Check if the type has a generic param with the same name
// as the assoc type name in the associated item binding.
let generics = tcx.generics_of(def_id);
let matching_param =
generics.own_params.iter().find(|p| p.name.as_str() == constraint.ident.as_str());
let matching_param = generics.own_params.iter().find(|p| p.name == constraint.ident.name);

// Now emit the appropriate suggestion
if let Some(matching_param) = matching_param {
match (&matching_param.kind, term) {
(GenericParamDefKind::Type { .. }, hir::Term::Ty(ty)) => {
suggest_direct_use(&mut err, ty.span);
}
(GenericParamDefKind::Const { .. }, hir::Term::Const(c)) => {
match (constraint.kind, &matching_param.kind) {
(
hir::AssocItemConstraintKind::Equality { term: hir::Term::Ty(ty) },
GenericParamDefKind::Type { .. },
) => suggest_direct_use(&mut err, ty.span),
(
hir::AssocItemConstraintKind::Equality { term: hir::Term::Const(c) },
GenericParamDefKind::Const { .. },
) => {
let span = tcx.hir().span(c.hir_id);
suggest_direct_use(&mut err, span);
}
(hir::AssocItemConstraintKind::Bound { bounds }, _) => {
// Suggest `impl<T: Bound> Trait<T> for Foo` when finding
// `impl Trait<T: Bound> for Foo`

// Get the parent impl block based on the binding we have
// and the trait DefId
let impl_block = tcx
.hir()
.parent_iter(constraint.hir_id)
.find_map(|(_, node)| node.impl_block_of_trait(def_id));

let type_with_constraints =
tcx.sess.source_map().span_to_snippet(constraint.span);

if let Some(impl_block) = impl_block
&& let Ok(type_with_constraints) = type_with_constraints
{
// Filter out the lifetime parameters because
// they should be declared before the type parameter
let lifetimes: String = bounds
.iter()
.filter_map(|bound| {
if let hir::GenericBound::Outlives(lifetime) = bound {
Some(format!("{lifetime}, "))
} else {
None
}
})
.collect();
// Figure out a span and suggestion string based on
// whether there are any existing parameters
let param_decl = if let Some(param_span) =
impl_block.generics.span_for_param_suggestion()
{
(param_span, format!(", {lifetimes}{type_with_constraints}"))
} else {
(
impl_block.generics.span.shrink_to_lo(),
format!("<{lifetimes}{type_with_constraints}>"),
)
};
let suggestions =
vec![param_decl, (constraint.span, format!("{}", matching_param.name))];

err.multipart_suggestion_verbose(
format!("declare the type parameter right after the `impl` keyword"),
suggestions,
Applicability::MaybeIncorrect,
);
}
}
_ => suggest_removal(&mut err),
}
} else {
Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ pub(crate) fn check_generic_arg_count(

let num_default_params = expected_max - expected_min;

let mut all_params_are_binded = false;
let gen_args_info = if provided > expected_max {
invalid_args.extend((expected_max..provided).map(|i| i + args_offset));
let num_redundant_args = provided - expected_max;
Expand All @@ -547,6 +548,20 @@ pub(crate) fn check_generic_arg_count(
} else {
let num_missing_args = expected_max - provided;

let constraint_names: Vec<_> =
gen_args.constraints.iter().map(|b| b.ident.name).collect();
let param_names: Vec<_> = gen_params
.own_params
.iter()
.filter(|param| !has_self || param.index != 0) // Assumes `Self` will always be the first parameter
.map(|param| param.name)
.collect();
if constraint_names == param_names {
// We set this to true and delay emitting `WrongNumberOfGenericArgs`
// to provide a succinct error for cases like issue #113073
all_params_are_binded = true;
};

GenericArgsInfo::MissingTypesOrConsts {
num_missing_args,
num_default_params,
Expand All @@ -567,7 +582,7 @@ pub(crate) fn check_generic_arg_count(
def_id,
)
.diagnostic()
.emit()
.emit_unless(all_params_are_binded)
});

Err(reported)
Expand Down
1 change: 1 addition & 0 deletions tests/ui/associated-type-bounds/no-gat-position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub trait Iter {

fn next<'a>(&'a mut self) -> Option<Self::Item<'a, As1: Copy>>;
//~^ ERROR associated item constraints are not allowed here
//~| HELP consider removing this associated item constraint
}

impl Iter for () {
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/associated-type-bounds/no-gat-position.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error[E0229]: associated item constraints are not allowed here
|
LL | fn next<'a>(&'a mut self) -> Option<Self::Item<'a, As1: Copy>>;
| ^^^^^^^^^ associated item constraint not allowed here
|
help: consider removing this associated item constraint
|
LL | fn next<'a>(&'a mut self) -> Option<Self::Item<'a, As1: Copy>>;
| ~~~~~~~~~~~

error: aborting due to 1 previous error

Expand Down
2 changes: 0 additions & 2 deletions tests/ui/associated-types/associated-types-eq-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ trait Tr3<const N: i32, T2, T3> {
impl Tr3<N
//~^ ERROR associated item constraints are not allowed here
//~| ERROR associated const equality is incomplete
//~| ERROR trait takes 3 generic arguments but 0 generic arguments were supplied
= 42, T2 = Qux, T3 = usize> for Bar {
}

Expand All @@ -92,7 +91,6 @@ impl Tr3<n = 42, T2 = Qux, T3 = usize> for Qux {
// matches the const param ident but the constraint is a type arg
impl Tr3<N = u32, T2 = Qux, T3 = usize> for Bar {
//~^ ERROR associated item constraints are not allowed here
//~| ERROR trait takes 3 generic arguments but 0 generic arguments were supplied
}

// Test for when equality constraint's ident
Expand Down
62 changes: 14 additions & 48 deletions tests/ui/associated-types/associated-types-eq-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ LL | impl Tr3<N
| __________^
LL | |
LL | |
LL | |
LL | | = 42, T2 = Qux, T3 = usize> for Bar {
| |____^
|
Expand All @@ -14,7 +13,7 @@ LL | | = 42, T2 = Qux, T3 = usize> for Bar {
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0658]: associated const equality is incomplete
--> $DIR/associated-types-eq-2.rs:85:10
--> $DIR/associated-types-eq-2.rs:84:10
|
LL | impl Tr3<n = 42, T2 = Qux, T3 = usize> for Qux {
| ^^^^^^
Expand All @@ -24,7 +23,7 @@ LL | impl Tr3<n = 42, T2 = Qux, T3 = usize> for Qux {
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0658]: associated const equality is incomplete
--> $DIR/associated-types-eq-2.rs:100:14
--> $DIR/associated-types-eq-2.rs:98:14
|
LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar {
| ^^^^^^^
Expand All @@ -34,7 +33,7 @@ LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar {
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0658]: associated const equality is incomplete
--> $DIR/associated-types-eq-2.rs:108:10
--> $DIR/associated-types-eq-2.rs:106:10
|
LL | impl Tr3<X = 42, Y = Qux, Z = usize> for Bar {
| ^^^^^^
Expand Down Expand Up @@ -190,30 +189,13 @@ help: to use `GenericTerm<i32>` as a generic argument specify it directly
LL | impl Tr2<i32, Qux, GenericTerm<i32>> for Bar {
| ~~~~~~~~~~~~~~~~

error[E0107]: trait takes 3 generic arguments but 0 generic arguments were supplied
--> $DIR/associated-types-eq-2.rs:76:6
|
LL | impl Tr3<N
| ^^^ expected 3 generic arguments
|
note: trait defined here, with 3 generic parameters: `N`, `T2`, `T3`
--> $DIR/associated-types-eq-2.rs:69:7
|
LL | trait Tr3<const N: i32, T2, T3> {
| ^^^ ------------ -- --
help: add missing generic arguments
|
LL | impl Tr3<N, T2, T3, N
| ++++++++++

error[E0229]: associated item constraints are not allowed here
--> $DIR/associated-types-eq-2.rs:76:10
|
LL | impl Tr3<N
| __________^
LL | |
LL | |
LL | |
LL | | = 42, T2 = Qux, T3 = usize> for Bar {
| |____^ associated item constraint not allowed here
|
Expand All @@ -223,7 +205,7 @@ LL | impl Tr3<42, T2 = Qux, T3 = usize> for Bar {
| ~~

error[E0107]: trait takes 3 generic arguments but 0 generic arguments were supplied
--> $DIR/associated-types-eq-2.rs:85:6
--> $DIR/associated-types-eq-2.rs:84:6
|
LL | impl Tr3<n = 42, T2 = Qux, T3 = usize> for Qux {
| ^^^ expected 3 generic arguments
Expand All @@ -239,7 +221,7 @@ LL | impl Tr3<N, T2, T3, n = 42, T2 = Qux, T3 = usize> for Qux {
| ++++++++++

error[E0229]: associated item constraints are not allowed here
--> $DIR/associated-types-eq-2.rs:85:10
--> $DIR/associated-types-eq-2.rs:84:10
|
LL | impl Tr3<n = 42, T2 = Qux, T3 = usize> for Qux {
| ^^^^^^ associated item constraint not allowed here
Expand All @@ -249,24 +231,8 @@ help: consider removing this associated item binding
LL | impl Tr3<n = 42, T2 = Qux, T3 = usize> for Qux {
| ~~~~~~~

error[E0107]: trait takes 3 generic arguments but 0 generic arguments were supplied
--> $DIR/associated-types-eq-2.rs:93:6
|
LL | impl Tr3<N = u32, T2 = Qux, T3 = usize> for Bar {
| ^^^ expected 3 generic arguments
|
note: trait defined here, with 3 generic parameters: `N`, `T2`, `T3`
--> $DIR/associated-types-eq-2.rs:69:7
|
LL | trait Tr3<const N: i32, T2, T3> {
| ^^^ ------------ -- --
help: add missing generic arguments
|
LL | impl Tr3<N, T2, T3, N = u32, T2 = Qux, T3 = usize> for Bar {
| ++++++++++

error[E0229]: associated item constraints are not allowed here
--> $DIR/associated-types-eq-2.rs:93:10
--> $DIR/associated-types-eq-2.rs:92:10
|
LL | impl Tr3<N = u32, T2 = Qux, T3 = usize> for Bar {
| ^^^^^^^ associated item constraint not allowed here
Expand All @@ -277,7 +243,7 @@ LL | impl Tr3<N = u32, T2 = Qux, T3 = usize> for Bar {
| ~~~~~~~~

error[E0107]: trait takes 3 generic arguments but 1 generic argument was supplied
--> $DIR/associated-types-eq-2.rs:100:6
--> $DIR/associated-types-eq-2.rs:98:6
|
LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar {
| ^^^ -- supplied 1 generic argument
Expand All @@ -295,7 +261,7 @@ LL | impl Tr3<42, T2, T3, T2 = 42, T3 = usize> for Bar {
| ++++++++

error[E0229]: associated item constraints are not allowed here
--> $DIR/associated-types-eq-2.rs:100:14
--> $DIR/associated-types-eq-2.rs:98:14
|
LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar {
| ^^^^^^^ associated item constraint not allowed here
Expand All @@ -306,7 +272,7 @@ LL | impl Tr3<42, T2 = 42, T3 = usize> for Bar {
| ~~~~~~~~~

error[E0107]: trait takes 3 generic arguments but 0 generic arguments were supplied
--> $DIR/associated-types-eq-2.rs:108:6
--> $DIR/associated-types-eq-2.rs:106:6
|
LL | impl Tr3<X = 42, Y = Qux, Z = usize> for Bar {
| ^^^ expected 3 generic arguments
Expand All @@ -322,7 +288,7 @@ LL | impl Tr3<N, T2, T3, X = 42, Y = Qux, Z = usize> for Bar {
| ++++++++++

error[E0229]: associated item constraints are not allowed here
--> $DIR/associated-types-eq-2.rs:108:10
--> $DIR/associated-types-eq-2.rs:106:10
|
LL | impl Tr3<X = 42, Y = Qux, Z = usize> for Bar {
| ^^^^^^ associated item constraint not allowed here
Expand All @@ -333,13 +299,13 @@ LL | impl Tr3<X = 42, Y = Qux, Z = usize> for Bar {
| ~~~~~~~

error[E0107]: struct takes 1 generic argument but 0 generic arguments were supplied
--> $DIR/associated-types-eq-2.rs:119:13
--> $DIR/associated-types-eq-2.rs:117:13
|
LL | impl<'a, T> St<'a , T = Qux> {
| ^^ expected 1 generic argument
|
note: struct defined here, with 1 generic parameter: `T`
--> $DIR/associated-types-eq-2.rs:117:8
--> $DIR/associated-types-eq-2.rs:115:8
|
LL | struct St<'a, T> { v: &'a T }
| ^^ -
Expand All @@ -349,7 +315,7 @@ LL | impl<'a, T> St<'a, T , T = Qux> {
| +++

error[E0229]: associated item constraints are not allowed here
--> $DIR/associated-types-eq-2.rs:119:21
--> $DIR/associated-types-eq-2.rs:117:21
|
LL | impl<'a, T> St<'a , T = Qux> {
| ^^^^^^^ associated item constraint not allowed here
Expand All @@ -359,7 +325,7 @@ help: to use `Qux` as a generic argument specify it directly
LL | impl<'a, T> St<'a , Qux> {
| ~~~

error: aborting due to 27 previous errors
error: aborting due to 25 previous errors

Some errors have detailed explanations: E0046, E0107, E0229, E0658.
For more information about an error, try `rustc --explain E0046`.
Loading

0 comments on commit bfe0323

Please sign in to comment.