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

typeck hang with recursive type #2063

Closed
jruderman opened this issue Mar 28, 2012 · 19 comments
Closed

typeck hang with recursive type #2063

jruderman opened this issue Mar 28, 2012 · 19 comments
Assignees
Labels
A-typesystem Area: The type system P-medium Medium priority

Comments

@jruderman
Copy link
Contributor

This hangs the compiler in typeck:

enum t = @t;
fn main() {
    let x : t;
    x.next = x;
}
@nikomatsakis
Copy link
Contributor

probably my fault.

@nikomatsakis
Copy link
Contributor

Not at all what I expected it to be. Rather, a loop in autoderef. I will fix the loop but I suppose it might also make sense to reject this type as uninstanitable---there should be some enum variant that does not require an instance of the enum itself. But I am not sure this is worth checking in general: after all, the user could never successfully construct an instance of that type, and to check it in general requires expanding and traversing enums and so forth.

@jruderman
Copy link
Contributor Author

I think rejecting the type makes more sense. If we reject the type immediately, we won't need to change the autoderef loop, right?

@nikomatsakis
Copy link
Contributor

That's the idea. I independently came to the same conclusion. In particular, there are multiple autoderef loops (which ought to be consolidated more, but still) and I don't like the idea of this same kind of bug creeping up in another of them.

@nikomatsakis
Copy link
Contributor

Also, it's just a nicer error message.

@nikomatsakis
Copy link
Contributor

Unfortunately, we still need the loop check in the autoderef loop for typeck, because the actual checking that the enum is instantiable and the checking of the fn body occur in the same pass over the AST. We could change it to have three passes (one to compute types and generate info, one to check for instantiability, and one to check fn bodies and so forth) but that seems annoying.

@jruderman
Copy link
Contributor Author

Could the pass bail out early after encountering an uninstantiable type?

@nikomatsakis
Copy link
Contributor

@jruderman No, the problem is that the autoderef loop could occur before we get around to checking whether the type is instantiable. As I said, I could restructure the pass to make sure we check all types for instantiability (and possibly other properties) before we delve into function bodies and expressions, but otherwise I think we still need to be careful when autoderef'ing. Such a restructuring might be a good idea though.

@nikomatsakis
Copy link
Contributor

Fixed in 23f92ea

@msullivan
Copy link
Contributor

Should this be closed, then?

@nikomatsakis
Copy link
Contributor

Yes.

@alexcrichton
Copy link
Member

The issue-2063.rs test has been xfail'd for some time now, but this issue appears to be cropping up again.

@alexcrichton alexcrichton reopened this May 6, 2013
@bblum
Copy link
Contributor

bblum commented Jul 22, 2013

That test case emits an error before hanging: this type cannot be instantiated without an instance of itself

One easy way to fix this would be to make that error emission site do a span_fatal, so that the compilation phase stops running before it gets to the hang. Not sure if this is the right thing to do.

Nominating well-covered.

@nikomatsakis
Copy link
Contributor

It might be. The idea of this error was that code in typeck had to worry about this kind of infinite loop case, but not other code. But maybe it's too much trouble even within typeck.

@graydon
Copy link
Contributor

graydon commented Aug 1, 2013

accepted for production-ready milestone

@catamorphism
Copy link
Contributor

High, but not 1.0

@alexcrichton
Copy link
Member

Fixing this bug should also fix this code:

enum A {
    A(B)
}

enum B {
    B1, B2(B)
}

fn main() {}

@isabelmu
Copy link
Contributor

Not sure under what circumstances issue-2063.rs was originally xfailed, but as of now it's missing #[feature(managed_boxes)]; and emits an extra error: "type t does not implement any method in scope named to_str". I enabled managed boxes (and tried adding a &self parameter to the to_str call), but could not get the compiler to hang. It emits the expected error ("this type cannot be instantiated without an instance of itself; consider using Option<t>") and quits.

The recursive-enum stack overflow issue pointed referenced by @alexcrichton is described in #3008, and addressed in pull request #11839. I don't think it's related to this.

@flaper87
Copy link
Contributor

issue-2063.rs is not xfail'd anymore and tests introduced in #3008 already test @alexcrichton example. Look at issue-3008-1.rs

This seems to be fixed, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

9 participants