Skip to content

Improve unconstrained impl diagnostic (fixes #107295) #126026

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

Closed
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
104 changes: 96 additions & 8 deletions compiler/rustc_hir_analysis/src/impl_wf_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use crate::constrained_generic_params as cgp;
use min_specialization::check_min_specialization;

use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::{codes::*, struct_span_code_err};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::LocalDefId;
Expand Down Expand Up @@ -88,14 +88,36 @@ fn enforce_impl_params_are_constrained(

impl_trait_ref.error_reported()?;

let mut input_parameters = cgp::parameters_for_impl(tcx, impl_self_ty, impl_trait_ref);
let mut constrained_parameters = cgp::parameters_for_impl(tcx, impl_self_ty, impl_trait_ref);
cgp::identify_constrained_generic_params(
tcx,
impl_predicates,
impl_trait_ref,
&mut input_parameters,
&mut constrained_parameters,
);

// Reasons each generic is unconstrained, or Other if not present
let mut unconstrained_reasons = FxHashMap::default();
for (clause, _) in impl_predicates.predicates {
if let Some(projection) = clause.as_projection_clause() {
let mentioned_params = cgp::parameters_for(tcx, projection.term().skip_binder(), true);
let is_clause_circular =
Some(projection.required_poly_trait_ref(tcx).skip_binder()) == impl_trait_ref;
Comment on lines +103 to +105
Copy link
Author

Choose a reason for hiding this comment

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

Almost forgot, but wanted to add a quick note/question regarding skip_binder here... from what I understand, binders correspond to things like for<'a>, and skip_binder is generally a bad idea since it just unwraps the binder wo/ updating references to the quantifier.

My Q: is this specific usage safe? Or is some else more appropriate? (My understanding of this is a bit hazy 🙂)


for param in mentioned_params {
if is_clause_circular {
// Potentially override BoundToUnconstrainedProjection
unconstrained_reasons.insert(param, UnconstrainedReason::BoundCircularly);
} else {
// Don't override BoundCircularly
unconstrained_reasons
.entry(param)
.or_insert(UnconstrainedReason::BoundToUnconstrainedProjection);
}
}
}
}

// Disallow unconstrained lifetimes, but only if they appear in assoc types.
let lifetimes_in_associated_types: FxHashSet<_> = tcx
.associated_item_def_ids(impl_def_id)
Expand All @@ -115,42 +137,56 @@ fn enforce_impl_params_are_constrained(
})
.collect();

let impl_kind = match impl_trait_ref {
Some(_) => ImplKind::ImplTrait,
None => ImplKind::InherentImpl,
};

let mut res = Ok(());
for param in &impl_generics.own_params {
let cgp_param = cgp::Parameter(param.index);
let unconstrained_reason =
unconstrained_reasons.get(&cgp_param).copied().unwrap_or_default();

match param.kind {
// Disallow ANY unconstrained type parameters.
ty::GenericParamDefKind::Type { .. } => {
let param_ty = ty::ParamTy::for_def(param);
if !input_parameters.contains(&cgp::Parameter::from(param_ty)) {
if !constrained_parameters.contains(&cgp_param) {
res = Err(report_unused_parameter(
tcx,
tcx.def_span(param.def_id),
"type",
param_ty.name,
impl_kind,
unconstrained_reason,
));
}
}
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)
if lifetimes_in_associated_types.contains(&cgp_param) && // (*)
!constrained_parameters.contains(&cgp_param)
{
res = Err(report_unused_parameter(
tcx,
tcx.def_span(param.def_id),
"lifetime",
param.name,
impl_kind,
unconstrained_reason,
));
}
}
ty::GenericParamDefKind::Const { .. } => {
let param_ct = ty::ParamConst::for_def(param);
if !input_parameters.contains(&cgp::Parameter::from(param_ct)) {
if !constrained_parameters.contains(&cgp_param) {
res = Err(report_unused_parameter(
tcx,
tcx.def_span(param.def_id),
"const",
param_ct.name,
impl_kind,
unconstrained_reason,
));
}
}
Expand Down Expand Up @@ -178,11 +214,30 @@ fn enforce_impl_params_are_constrained(
// used elsewhere are not projected back out.
}

#[derive(Copy, Clone)]
enum ImplKind {
ImplTrait,
InherentImpl,
}

#[derive(Copy, Clone, Default)]
enum UnconstrainedReason {
/// Not used in the impl trait, self type, nor bound to any projection
#[default]
Other,
/// Bound to a projection, but the LHS was not constrained
BoundToUnconstrainedProjection,
/// Bound to a projection, but the LHS is the trait being implemented
BoundCircularly,
}

fn report_unused_parameter(
tcx: TyCtxt<'_>,
span: Span,
kind: &str,
name: Symbol,
impl_kind: ImplKind,
unconstrained_reason: UnconstrainedReason,
) -> ErrorGuaranteed {
let mut err = struct_span_code_err!(
tcx.dcx(),
Expand All @@ -194,6 +249,38 @@ fn report_unused_parameter(
name
);
err.span_label(span, format!("unconstrained {kind} parameter"));

match impl_kind {
ImplKind::ImplTrait => {
err.note(format!(
"to constrain `{name}`, use it in the implemented trait, in the self type, \
or in an equality with an associated type"
));
}
ImplKind::InherentImpl => {
err.note(format!(
"to constrain `{name}`, use it in the self type, \
or in an equality with an associated type"
));
}
}

match unconstrained_reason {
UnconstrainedReason::BoundToUnconstrainedProjection => {
err.note(format!(
"`{name}` is bound to an associated type, \
but the reference to the associated type itself uses unconstrained generic parameters"
));
}
UnconstrainedReason::BoundCircularly => {
err.note(format!(
"`{name}` is bound to an associated type, \
but the associated type is circularly defined in this impl"
));
}
UnconstrainedReason::Other => {}
}

if kind == "const" {
err.note(
"expressions using a const parameter must map each value to a distinct output value",
Expand All @@ -202,5 +289,6 @@ fn report_unused_parameter(
"proving the result of expressions other than the parameter are unique is not supported",
);
}

err.emit()
}
2 changes: 2 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,8 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
|
LL | impl<T> Allocator for DefaultAllocator {
| ^ unconstrained type parameter
|
= note: to constrain `T`, use it in the implemented trait, in the self type, or in an equality with an associated type

error: aborting due to 1 previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ 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
|
= note: to constrain `Q`, use it in the implemented trait, in the self type, or in an equality with an associated type

error: aborting due to 1 previous error

Expand Down
4 changes: 4 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,16 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
|
LL | impl<T: Tr> S<T::Assoc> {
| ^ unconstrained type parameter
|
= note: to constrain `T`, use it in the self type, or in an equality with an associated type

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
|
= note: to constrain `'a`, use it in the implemented trait, in the self type, or in an equality with an associated type

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait,
|
LL | impl<'a> Actor for () {
| ^^ unconstrained lifetime parameter
|
= note: to constrain `'a`, use it in the implemented trait, in the self type, or in an equality with an associated type

error: aborting due to 2 previous errors

Expand Down
1 change: 1 addition & 0 deletions tests/ui/async-await/issues/issue-78654.full.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ error[E0207]: the const parameter `H` is not constrained by the impl trait, self
LL | impl<const H: feature> Foo {
| ^^^^^^^^^^^^^^^^ unconstrained const parameter
|
= note: to constrain `H`, use it in the self type, or in an equality with an associated type
= 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

Expand Down
1 change: 1 addition & 0 deletions tests/ui/async-await/issues/issue-78654.min.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ error[E0207]: the const parameter `H` is not constrained by the impl trait, self
LL | impl<const H: feature> Foo {
| ^^^^^^^^^^^^^^^^ unconstrained const parameter
|
= note: to constrain `H`, use it in the self type, or in an equality with an associated type
= 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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ error[E0207]: the const parameter `N` is not constrained by the impl trait, self
LL | impl<'a, T, const N: usize> Iterator for ConstChunksExact<'a, T, {}> {
| ^^^^^^^^^^^^^^ unconstrained const parameter
|
= note: to constrain `N`, use it in the implemented trait, in the self type, or in an equality with an associated type
= 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

Expand Down
2 changes: 2 additions & 0 deletions tests/ui/const-generics/issues/issue-68366.full.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ error[E0207]: the const parameter `N` is not constrained by the impl trait, self
LL | impl <const N: usize> Collatz<{Some(N)}> {}
| ^^^^^^^^^^^^^^ unconstrained const parameter
|
= note: to constrain `N`, use it in the self type, or in an equality with an associated type
= 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

Expand All @@ -25,6 +26,7 @@ error[E0207]: the const parameter `N` is not constrained by the impl trait, self
LL | impl<const N: usize> Foo {}
| ^^^^^^^^^^^^^^ unconstrained const parameter
|
= note: to constrain `N`, use it in the self type, or in an equality with an associated type
= 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

Expand Down
2 changes: 2 additions & 0 deletions tests/ui/const-generics/issues/issue-68366.min.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ error[E0207]: the const parameter `N` is not constrained by the impl trait, self
LL | impl <const N: usize> Collatz<{Some(N)}> {}
| ^^^^^^^^^^^^^^ unconstrained const parameter
|
= note: to constrain `N`, use it in the self type, or in an equality with an associated type
= 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

Expand All @@ -34,6 +35,7 @@ error[E0207]: the const parameter `N` is not constrained by the impl trait, self
LL | impl<const N: usize> Foo {}
| ^^^^^^^^^^^^^^ unconstrained const parameter
|
= note: to constrain `N`, use it in the self type, or in an equality with an associated type
= 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

Expand Down
1 change: 1 addition & 0 deletions tests/ui/consts/rustc-impl-const-stability.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ error[E0207]: the const parameter `host` is not constrained by the impl trait, s
LL | impl const Default for Data {
| ^^^^^ unconstrained const parameter
|
= note: to constrain `host`, use it in the implemented trait, in the self type, or in an equality with an associated type
= 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

Expand Down
2 changes: 2 additions & 0 deletions tests/ui/error-codes/E0207.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
|
LL | impl<T: Default> Foo {
| ^ unconstrained type parameter
|
= note: to constrain `T`, use it in the self type, or in an equality with an associated type

error: aborting due to 1 previous error

Expand Down
3 changes: 3 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,9 @@ 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
|
= note: to constrain `U`, use it in the implemented trait, in the self type, or in an equality with an associated type
= note: `U` is bound to an associated type, but the reference to the associated type itself uses unconstrained generic parameters

error[E0309]: the parameter type `U` may not live long enough
--> $DIR/issue-87735.rs:34:21
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/generic-associated-types/bugs/issue-88526.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ error[E0207]: the type parameter `I` is not constrained by the impl trait, self
|
LL | impl<'q, Q, I, F> A for TestB<Q, F>
| ^ unconstrained type parameter
|
= note: to constrain `I`, use it in the implemented trait, in the self type, or in an equality with an associated type
= note: `I` is bound to an associated type, but the reference to the associated type itself uses unconstrained generic parameters

error[E0309]: the parameter type `F` may not live long enough
--> $DIR/issue-88526.rs:16:18
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ error[E0207]: the type parameter `T1` is not constrained by the impl trait, self
|
LL | impl <T, T1> Foo for T {
| ^^ unconstrained type parameter
|
= note: to constrain `T1`, use it in the implemented trait, in the self type, or in an equality with an associated type

error: aborting due to 3 previous errors

Expand Down
2 changes: 2 additions & 0 deletions tests/ui/impl-trait/in-trait/unconstrained-lt.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait,
|
LL | impl<'a, T> Foo for T {
| ^^ unconstrained lifetime parameter
|
= note: to constrain `'a`, use it in the implemented trait, in the self type, or in an equality with an associated type

error: aborting due to 1 previous error

Expand Down
2 changes: 2 additions & 0 deletions tests/ui/impl-trait/issues/issue-87340.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
|
LL | impl<T> X for () {
| ^ unconstrained type parameter
|
= note: to constrain `T`, use it in the implemented trait, in the self type, or in an equality with an associated type

error[E0282]: type annotations needed
--> $DIR/issue-87340.rs:11:23
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/impl-unused-rps-in-assoc-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait,
|
LL | impl<'a> Fun for Holder {
| ^^ unconstrained lifetime parameter
|
= note: to constrain `'a`, use it in the implemented trait, in the self type, or in an equality with an associated type

error: aborting due to 1 previous error

Expand Down
4 changes: 4 additions & 0 deletions tests/ui/impl-unused-tps-inherent.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
|
LL | impl<T> MyType {
| ^ unconstrained type parameter
|
= note: to constrain `T`, use it in the self type, or in an equality with an associated type

error[E0207]: the type parameter `U` is not constrained by the impl trait, self type, or predicates
--> $DIR/impl-unused-tps-inherent.rs:17:8
|
LL | impl<T,U> MyType1<T> {
| ^ unconstrained type parameter
|
= note: to constrain `U`, use it in the self type, or in an equality with an associated type

error: aborting due to 2 previous errors

Expand Down
Loading
Loading