ICE on recursive type #4363
Comments
|
Thanks for the report. This is a known issue: #4218 -- but keep the bug reports coming, it's way better to deal with some duplicates than to not know about a bug :-) |
|
Sorry, I was reading too fast. I think this is a different bug. I'll look at it more carefully. |
ghost
assigned
|
I thought at first that the code here should be accepted, but no reflection, I think it shouldn't be. Instantiating With that said, of course, this should be a proper type error and not an ICE. |
|
@nikomatsakis - I'd appreciate a second opinion here. I still think the type shouldn't be allowed, but I'm not sure I can explain why. It's definitely instantiable, and if I change |
|
I'm thinking that an additional check, enforcing the rule "If types A and B are part of a mutually recursive cycle of types, then B must be used monomorphically when it occurs within A, and vice versa", would disallow this (thus, the program here would be rejected, but it would be accepted with |
|
Interesting. I hadn't thought about this possibility for infinite type generation, though it seems obvious in retrospect. @catamorphism I'm not sure what the best rule is. I think the type is both instantiable and representable as is (considering each type independently, the size of any given type is computable for a given Consider something like: Ignoring the question of instantiability, which can be addressed by adding some I would like to replace the trans check with a similar test. I remember trying to think of a good rule for trans and getting frustrated because any rule I came up with amounted to tracing the call graph in a fairly complex and deep way---basically redoing the work trans was already doing. The simplest thing of course for this case would be to do what trans does: expand the type and keep a set of types we've seen. If the recursion depth ever grows too high then we abort. As for the annoyance of having three distinct admission tests for types, I hear you. I was already annoyed that there seemed to be a need for two such tests. But they all test different things and I don't see an obvious way to combine them. |
|
Not sure if a more minimal example is needed, but here's one: (If you remove the option, it errors, telling you to use an option.) |
|
reproduced on 2013-03-22 |
|
non-critical for 0.6, de-milestoning |
|
reproduced on d35eb6e |
|
Nominating for milestone 1, well-defined. This is an annoying edge case, but I think it's important to specify what type definitions are acceptable. |
|
accepted for well-defined milestone |
|
@bblum's example is actually fixed by adding a non-representable error for structs (pull request #11839, issue #3779). Here is a new minimal example for this issue, usable as a test: Equally: |
|
Nominating for removal from 1.0 milestone, as this can be fixed backwards compatibly. |
|
P-high, not 1.0 |
|
This no longer causes an ICE. (Code modified slightly to match current syntax, disable style warnings, and add a |
|
Thanks @Twisol ! |
|
I can still make rustc overflow its stack by using the recursive type: fn main() {
let _ = FingerTree::Deep(Node { count: 0,
front: Digit { count: 0, content: [None, None, None, None] },
inner: Box::new(FingerTree::Single((1, 2))),
back: Digit { count: 0, content: [None, None, None, None] }}
);
} |
|
The compiler is currently infinite-looping (but not, AFAICT, stack-overflowing) on the @goffrie example. Actually maybe I'll open a fresh issue to track this. |
Note:
inner: ~FingerTree<(A,A)>,seems to be the problematic bit. If I make itinner: ~FingerTree<A>,it will compile.The text was updated successfully, but these errors were encountered: