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

#92917 breaks type inference on GATs #93874

Closed
MingweiSamuel opened this issue Feb 10, 2022 · 6 comments · Fixed by #93892
Closed

#92917 breaks type inference on GATs #93874

MingweiSamuel opened this issue Feb 10, 2022 · 6 comments · Fixed by #93892
Labels
C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@MingweiSamuel
Copy link
Contributor

MingweiSamuel commented Feb 10, 2022

Seems to be caused by #92917 @jackh726

Not a fix, but a mitigation to prevent a backwards-compatible hazard where we normalize using a predicate only because it's the only one available, but shouldn't.

So this regression seems to be intentional, but I want to make sure because even very basic inference fails now, making any use of GATs very fill up with explicit type annotations. It doesn't look so bad in this example, but with complex types it's bad, and I think there's a more complex example the compiler can't even tell what _x is at all, but I'm still working on that.

Code

#![feature(generic_associated_types)]

pub trait Build {
    type Output<O>;
    fn build<O>(self, input: O) -> Self::Output<O>;
}

pub struct IdentityBuild;
impl Build for IdentityBuild {
    type Output<O> = O;
    fn build<O>(self, input: O) -> Self::Output<O> {
        input
    }
}

fn a() {
    let _x: u8 = IdentityBuild.build(10);
    // ^type mismatch resolving `<IdentBuild as Build>::Output<i32> == u8`
}

fn b() {
    let _x: Vec<u8> = IdentityBuild.build(Vec::new());
    // ^type annotations needed: cannot satisfy `<IdentBuild as Build>::Output<Vec<_>> == Vec<u8>`
    // cannot satisfy `<IdentBuild as Build>::Output<Vec<_>> == Vec<u8>`
}

// edit: added
fn c() {
    let mut f = IdentityBuild.build(|| ());
    (f)();
    // ^type annotations needed
    // type must be known at this pointrustcE0282
    // main.rs(17, 9): consider giving `x` a type
}

pub fn main() {
    a();
    b();
    c();
}

I expected to see this happen: compiles without error

Instead, this happened: Type inference fails, Output<O> = O seems to be ignored or not understood by the compiler.

Version it worked on

It most recently worked on:

Version with regression

rustc --version --verbose:

rustc 1.60.0-nightly (e7aca8959 2022-02-09)
binary: rustc
commit-hash: e7aca895980f25f6d2d3c48e10fd04656764d1e4
commit-date: 2022-02-09
host: x86_64-pc-windows-msvc
release: 1.60.0-nightly
LLVM version: 13.0.0

Backtrace

(no crash)

@MingweiSamuel MingweiSamuel added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Feb 10, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Feb 10, 2022
@MingweiSamuel
Copy link
Contributor Author

MingweiSamuel commented Feb 10, 2022

Ah yeah, you also now have to make an existential type to name every unnameable type (e.g. types containing closures) if they go through a GAT (have to use #![feature(type_alias_impl_trait)])

#![feature(generic_associated_types)]
#![feature(type_alias_impl_trait)]

pub trait Build {
    type Output<O>;
    fn build<O>(self, input: O) -> Self::Output<O>;
}

pub struct IdentityBuild;
impl Build for IdentityBuild {
    type Output<O> = O;
    fn build<O>(self, input: O) -> Self::Output<O> {
        input
    }
}

fn x() {
    let mut f = IdentityBuild.build(|| ());
    (f)();
    // ^type annotations needed
    // type must be known at this pointrustcE0282
    // main.rs(17, 9): consider giving `x` a type
}
fn y() {
    type MyFn = impl FnOnce();
    let f: MyFn = IdentityBuild.build(|| ());
    (f)();
    // Compiles
}

pub fn main() {
    x();
}

@jackh726
Copy link
Member

"type mismatch resolving <IdentityBuild as Build>::Output<i32> == u8"

I think this probably can be solved by normalizing somewhere where we aren't now; there aren't inference variables here.

"type annotations needed: cannot satisfy <IdentityBuild as Build>::Output<Vec<_>> == Vec<u8>"

This, I'm not sure about, since we do have an inference variable. Still, I think we can normalize still and end up with the Vec<_> == Vec<u8> requirement - which is trivial.

@compiler-errors
Copy link
Member

compiler-errors commented Feb 10, 2022

@jackh726 cc: the fn trait normalization I added in #93361
(specifically normalizing the expected type of a projection obligation)

@jackh726
Copy link
Member

Does #93361 fix this?

@compiler-errors
Copy link
Member

You know what? It actually doesn't, sorry, should've checked first before I brought it up as a possibility.

The first error gets reported as <IdentityBuild as Build>::Output<i32> == u8 in the diagnostic, but the infcx.eq in project_and_unify_type actually tries to equate i32 == u8. Other than the diagnostic being a bit wrong, I wonder why numeric fallback is occurring here...

The second error ends up happening because of an ambiguity error during selection. I can look more into why that's failing.

@compiler-errors
Copy link
Member

So the second error is ambiguous because that type variable can't be proved Sized... (that obligation comes from a WF predicate I think?) 🤔

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 17, 2022
@bors bors closed this as completed in 1e2f63d Feb 19, 2022
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants