Skip to content

rustc does not detect some recursive struct definitions #17431

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

Closed
pythonesque opened this issue Sep 21, 2014 · 1 comment · Fixed by #17815
Closed

rustc does not detect some recursive struct definitions #17431

pythonesque opened this issue Sep 21, 2014 · 1 comment · Fixed by #17815

Comments

@pythonesque
Copy link
Contributor

I think the impl is just necessary to get Rust to try to instantiate it (otherwise it assumes it's unused). This may be a duplicate of issue #4363.

use std::sync::Mutex;

struct Foo {
    foo: Mutex<Option<Foo>>,
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

task 'rustc' has overflowed its stack
@isabelmu
Copy link
Contributor

isabelmu commented Oct 5, 2014

I believe this and #17483 are separate from #4363. The issue seems to be a bug in the ty::is_type_representable logic, which should catch type definition recursions that lead to infinitely large types.

Here's a case this logic does catch:

struct Foo { foo: Option<Foo> }
impl Foo { fn bar(&self) {} }

And a snippet of the debug log:

DEBUG:rustc::middle::ty: is_type_representable: Foo
DEBUG:rustc::middle::ty: type_structurally_recursive: Foo
DEBUG:rustc::middle::ty: type_structurally_recursive: core::option::Option<Foo>
DEBUG:rustc::middle::ty: type_structurally_recursive: Foo

The second time that Foo is seen, an error is generated, so rustc won't overflow later on.

But here's a case that it fails to catch:

struct Foo { foo: Option<Option<Foo>> }
impl Foo { fn bar(&self) {} }

And the corresponding debug output:

DEBUG:rustc::middle::ty: is_type_representable: Foo
DEBUG:rustc::middle::ty: type_structurally_recursive: Foo
DEBUG:rustc::middle::ty: type_structurally_recursive: core::option::Option<core::option::Option<Foo>>
DEBUG:rustc::middle::ty: type_structurally_recursive: core::option::Option<Foo>

Somehow we aren't getting all the way through to the inner Foo, so we don't end up reporting an error.

Here's the output for the original example, with Mutex<Option<Foo>>:

DEBUG:rustc::middle::ty: is_type_representable: Foo
DEBUG:rustc::middle::ty: type_structurally_recursive: Foo
DEBUG:rustc::middle::ty: type_structurally_recursive: sync::lock::Mutex<core::option::Option<Foo>>
DEBUG:rustc::middle::ty: type_structurally_recursive: sync::raw::Mutex
DEBUG:rustc::middle::ty: type_structurally_recursive: sync::raw::Sem<collections::vec::Vec<sync::raw::WaitQueue>>
DEBUG:rustc::middle::ty: type_structurally_recursive: sync::mutex::Mutex
DEBUG:rustc::middle::ty: type_structurally_recursive: Box<sync::mutex::StaticMutex>
DEBUG:rustc::middle::ty: type_structurally_recursive: core::cell::UnsafeCell<sync::raw::SemInner<collections::vec::Vec<sync::raw::WaitQueue>>>
DEBUG:rustc::middle::ty: type_structurally_recursive: sync::raw::SemInner<collections::vec::Vec<sync::raw::WaitQueue>>
DEBUG:rustc::middle::ty: type_structurally_recursive: int
DEBUG:rustc::middle::ty: type_structurally_recursive: sync::raw::WaitQueue
DEBUG:rustc::middle::ty: type_structurally_recursive: sync::comm::Receiver<sync::comm::Sender<()>>
DEBUG:rustc::middle::ty: type_structurally_recursive: core::cell::UnsafeCell<sync::comm::Flavor<sync::comm::Sender<()>>>

I'll see whether I can fix this.

bors added a commit that referenced this issue Oct 18, 2014
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<Option<Foo>> }
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<Option<Foo>> }``` as SelfRecursive, even though we correctly identified ```struct Foo { foo: Option<Foo> }```. 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<DefId>``` to ```Vec<t>``` 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<Foo> }
struct Bar<T> { x: Bar<Foo> }
```

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.
lnicola pushed a commit to lnicola/rust that referenced this issue Jun 23, 2024
feat: add space after specific keywords in completion

fix rust-lang#17428.

When completing some specific keywords, it would be convenient if r-a could automatically add a space afterwards.

This PR implements this feature for the following keywords:

- Visibility: `pub`, `pub(crate)`, `pub(super)`, `pub(in xxx)`
- Pattern: `ref` / `mut`
- Others: `unsafe` / `for` / `where`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants