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

Suggest how to fix with unconstrained type parameters #134270

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 55 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,61 @@ impl<'hir> Generics<'hir> {
bound_span.with_lo(bounds[bound_pos - 1].span().hi())
}
}

pub fn span_for_param_removal(&self, param_index: usize) -> Span {
if param_index >= self.params.len() {
return self.span.shrink_to_hi();
}

let is_impl_generic = |par: &&GenericParam<'_>| match par.kind {
GenericParamKind::Type { .. }
| GenericParamKind::Const { .. }
| GenericParamKind::Lifetime { kind: LifetimeParamKind::Explicit } => true,
_ => false,
};

// Find the span of the type parameter.
if let Some(next) = self.params[param_index + 1..].iter().find(is_impl_generic) {
self.params[param_index].span.until(next.span)
} else if let Some(prev) = self.params[..param_index].iter().rfind(is_impl_generic) {
let mut prev_span = prev.span;
// Consider the span of the bounds with the previous generic parameter when there is.
if let Some(prev_bounds_span) = self.span_for_param_bounds(prev) {
prev_span = prev_span.to(prev_bounds_span);
}

// Consider the span of the bounds with the current generic parameter when there is.
prev_span.shrink_to_hi().to(
if let Some(cur_bounds_span) = self.span_for_param_bounds(&self.params[param_index])
{
cur_bounds_span
} else {
self.params[param_index].span
},
)
} else {
// Remove also angle brackets <> when there is just ONE generic parameter.
self.span
}
}

fn span_for_param_bounds(&self, param: &GenericParam<'hir>) -> Option<Span> {
self.predicates
.iter()
.find(|pred| {
if let WherePredicateKind::BoundPredicate(WhereBoundPredicate {
origin: PredicateOrigin::GenericParam,
bounded_ty,
..
}) = pred.kind
{
bounded_ty.span == param.span
} else {
false
}
})
.map(|pred| pred.span)
}
}

/// A single predicate in a where-clause.
Expand Down
72 changes: 71 additions & 1 deletion compiler/rustc_hir_analysis/src/impl_wf_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ use std::assert_matches::debug_assert_matches;
use min_specialization::check_min_specialization;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::codes::*;
use rustc_errors::{Applicability, Diag};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::LocalDefId;
use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
use rustc_hir::{Path, QPath, Ty, TyKind};
use rustc_middle::ty::{self, GenericParamDef, TyCtxt, TypeVisitableExt};
use rustc_span::ErrorGuaranteed;

use crate::constrained_generic_params as cgp;
Expand Down Expand Up @@ -128,6 +130,21 @@ pub(crate) fn enforce_impl_lifetime_params_are_constrained(
for param in &impl_generics.own_params {
match param.kind {
ty::GenericParamDefKind::Lifetime => {
let param_lt = cgp::Parameter::from(param.to_early_bound_region_data());
if lifetimes_in_associated_types.contains(&param_lt) // (*)
&& !input_parameters.contains(&param_lt)
{
let mut diag = tcx.dcx().create_err(UnconstrainedGenericParameter {
span: tcx.def_span(param.def_id),
param_name: param.name,
param_def_kind: tcx.def_descr(param.def_id),
const_param_note: false,
const_param_note2: false,
});
diag.code(E0207);
suggest_to_remove_or_use_generic(tcx, &mut diag, impl_def_id, param);
res = Err(diag.emit());
}
// This is a horrible concession to reality. I think it'd be
// better to just ban unconstrained lifetimes outright, but in
// practice people do non-hygienic macros like:
Expand Down Expand Up @@ -158,6 +175,7 @@ pub(crate) fn enforce_impl_lifetime_params_are_constrained(
const_param_note2: false,
});
diag.code(E0207);
suggest_to_remove_or_use_generic(tcx, &mut diag, impl_def_id, param);
res = Err(diag.emit());
}
}
Expand Down Expand Up @@ -229,8 +247,60 @@ pub(crate) fn enforce_impl_non_lifetime_params_are_constrained(
const_param_note2: const_param_note,
});
diag.code(E0207);
suggest_to_remove_or_use_generic(tcx, &mut diag, impl_def_id, &param);
res = Err(diag.emit());
}
}
res
}

fn suggest_to_remove_or_use_generic(
tcx: TyCtxt<'_>,
diag: &mut Diag<'_>,
impl_def_id: LocalDefId,
param: &GenericParamDef,
) {
let node = tcx.hir_node_by_def_id(impl_def_id);
let hir_impl = node.expect_item().expect_impl();

let Some((index, _)) = hir_impl
.generics
.params
.iter()
.enumerate()
.find(|(_, par)| par.def_id.to_def_id() == param.def_id)
else {
return;
};

let mut suggestions = vec![];

// Suggestion for removing the type parameter.
suggestions.push(vec![(hir_impl.generics.span_for_param_removal(index), String::new())]);

// Suggestion for making use of the type parameter.
if let Some(path) = extract_ty_as_path(hir_impl.self_ty) {
let seg = path.segments.last().unwrap();
if let Some(args) = seg.args {
suggestions
.push(vec![(args.span().unwrap().shrink_to_hi(), format!(", {}", param.name))]);
} else {
suggestions.push(vec![(seg.ident.span.shrink_to_hi(), format!("<{}>", param.name))]);
}
}

diag.multipart_suggestions(
format!("either remove the type parameter {}, or make use of it, for example", param.name),
suggestions,
Applicability::MaybeIncorrect,
);
}

fn extract_ty_as_path<'hir>(ty: &Ty<'hir>) -> Option<&'hir Path<'hir>> {
match ty.kind {
TyKind::Path(QPath::Resolved(_, path)) => Some(path),
TyKind::Slice(ty) | TyKind::Array(ty, _) => extract_ty_as_path(ty),
TyKind::Ptr(ty) | TyKind::Ref(_, ty) => extract_ty_as_path(ty.ty),
_ => None,
}
}
8 changes: 8 additions & 0 deletions tests/rustdoc-ui/not-wf-ambiguous-normalization.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
|
LL | impl<T> Allocator for DefaultAllocator {
| ^ unconstrained type parameter
|
help: either remove the type parameter T, or make use of it, for example
|
LL - impl<T> Allocator for DefaultAllocator {
LL + impl Allocator for DefaultAllocator {
|
LL | impl<T> Allocator for DefaultAllocator<T> {
| +++

error: aborting due to 1 previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ error[E0207]: the type parameter `Q` is not constrained by the impl trait, self
|
LL | unsafe impl<Q: Trait> Send for Inner {}
| ^ unconstrained type parameter
|
help: either remove the type parameter Q, or make use of it, for example
|
LL - unsafe impl<Q: Trait> Send for Inner {}
LL + unsafe impl Send for Inner {}
|
LL | unsafe impl<Q: Trait> Send for Inner<Q> {}
| +++
Comment on lines +12 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really convinced that suggesting to use Q in some path is the right solution. Inner takes no generic params, so the suggestion is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do this, we must know the ty::Ty for self_ty (e.g. Inner here). Is there any way to get it?


error: aborting due to 1 previous error

Expand Down
16 changes: 16 additions & 0 deletions tests/ui/associated-types/issue-26262.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,28 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
|
LL | impl<T: Tr> S<T::Assoc> {
| ^ unconstrained type parameter
|
help: either remove the type parameter T, or make use of it, for example
|
LL - impl<T: Tr> S<T::Assoc> {
LL + impl S<T::Assoc> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case is more subtle. We should not suggest to remove T if it appears somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like avoidable, but only we can do is to compare symbols of generics parameters and, bounds of them and generics of the type. Is it acceptable, or do you know better ways?

|
LL | impl<T: Tr> S<T::Assoc, T> {
| +++

error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
--> $DIR/issue-26262.rs:17:6
|
LL | impl<'a,T: Trait2<'a>> Trait1<<T as Trait2<'a>>::Foo> for T {
| ^^ unconstrained lifetime parameter
|
help: either remove the type parameter 'a, or make use of it, for example
|
LL - impl<'a,T: Trait2<'a>> Trait1<<T as Trait2<'a>>::Foo> for T {
LL + impl<T: Trait2<'a>> Trait1<<T as Trait2<'a>>::Foo> for T {
|
LL | impl<'a,T: Trait2<'a>> Trait1<<T as Trait2<'a>>::Foo> for T<'a> {
| ++++

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait,
|
LL | impl<'a> Actor for () {
| ^^ unconstrained lifetime parameter
|
help: either remove the type parameter 'a, or make use of it, for example
|
LL - impl<'a> Actor for () {
LL + impl Actor for () {
|

error: aborting due to 1 previous error

Expand Down
7 changes: 7 additions & 0 deletions tests/ui/async-await/issues/issue-78654.full.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ LL | impl<const H: feature> Foo {
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter H, or make use of it, for example
|
LL - impl<const H: feature> Foo {
LL + impl Foo {
|
LL | impl<const H: feature> Foo<H> {
| +++

error: aborting due to 2 previous errors

Expand Down
7 changes: 7 additions & 0 deletions tests/ui/async-await/issues/issue-78654.min.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ LL | impl<const H: feature> Foo {
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter H, or make use of it, for example
|
LL - impl<const H: feature> Foo {
LL + impl Foo {
|
LL | impl<const H: feature> Foo<H> {
| +++

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ LL | impl<'a, const NUM: usize> std::ops::Add<&'a Foo> for Foo
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter NUM, or make use of it, for example
|
LL - impl<'a, const NUM: usize> std::ops::Add<&'a Foo> for Foo
LL + impl<'a> std::ops::Add<&'a Foo> for Foo
|
LL | impl<'a, const NUM: usize> std::ops::Add<&'a Foo> for Foo<NUM>
| +++++

error[E0284]: type annotations needed
--> $DIR/post-analysis-user-facing-param-env.rs:11:40
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ LL | impl<'a, T, const N: usize> Iterator for ConstChunksExact<'a, T, {}> {
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter N, or make use of it, for example
|
LL - impl<'a, T, const N: usize> Iterator for ConstChunksExact<'a, T, {}> {
LL + impl<'a, T> Iterator for ConstChunksExact<'a, T, {}> {
|
LL | impl<'a, T, const N: usize> Iterator for ConstChunksExact<'a, T, {}, N> {
| +++

error[E0308]: mismatched types
--> $DIR/ice-unexpected-inference-var-122549.rs:15:66
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/const-generics/issues/issue-68366.full.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ LL | impl <const N: usize> Collatz<{Some(N)}> {}
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter N, or make use of it, for example
|
LL - impl <const N: usize> Collatz<{Some(N)}> {}
LL + impl Collatz<{Some(N)}> {}
|
LL | impl <const N: usize> Collatz<{Some(N)}, N> {}
| +++

error[E0207]: the const parameter `N` is not constrained by the impl trait, self type, or predicates
--> $DIR/issue-68366.rs:19:6
Expand All @@ -27,6 +34,13 @@ LL | impl<const N: usize> Foo {}
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter N, or make use of it, for example
|
LL - impl<const N: usize> Foo {}
LL + impl Foo {}
|
LL | impl<const N: usize> Foo<N> {}
| +++

error: overly complex generic constant
--> $DIR/issue-68366.rs:12:31
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/const-generics/issues/issue-68366.min.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ LL | impl <const N: usize> Collatz<{Some(N)}> {}
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter N, or make use of it, for example
|
LL - impl <const N: usize> Collatz<{Some(N)}> {}
LL + impl Collatz<{Some(N)}> {}
|
LL | impl <const N: usize> Collatz<{Some(N)}, N> {}
| +++

error[E0207]: the const parameter `N` is not constrained by the impl trait, self type, or predicates
--> $DIR/issue-68366.rs:19:6
Expand All @@ -36,6 +43,13 @@ LL | impl<const N: usize> Foo {}
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter N, or make use of it, for example
|
LL - impl<const N: usize> Foo {}
LL + impl Foo {}
|
LL | impl<const N: usize> Foo<N> {}
| +++

error: aborting due to 4 previous errors

Expand Down
7 changes: 7 additions & 0 deletions tests/ui/dropck/unconstrained_const_param_on_drop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ LL | impl<const UNUSED: usize> Drop for Foo {}
|
= note: expressions using a const parameter must map each value to a distinct output value
= note: proving the result of expressions other than the parameter are unique is not supported
help: either remove the type parameter UNUSED, or make use of it, for example
|
LL - impl<const UNUSED: usize> Drop for Foo {}
LL + impl Drop for Foo {}
|
LL | impl<const UNUSED: usize> Drop for Foo<UNUSED> {}
| ++++++++

error: aborting due to 2 previous errors

Expand Down
8 changes: 8 additions & 0 deletions tests/ui/error-codes/E0207.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
|
LL | impl<T: Default> Foo {
| ^ unconstrained type parameter
|
help: either remove the type parameter T, or make use of it, for example
|
LL - impl<T: Default> Foo {
LL + impl Foo {
|
LL | impl<T: Default> Foo<T> {
| +++

error: aborting due to 1 previous error

Expand Down
8 changes: 8 additions & 0 deletions tests/ui/generic-associated-types/bugs/issue-87735.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ error[E0207]: the type parameter `U` is not constrained by the impl trait, self
|
LL | impl<'b, T, U> AsRef2 for Foo<T>
| ^ unconstrained type parameter
|
help: either remove the type parameter U, or make use of it, for example
|
LL - impl<'b, T, U> AsRef2 for Foo<T>
LL + impl<'b, T> AsRef2 for Foo<T>
|
LL | impl<'b, T, U> AsRef2 for Foo<T, U>
| +++

error[E0309]: the parameter type `U` may not live long enough
--> $DIR/issue-87735.rs:34:21
Expand Down
Loading
Loading