Skip to content

Commit 3e264ba

Browse files
committed
Auto merge of rust-lang#11881 - y21:issue11880, r=Alexendoo
[`implied_bounds_in_impls`]: avoid linting on overlapping associated tys Fixes rust-lang#11880 Before this change, we were simply ignoring associated types (except for suggestion purposes), because of an incorrect assumption (see the comment that I also removed). For something like ```rs trait X { type T; } trait Y: X { type T; } // Can't constrain `X::T` through `Y` fn f() -> impl X<T = i32> + Y<T = u32> { ... } ``` We now avoid linting if the implied bound (`X<T = i32>`) "names" associated types that also exists in the implying trait (`trait Y`). Here that would be the case. But if we only wrote `impl X + Y<T = u32>` then that's ok because `X::T` was never constrained in the first place. I haven't really thought about how this interacts with GATs, but I think it's fine. Fine as in, it might create false negatives, but hopefully no false positives. (The diff is slightly annoying because of formatting things. Really the only thing that changed in the if chain is extracting the `implied_by_def_id` which is needed for getting associated types from the trait, and of course actually checking for overlap) cc `@Jarcho` ? idk if you want to review this or not. I assume you looked into this code a bit to find this bug. changelog: [`implied_bounds_in_impls`]: avoid linting when associated type from supertrait can't be constrained through the implying trait bound
2 parents 3e3a09e + 92616c0 commit 3e264ba

File tree

4 files changed

+104
-1
lines changed

4 files changed

+104
-1
lines changed

clippy_lints/src/implied_bounds_in_impls.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,15 @@ fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
317317
&& let implied_bindings = path.args.map_or([].as_slice(), |a| a.bindings)
318318
&& let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id()
319319
&& let Some(bound) = find_bound_in_supertraits(cx, def_id, implied_args, &supertraits)
320+
// If the implied bound has a type binding that also exists in the implied-by trait,
321+
// then we shouldn't lint. See #11880 for an example.
322+
&& let assocs = cx.tcx.associated_items(bound.trait_def_id)
323+
&& !implied_bindings.iter().any(|binding| {
324+
assocs
325+
.filter_by_name_unhygienic(binding.ident.name)
326+
.next()
327+
.is_some_and(|assoc| assoc.kind == ty::AssocKind::Type)
328+
})
320329
{
321330
emit_lint(cx, poly_trait, opaque_ty, index, implied_bindings, bound);
322331
}

tests/ui/implied_bounds_in_impls.fixed

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,4 +121,33 @@ mod issue11435 {
121121
fn f3() -> impl Trait4<i8, X = i32, Y = i128> {}
122122
}
123123

124+
fn issue11880() {
125+
trait X {
126+
type T;
127+
type U;
128+
}
129+
trait Y: X {
130+
type T;
131+
type V;
132+
}
133+
impl X for () {
134+
type T = i32;
135+
type U = String;
136+
}
137+
impl Y for () {
138+
type T = u32;
139+
type V = Vec<u8>;
140+
}
141+
142+
// Can't constrain `X::T` through `Y`
143+
fn f() -> impl X<T = i32> + Y {}
144+
fn f2() -> impl X<T = i32> + Y<T = u32> {}
145+
146+
// X::T is never constrained in the first place, so it can be omitted
147+
// and left unconstrained
148+
fn f3() -> impl Y {}
149+
fn f4() -> impl Y<T = u32> {}
150+
fn f5() -> impl Y<T = u32, U = String> {}
151+
}
152+
124153
fn main() {}

tests/ui/implied_bounds_in_impls.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,4 +121,33 @@ mod issue11435 {
121121
fn f3() -> impl Trait3<i8, i16, i64, X = i32, Y = i128> + Trait4<i8, X = i32> {}
122122
}
123123

124+
fn issue11880() {
125+
trait X {
126+
type T;
127+
type U;
128+
}
129+
trait Y: X {
130+
type T;
131+
type V;
132+
}
133+
impl X for () {
134+
type T = i32;
135+
type U = String;
136+
}
137+
impl Y for () {
138+
type T = u32;
139+
type V = Vec<u8>;
140+
}
141+
142+
// Can't constrain `X::T` through `Y`
143+
fn f() -> impl X<T = i32> + Y {}
144+
fn f2() -> impl X<T = i32> + Y<T = u32> {}
145+
146+
// X::T is never constrained in the first place, so it can be omitted
147+
// and left unconstrained
148+
fn f3() -> impl X + Y {}
149+
fn f4() -> impl X + Y<T = u32> {}
150+
fn f5() -> impl X<U = String> + Y<T = u32> {}
151+
}
152+
124153
fn main() {}

tests/ui/implied_bounds_in_impls.stderr

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,5 +192,41 @@ LL - fn f3() -> impl Trait3<i8, i16, i64, X = i32, Y = i128> + Trait4<i8, X
192192
LL + fn f3() -> impl Trait4<i8, X = i32, Y = i128> {}
193193
|
194194

195-
error: aborting due to 16 previous errors
195+
error: this bound is already specified as the supertrait of `Y`
196+
--> $DIR/implied_bounds_in_impls.rs:148:21
197+
|
198+
LL | fn f3() -> impl X + Y {}
199+
| ^
200+
|
201+
help: try removing this bound
202+
|
203+
LL - fn f3() -> impl X + Y {}
204+
LL + fn f3() -> impl Y {}
205+
|
206+
207+
error: this bound is already specified as the supertrait of `Y<T = u32>`
208+
--> $DIR/implied_bounds_in_impls.rs:149:21
209+
|
210+
LL | fn f4() -> impl X + Y<T = u32> {}
211+
| ^
212+
|
213+
help: try removing this bound
214+
|
215+
LL - fn f4() -> impl X + Y<T = u32> {}
216+
LL + fn f4() -> impl Y<T = u32> {}
217+
|
218+
219+
error: this bound is already specified as the supertrait of `Y<T = u32>`
220+
--> $DIR/implied_bounds_in_impls.rs:150:21
221+
|
222+
LL | fn f5() -> impl X<U = String> + Y<T = u32> {}
223+
| ^^^^^^^^^^^^^
224+
|
225+
help: try removing this bound
226+
|
227+
LL - fn f5() -> impl X<U = String> + Y<T = u32> {}
228+
LL + fn f5() -> impl Y<T = u32, U = String> {}
229+
|
230+
231+
error: aborting due to 19 previous errors
196232

0 commit comments

Comments
 (0)