Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 7 pull requests #126473

Merged
merged 17 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
E0229: Suggest Moving Type Constraints to Type Parameter Declaration
  • Loading branch information
veera-sivarajan committed Jun 12, 2024
commit 5da1b4189e8b6820624ea548efe90c2cc35d576c
24 changes: 24 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2456,6 +2456,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 @@ -3735,6 +3744,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
2 changes: 1 addition & 1 deletion tests/ui/associated-type-bounds/no-gat-position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +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 binding
//~| HELP consider removing this associated item constraint
}

impl Iter for () {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/associated-type-bounds/no-gat-position.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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 binding
help: consider removing this associated item constraint
|
LL | fn next<'a>(&'a mut self) -> Option<Self::Item<'a, As1: Copy>>;
| ~~~~~~~~~~~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ error[E0229]: associated item constraints are not allowed here
LL | impl Super1<'_, bar(): Send> for () {}
| ^^^^^^^^^^^ associated item constraint not allowed here
|
help: consider removing this associated item binding
help: consider removing this associated item constraint
|
LL | impl Super1<'_, bar(): Send> for () {}
| ~~~~~~~~~~~~~
Expand Down
Loading