Repository navigation
Returning an impl Trait in associated type position results in error at point-of-use instead of point-of-definition #72614
Description
Activity
@rustbot modify labels to +A-impl-trait
- addedA-impl-traitArea: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch.Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch.
on May 26, 2020 - addedT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.T-langRelevant to the language teamRelevant to the language team
on May 31, 2020 Fixed in 1.49.0
@rustbot label E-needs-test
- addedE-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
on May 18, 2022 So the problem here was diagnostics, right? Since it now compiles fine, I think it's more of "hidden" than "fixed". Given that, I'm not sure if it's really worth adding a regression test, any thoughts?
Wait, how does this compile now?
fn fooshould be invalid, it's returning a type that doesn't implement the trait it claims to implement.A slight modification shows that it's now somehow leaking additional trait impls that weren't declared on the
impl Subtype (playground):pub trait Foo { type Bar: std::ops::Add<Self::Bar, Output = Self::Bar> + std::fmt::Debug; fn bar(self) -> Self::Bar; } impl<T: std::ops::Add<T, Output = T> + std::fmt::Debug> Foo for T { type Bar = T; fn bar(self) -> Self::Bar { self } } pub fn foo() -> impl Foo<Bar = impl std::ops::Sub<usize>> { 5usize } fn baz<T: Foo>(foo1: T, foo2: T) { dbg!(foo1.bar() + foo2.bar()); } fn main() { baz(foo(), foo()); }
Reacted by Ali MJ Al-NasrawyYea, this is pretty much that one backcompat hack that I want to eliminate:
should get removed and cratered.selcx.infcx().replace_opaque_types_with_inference_vars( - addedC-bugCategory: This is a bug.Category: This is a bug.and removedE-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
on May 23, 2022 @oli-obk thanks for the pointer! Are we ready to run crater or do you want to add some more patches before it?
Unless something comes up during the removal PR itself, this should be ready to go
Reacted by Yuki OkushiFixed in 1.49.0
Fixed by #73905 specifically.
The reason this works is that in the original example we replace all opaque types in the function signature with inference variables (for typeck, nowhere else!). So in
pub fn foo() -> impl Foo<Bar = impl std::ops::Sub<usize>> { 5usize }
impl std::ops::Sub<usize>becomes_1with an obligation that_1: Sub<usize>.- meaning
impl Foo<Bar = impl Sub<usize>>becomesimpl Foo<Bar = _1> - and then that becomes
_2with an obligation that_2: Fooand<_2 as Foo>::Bar = _1(from the impl) and<_2 as Foo>::Bar: Add<<_2 as Foo>::Bar>(from the trait)
now... during typeck we figure out that the return type of the function is
usizebecause of the5usize. So we figure out that_2is actually5usize.This means that we now have to prove
usize: Fooand<usize as Foo>::Bar = _1Luckily that is easy to prove, as there's
impl<T: std::ops::Add<T>> Foo for T { type Bar = T; fn bar(self) -> Self::Bar { self } }
that
usizetrivially satisfies (usize: Add<usize>does hold after all).Due to that impl we know
<usize as Foo>::Bar = usizeDue to our obligations we know
<_2 as Foo>:: Bar = _1+_2 = usizeso<usize as Foo>::Bar = _1makes us know that
_1 = usize.Furthermore we got
<_2 as Foo>::Bar: Add<<_2 as Foo>::Bar>from the trait, which we can now resolve to<usize as Foo>::Bar: Add<<usize as Foo>::Bar>which isusize: Add<usize>which holds.So this passes compilation so far. Now the question is why
foo().bar() - 4usize;passes, which was what originally failed before #73905. The reason that failed is that it tried to proveimpl Sub<usize>: Add<impl Sub<usize>>instead of just accepting that it must hold, as otherwisefoowould not have compiled. Note that you can't immediately assume thatAddholds (foo().bar() + 4usize;does not compile after all), but you can get there as shown in #72614 (comment)Reacted by Yuki Okushi and Ali MJ Al-NasrawyThanks for the detailed explanation, Oli! I've now understood it :)
So, the original issue here was a diagnostics one and the compiler now accepts that code, I think we could just clone this issue without a test.
If someone wants to add it, feel free to re-open and add theneeds-testlabel.I don't understand why it is allowed to bring this additional information in:
and
<_2 as Foo>::Bar: Add<<_2 as Foo>::Bar>(from the trait)The type
impl Sub<usize>should not allow leaking other non-auto traits it implements.There is no leaking happening. There is a weird situation where as long as you only know about the associated type, but not what it resolves to, you know about the Add but not the Sub. When you normalize the assoc type to its concrete type, you lose the information about the Add but gain the Sub.
Example (playground):
This should error at the definition of
foosinceimpl std::ops::Sub<usize>does not satisfy thestd::ops::Addbound required byFoo::Bar. Instead it errors at the use-site inmain, and if that line is deleted (e.g. if this is apub fnin a library) there is no error.