Skip to content

Commit c014525

Browse files
committed
Auto merge of #148788 - TomtheCoder2:unconstrained-parameter-fix, r=davidtwco
Unconstrained parameter fix This PR is an attempt to solve the issue described in the issue #107295
2 parents 88ba7fb + d02b4c8 commit c014525

42 files changed

Lines changed: 734 additions & 21 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

compiler/rustc_hir/src/hir.rs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,6 +1088,93 @@ impl<'hir> Generics<'hir> {
10881088
bound_span.with_lo(bounds[bound_pos - 1].span().hi())
10891089
}
10901090
}
1091+
1092+
/// Computes the span representing the removal of a generic parameter at `param_index`.
1093+
///
1094+
/// This function identifies the correct slice of source code to delete so that the
1095+
/// remaining generic list remains syntactically valid (handling commas and brackets).
1096+
///
1097+
/// ### Examples
1098+
///
1099+
/// 1. **With a following parameter:** (Includes the trailing comma)
1100+
/// - Input: `<T, U>` (index 0)
1101+
/// - Produces span for: `T, `
1102+
///
1103+
/// 2. **With a previous parameter:** (Includes the leading comma and bounds)
1104+
/// - Input: `<T: Clone, U>` (index 1)
1105+
/// - Produces span for: `, U`
1106+
///
1107+
/// 3. **The only parameter:** (Includes the angle brackets)
1108+
/// - Input: `<T>` (index 0)
1109+
/// - Produces span for: `<T>`
1110+
///
1111+
/// 4. **Parameter with where-clause bounds:**
1112+
/// - Input: `fn foo<T, U>() where T: Copy` (index 0)
1113+
/// - Produces span for: `T, ` (The where-clause remains for other logic to handle).
1114+
pub fn span_for_param_removal(&self, param_index: usize) -> Span {
1115+
if param_index >= self.params.len() {
1116+
return self.span.shrink_to_hi();
1117+
}
1118+
1119+
let is_param_explicit = |par: &&GenericParam<'_>| match par.kind {
1120+
GenericParamKind::Type { .. }
1121+
| GenericParamKind::Const { .. }
1122+
| GenericParamKind::Lifetime { kind: LifetimeParamKind::Explicit } => true,
1123+
_ => false,
1124+
};
1125+
1126+
// Find the span of the type parameter.
1127+
if let Some(next) = self.params[param_index + 1..].iter().find(is_param_explicit) {
1128+
self.params[param_index].span.until(next.span)
1129+
} else if let Some(prev) = self.params[..param_index].iter().rfind(is_param_explicit) {
1130+
let mut prev_span = prev.span;
1131+
// Consider the span of the bounds with the previous generic parameter when there is.
1132+
if let Some(prev_bounds_span) = self.span_for_param_bounds(prev) {
1133+
prev_span = prev_span.to(prev_bounds_span);
1134+
}
1135+
1136+
// Consider the span of the bounds with the current generic parameter when there is.
1137+
prev_span.shrink_to_hi().to(
1138+
if let Some(cur_bounds_span) = self.span_for_param_bounds(&self.params[param_index])
1139+
{
1140+
cur_bounds_span
1141+
} else {
1142+
self.params[param_index].span
1143+
},
1144+
)
1145+
} else {
1146+
// Remove also angle brackets <> when there is just ONE generic parameter.
1147+
self.span
1148+
}
1149+
}
1150+
1151+
/// Returns the span of the `WherePredicate` associated with the given `GenericParam`, if any.
1152+
///
1153+
/// This looks specifically for predicates in the `where` clause that were generated
1154+
/// from the parameter definition (e.g., `T` in `where T: Bound`).
1155+
///
1156+
/// ### Example
1157+
///
1158+
/// - Input: `param` representing `T`
1159+
/// - Context: `where T: Clone + Default, U: Copy`
1160+
/// - Returns: Span of `T: Clone + Default`
1161+
fn span_for_param_bounds(&self, param: &GenericParam<'hir>) -> Option<Span> {
1162+
self.predicates
1163+
.iter()
1164+
.find(|pred| {
1165+
if let WherePredicateKind::BoundPredicate(WhereBoundPredicate {
1166+
origin: PredicateOrigin::GenericParam,
1167+
bounded_ty,
1168+
..
1169+
}) = pred.kind
1170+
{
1171+
bounded_ty.span == param.span
1172+
} else {
1173+
false
1174+
}
1175+
})
1176+
.map(|pred| pred.span)
1177+
}
10911178
}
10921179

10931180
/// A single predicate in a where-clause.

compiler/rustc_hir_analysis/src/errors.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ pub(crate) mod wrong_number_of_generic_args;
1515
mod precise_captures;
1616
pub(crate) use precise_captures::*;
1717

18+
pub(crate) mod remove_or_use_generic;
19+
1820
#[derive(Diagnostic)]
1921
#[diag("ambiguous associated {$assoc_kind} `{$assoc_ident}` in bounds of `{$qself}`")]
2022
pub(crate) struct AmbiguousAssocItem<'a> {
Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
use std::ops::ControlFlow;
2+
3+
use rustc_errors::{Applicability, Diag};
4+
use rustc_hir::def::DefKind;
5+
use rustc_hir::def_id::{DefId, LocalDefId};
6+
use rustc_hir::intravisit::{self, Visitor, walk_lifetime};
7+
use rustc_hir::{GenericArg, HirId, LifetimeKind, Path, QPath, TyKind};
8+
use rustc_middle::hir::nested_filter::All;
9+
use rustc_middle::ty::{GenericParamDef, GenericParamDefKind, TyCtxt};
10+
11+
use crate::hir::def::Res;
12+
13+
/// Use a Visitor to find usages of the type or lifetime parameter
14+
struct ParamUsageVisitor<'tcx> {
15+
tcx: TyCtxt<'tcx>,
16+
/// The `DefId` of the generic parameter we are looking for.
17+
param_def_id: DefId,
18+
found: bool,
19+
}
20+
21+
impl<'tcx> Visitor<'tcx> for ParamUsageVisitor<'tcx> {
22+
type NestedFilter = All;
23+
24+
fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
25+
self.tcx
26+
}
27+
28+
type Result = ControlFlow<()>;
29+
30+
fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) -> Self::Result {
31+
if let Some(res_def_id) = path.res.opt_def_id() {
32+
if res_def_id == self.param_def_id {
33+
self.found = true;
34+
return ControlFlow::Break(());
35+
}
36+
}
37+
intravisit::walk_path(self, path)
38+
}
39+
40+
fn visit_lifetime(&mut self, lifetime: &'tcx rustc_hir::Lifetime) -> Self::Result {
41+
if let LifetimeKind::Param(id) = lifetime.kind {
42+
if let Some(local_def_id) = self.param_def_id.as_local() {
43+
if id == local_def_id {
44+
self.found = true;
45+
return ControlFlow::Break(());
46+
}
47+
}
48+
}
49+
walk_lifetime(self, lifetime)
50+
}
51+
}
52+
53+
/// Adds a suggestion to a diagnostic to either remove an unused generic parameter, or use it.
54+
///
55+
/// # Examples
56+
///
57+
/// - `impl<T> Struct { ... }` where `T` is unused -> suggests removing `T` or using it.
58+
/// - `impl<T> Struct { // T used in here }` where `T` is used in the body but not in the self type -> suggests adding `T` to the self type and struct definition.
59+
/// - `impl<T> Struct { ... }` where the struct has a generic parameter with a default -> suggests adding `T` to the self type.
60+
pub(crate) fn suggest_to_remove_or_use_generic(
61+
tcx: TyCtxt<'_>,
62+
diag: &mut Diag<'_>,
63+
impl_def_id: LocalDefId,
64+
param: &GenericParamDef,
65+
is_lifetime: bool,
66+
) {
67+
let node = tcx.hir_node_by_def_id(impl_def_id);
68+
let hir_impl = node.expect_item().expect_impl();
69+
70+
let Some((index, _)) = hir_impl
71+
.generics
72+
.params
73+
.iter()
74+
.enumerate()
75+
.find(|(_, par)| par.def_id.to_def_id() == param.def_id)
76+
else {
77+
return;
78+
};
79+
80+
// Get the Struct/ADT definition ID from the self type
81+
let struct_def_id = if let TyKind::Path(QPath::Resolved(_, path)) = hir_impl.self_ty.kind
82+
&& let Res::Def(DefKind::Struct | DefKind::Enum | DefKind::Union, def_id) = path.res
83+
{
84+
def_id
85+
} else {
86+
return;
87+
};
88+
89+
// Count how many generic parameters are defined in the struct definition
90+
let generics = tcx.generics_of(struct_def_id);
91+
let total_params = generics
92+
.own_params
93+
.iter()
94+
.filter(|p| {
95+
if is_lifetime {
96+
matches!(p.kind, GenericParamDefKind::Lifetime)
97+
} else {
98+
matches!(p.kind, GenericParamDefKind::Type { .. })
99+
}
100+
})
101+
.count();
102+
103+
// Count how many arguments are currently provided in the impl
104+
let mut provided_params = 0;
105+
let mut last_segment_args = None;
106+
107+
if let TyKind::Path(QPath::Resolved(_, path)) = hir_impl.self_ty.kind
108+
&& let Some(seg) = path.segments.last()
109+
&& let Some(args) = seg.args
110+
{
111+
last_segment_args = Some(args);
112+
provided_params = args
113+
.args
114+
.iter()
115+
.filter(|arg| match arg {
116+
GenericArg::Lifetime(_) => is_lifetime,
117+
GenericArg::Type(_) => !is_lifetime,
118+
_ => false,
119+
})
120+
.count();
121+
}
122+
123+
let mut visitor = ParamUsageVisitor { tcx, param_def_id: param.def_id, found: false };
124+
for item_ref in hir_impl.items {
125+
let _ = visitor.visit_impl_item_ref(item_ref);
126+
if visitor.found {
127+
break;
128+
}
129+
}
130+
let is_param_used = visitor.found;
131+
132+
let mut suggestions = vec![];
133+
134+
// Option A: Remove (Only if not used in body)
135+
if !is_param_used {
136+
suggestions.push((hir_impl.generics.span_for_param_removal(index), String::new()));
137+
}
138+
139+
// Option B: Suggest adding only if there's an available parameter in the struct definition
140+
// or the parameter is already used somewhere, then we suggest adding to the impl struct and the struct definition
141+
if provided_params < total_params || is_param_used {
142+
if let Some(args) = last_segment_args {
143+
// Struct already has <...>, append to it
144+
suggestions.push((args.span().unwrap().shrink_to_hi(), format!(", {}", param.name)));
145+
} else if let TyKind::Path(QPath::Resolved(_, path)) = hir_impl.self_ty.kind {
146+
// Struct has no <...> yet, add it
147+
let seg = path.segments.last().unwrap();
148+
suggestions.push((seg.ident.span.shrink_to_hi(), format!("<{}>", param.name)));
149+
}
150+
if is_param_used {
151+
// If the parameter is used in the body, we also want to suggest adding it to the struct definition if it's not already there
152+
let struct_span = tcx.def_span(struct_def_id);
153+
let last_param_span = if let Some(local_def_id) = struct_def_id.as_local() {
154+
let hir_struct = tcx.hir_node_by_def_id(local_def_id).expect_item().expect_struct();
155+
hir_struct.1.params.last().map(|param| param.span)
156+
} else {
157+
let generics = tcx.generics_of(struct_def_id);
158+
generics.own_params.last().map(|param| tcx.def_span(param.def_id))
159+
};
160+
161+
if let Some(last_param_span) = last_param_span {
162+
suggestions.push((last_param_span.shrink_to_hi(), format!(", {}", param.name)));
163+
} else {
164+
suggestions.push((struct_span.shrink_to_hi(), format!("<{}>", param.name)));
165+
}
166+
}
167+
}
168+
169+
if suggestions.is_empty() {
170+
return;
171+
}
172+
173+
let parameter_type = if is_lifetime { "lifetime" } else { "type" };
174+
if is_param_used {
175+
let msg = format!(
176+
"use the {} parameter `{}` in the `{}` type and use it in the type definition",
177+
parameter_type,
178+
param.name,
179+
tcx.def_path_str(struct_def_id)
180+
);
181+
diag.multipart_suggestion(
182+
msg,
183+
vec![
184+
(suggestions[0].0, suggestions[0].1.clone()),
185+
(suggestions[1].0, suggestions[1].1.clone()),
186+
],
187+
Applicability::MaybeIncorrect,
188+
);
189+
} else {
190+
let msg = if suggestions.len() == 2 {
191+
format!("either remove the unused {} parameter `{}`", parameter_type, param.name)
192+
} else {
193+
format!("remove the unused {} parameter `{}`", parameter_type, param.name)
194+
};
195+
diag.span_suggestion(
196+
suggestions[0].0,
197+
msg,
198+
suggestions[0].1.clone(),
199+
Applicability::MaybeIncorrect,
200+
);
201+
if suggestions.len() == 2 {
202+
let msg = format!("or use it");
203+
diag.span_suggestion(
204+
suggestions[1].0,
205+
msg,
206+
suggestions[1].1.clone(),
207+
Applicability::MaybeIncorrect,
208+
);
209+
}
210+
};
211+
}

compiler/rustc_hir_analysis/src/impl_wf_check.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use rustc_span::{ErrorGuaranteed, kw};
2121

2222
use crate::constrained_generic_params as cgp;
2323
use crate::errors::UnconstrainedGenericParameter;
24+
use crate::errors::remove_or_use_generic::suggest_to_remove_or_use_generic;
2425

2526
mod min_specialization;
2627

@@ -177,6 +178,7 @@ pub(crate) fn enforce_impl_lifetime_params_are_constrained(
177178
);
178179
}
179180
}
181+
suggest_to_remove_or_use_generic(tcx, &mut diag, impl_def_id, param, true);
180182
res = Err(diag.emit());
181183
}
182184
}
@@ -242,6 +244,7 @@ pub(crate) fn enforce_impl_non_lifetime_params_are_constrained(
242244
const_param_note2: const_param_note,
243245
});
244246
diag.code(E0207);
247+
suggest_to_remove_or_use_generic(tcx, &mut diag, impl_def_id, &param, false);
245248
res = Err(diag.emit());
246249
}
247250
}

tests/rustdoc-ui/not-wf-ambiguous-normalization.stderr

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ error[E0207]: the type parameter `T` is not constrained by the impl trait, self
22
--> $DIR/not-wf-ambiguous-normalization.rs:14:6
33
|
44
LL | impl<T> Allocator for DefaultAllocator {
5-
| ^ unconstrained type parameter
5+
| -^-
6+
| ||
7+
| |unconstrained type parameter
8+
| help: remove the unused type parameter `T`
69

710
error: aborting due to 1 previous error
811

tests/rustdoc-ui/synthetic-auto-trait-impls/unconstrained-param-in-impl-ambiguity.stderr

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ error[E0207]: the type parameter `Q` is not constrained by the impl trait, self
22
--> $DIR/unconstrained-param-in-impl-ambiguity.rs:7:13
33
|
44
LL | unsafe impl<Q: Trait> Send for Inner {}
5-
| ^ unconstrained type parameter
5+
| -^--------
6+
| ||
7+
| |unconstrained type parameter
8+
| help: remove the unused type parameter `Q`
69

710
error: aborting due to 1 previous error
811

tests/ui/associated-inherent-types/hr-do-not-blame-outlives-static-ice.stderr

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@ error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait,
33
|
44
LL | impl<'a> Foo<fn(&())> {
55
| ^^ unconstrained lifetime parameter
6+
|
7+
help: use the lifetime parameter `'a` in the `Foo` type and use it in the type definition
8+
|
9+
LL ~ struct Foo<T, 'a>(T);
10+
LL |
11+
LL ~ impl<'a> Foo<fn(&()), 'a> {
12+
|
613

714
error[E0308]: mismatched types
815
--> $DIR/hr-do-not-blame-outlives-static-ice.rs:13:11

tests/ui/associated-inherent-types/inherent-assoc-ty-mismatch-issue-153539.stderr

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ error[E0207]: the type parameter `X` is not constrained by the impl trait, self
22
--> $DIR/inherent-assoc-ty-mismatch-issue-153539.rs:9:6
33
|
44
LL | impl<X> S<'_> {
5-
| ^ unconstrained type parameter
5+
| -^-
6+
| ||
7+
| |unconstrained type parameter
8+
| help: remove the unused type parameter `X`
69

710
error[E0308]: mismatched types
811
--> $DIR/inherent-assoc-ty-mismatch-issue-153539.rs:17:5

0 commit comments

Comments
 (0)