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

Order-dependent type inference failure with match #25165

Open
pythonesque opened this issue May 7, 2015 · 10 comments
Open

Order-dependent type inference failure with match #25165

pythonesque opened this issue May 7, 2015 · 10 comments
Labels
A-type-system Area: Type system C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@pythonesque
Copy link
Contributor

Not sure if this has been reported yet, but it really seems like Rust should be able to do better here.

This also works if we replace the match with an if let and move the res assignment below it.

Note that this is a reduced testcase and therefore contrived, but the original issue can be seen here.


struct Foo;

impl Foo {
    fn foo(self) -> () { () }
}

fn main() {
    let mut res = None; // Compiles if we use `None::<Foo>`;
    match res {
        Some(winning) => { let _: () = winning.foo(); },
        None => res = Some(Foo), // Compiles if we move the None clause above the Some.
    }
}
<anon>:10:40: 10:53 error: the type of this value must be known in this context
<anon>:10         Some(winning) => { let _: () = winning.foo(); },
                                                 ^~~~~~~~~~~~~
@pythonesque
Copy link
Contributor Author

I think #22165 may be related too (in particular #22165 (comment)).

@pythonesque
Copy link
Contributor Author

Looks identical to #19855.

@nikomatsakis Could you clarify what would be required to fix this? It sort of feels like a natural candidate for a bidirectional typechecking approach.

I strongly disagree that this is expected behavior from the user side of things, even if it falls out naturally from our current inference system. I've run into this many times before and it's always felt like a pretty substantial wart.

@steveklabnik steveklabnik added the A-type-system Area: Type system label May 7, 2015
@arielb1
Copy link
Contributor

arielb1 commented May 7, 2015

@pythonesque

We would probably have to run inference in a loop of some sort.

@ebfull
Copy link
Contributor

ebfull commented Nov 14, 2015

I ran into this in #29834. It definitely is a wart. In my case it prevents me from making wrappers around closures without breaking the inference logic:

struct Wrapper<F>(F);
struct Foo;

impl Foo {
    fn stuff(self) { }
}

fn api<F: Fn(Foo)>(f: Wrapper<F>) { }

fn main() {
    api(Wrapper(|b| {
        b.stuff(); //~ ERROR the type of this value must be known in this context
    }))
}

... which makes my API really ugly. I too would like to know what's involved with changing this behavior. Perhaps the new MIR stuff can help?

@arielb1
Copy link
Contributor

arielb1 commented Nov 15, 2015

@ebfull

MIR runs only after typechecking, so it can't help.

Your example should be able to be made to work, though.

@arielb1
Copy link
Contributor

arielb1 commented Nov 15, 2015

cc @nikomatsakis

@nikomatsakis
Copy link
Contributor

The reason that this doesn't work is that the type checker's current design includes various points where it cannot make progress unless it has enough type information. I think the fix would be to reimplement the type checker so that everything is a generic constraint that can be deferred; this way, we would just postpone processing winning.foo() until we learn the type of winning from the other arm. I've been wanting to do some general refactoring of typeck in this direction for some time, but it'd be a big project. I was hoping though to generally refactor typeck as part of the work on MIR -- specifically, to strip out region processing and move that to MIR -- and I was hoping to explore this at that time.

@arielb1
Copy link
Contributor

arielb1 commented Dec 1, 2015

@nikomatsakis

This may be somewhat problematic because of coercions and autoderefs. Anyway, I feel like regionck is separated enough from typeck.

@nikomatsakis
Copy link
Contributor

@arielb1 hmm, yes, I agree there are big challenges around coercion. I was hoping we could work around those by being capturing the source order somewhat in the graph, but it may be that it prevents the whole idea from really panning out.

I strongly disagree about regionck and typeck though. But we can discuss at another time, it's not really that relevant to this bug.

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 22, 2017
@pnkfelix pnkfelix added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Mar 18, 2019
@pnkfelix
Copy link
Member

(switched this from C-bug to C-enhancement, because I do not think this represent anything incorrect in our existing type inference; it just is one of many annoying short-comings that we might attempt to address in the future.)

@Noratrieb Noratrieb added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants