rust-lang/rust — pull request #17815 by typelist (2014-10-06 07:24:49+00:00)
Title: Bug fixes for struct+enum representability check

The representability-checking routine ```is_type_representable``` failed to detect structural recursion in some cases, leading to stack overflow later on.

The first problem was in the loop in the ```find_nonrepresentable``` function. We were improperly terminating the iteration if we saw a ```ContainsRecursive``` condition. We should have kept going in case a later member of the struct (or enum, etc) being examined was ```SelfRecursive```. The example from #17431 triggered this issue:

```rust
use std::sync::Mutex;
struct Foo { foo: Mutex> }
impl Foo { fn bar(self) {} }
fn main() {}
```

I'm not 100% sure, but I think the ```ty_enum``` case of ```fn type_structurally_recursive``` had a similar problem, since it could ```break``` on ```ContainsRecursive``` before looking at all variants. I've replaced this with a ```flat_map``` call.

The second problem was that we were failing to identify code like ```struct Foo { foo: Option> }``` as SelfRecursive, even though we correctly identified ```struct Foo { foo: Option }```. This was caused by using DefId's for the ```ContainsRecursive``` check, which meant the nested ```Option```s were identified as illegally recursive (because ```ContainsRecursive``` is not an error, we would then keep compiling and eventually hit a stack overflow).

In order to make sure that we can recurse through the different ```Option``` invocations, I've changed the type of ```seen``` from ```Vec``` to ```Vec``` and added a separate ```same_type``` function to check whether two types are the same when generics are taken into account. Now we only return ```ContainsRecursive``` when this stricter check is satisfied. (There's probably a better way to do this, and I'm not sure my code is entirely correct--but my knowledge of rustc internals is pretty limited, so any help here would be appreciated!)

Note that the ```SelfRecursive``` check is still comparing ```DefId```s--this is necessary to prevent code like this from being allowed:

```rust
struct Foo { x: Bar }
struct Bar { x: Bar }
```

All four of the new ```issue-17431``` tests cause infinite recursion on master, and errors with this pull request. I wrote the extra ```issue-3008-4.rs``` test to make sure I wasn't introducing a regression.

Fixes #17431.