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

Stack overflow when bounding FnOnce with Send + 'static #23003

Closed
carllerche opened this issue Mar 3, 2015 · 12 comments
Closed

Stack overflow when bounding FnOnce with Send + 'static #23003

carllerche opened this issue Mar 3, 2015 · 12 comments
Labels
A-associated-items Area: Associated items (types, constants & functions) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@carllerche
Copy link
Member

I'm not exactly sure what is happening, but I thought I would file an issue. I am attempting to update syncbox (https://github.com/carllerche/syncbox). I have to add 'static bounds to types that were previously only bound by Send, however it seems to be causing stack overflow errors in rustc.

I could not make a simple repro, but here is the code in a single file:

https://gist.github.com/carllerche/7961c85b171c829cd625

cc @nikomatsakis

@jdm jdm added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Mar 3, 2015
@carllerche
Copy link
Member Author

mr. @alexcrichton has used creduce to reduce the case:

use std::marker::PhantomData;
use std::sync::atomic::AtomicUsize;
struct Receipt<A: Async> {
    core: (),
    count: u64,
    marker: PhantomData<A>,
}
struct Complete<T, E> {
    core: Option<Core<T, E>>,
}
impl <T: Send, E: Send> Async for Complete<T, E> {type
    Cancel
    =
    Receipt<Complete<T, E>>;
}

impl <T: Send, E: Send> Cancel<Complete<T, E>> for Receipt<Complete<T, E>> {
    fn cancel(self) -> Option<Complete<T, E>> { }
}

struct Core<T, E> {
    refs: AtomicUsize,
    state: AtomicState,
    consumer_wait: Option<Callback<T, E>>,
    producer_wait: Option<Callback<T, E>>,
    val: AsyncResult<T, E>, //
}
struct AtomicState;
type Callback<T, E> = Box<BoxedReceive<Core<T, E>>>;
trait Async {type Cancel: Cancel<Self>; }
trait Cancel<A> {
    fn cancel(self) -> Option<A>;
}
type AsyncResult<T, E> = Result<T, AsyncError<E>>;
enum AsyncError<E> { ExecutionError(E), }
trait BoxedReceive<T> {
    fn receive_boxed(Box<Self>, T);
}

@nikomatsakis
Copy link
Contributor

This was caused by fixing #22246 (PR #22436 in particular). I have to think on what's the best fix here.

@nikomatsakis
Copy link
Contributor

Just FYI, you can workaround the problem by removing the A:Async bound from Receipt. What happens is that, per #22246, for Result<A> to be legal in some context with lifetime 'x, it winds up requiring that A::Cancel :'x, which in turn isresult : 'x`. Probably the fix is just to check that we don't reach the same type again. This wasn't necessary before because types were always of finite size.

@nikomatsakis
Copy link
Contributor

So adding a cache is probably the way to fix this crash. What worries me is that the set of types might be ever-expanding. We could add a recursion depth to counter that but it feels like it might rule out potentially legitimate types? I guess they can probably always be phrased differently. Let me try to work up a concrete example of what I mean.

@nikomatsakis
Copy link
Contributor

Reduced more:

use std::marker::PhantomData;

trait Async {
    type Cancel;
}

struct Receipt<A:Async> {
    marker: PhantomData<A>,
}

struct Complete {
    core: Option<()>,
}

impl Async for Complete {
    type Cancel = Receipt<Complete>;
}

fn foo(r: Receipt<Complete>) { }

fn main() { }

@nikomatsakis nikomatsakis added the A-associated-items Area: Associated items (types, constants & functions) label Mar 4, 2015
@nikomatsakis
Copy link
Contributor

This variant generates (at least in my version, which contains a fix for the above issue), an overflow error:

use std::marker::PhantomData;

trait Async {
    type Cancel;
}

struct Receipt<A:Async> {
    marker: PhantomData<A>,
}

struct Complete<B> {
    core: Option<B>,
}

impl<B> Async for Complete<B> {
    type Cancel = Receipt<Complete<Option<B>>>;
}

fn foo(r: Receipt<Complete<()>>) { }

fn main() { }

The reason is because Complete<()>::Cancel, normalized, is Receipt<Complete<Option<()>>>, which in turn expands out to Receipt<Complete<Option<Option<()>>>> and so forth.

I am not sure if this overflow error is entirely legitimate -- by which I mean, it falls out of the rules as we have them today, but it doesn't feel entirely necessary.

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Mar 4, 2015
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 4, 2015
 When generating WF criteria, do not visit the same type more than once. Fixes an infinite stack overflow (rust-lang#23003).

r? @aturon
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 5, 2015
 When generating WF criteria, do not visit the same type more than once. Fixes an infinite stack overflow (rust-lang#23003).

r? @aturon
@arielb1
Copy link
Contributor

arielb1 commented Mar 5, 2015

A mutated version (without Sized) causes a rustc stack overflow/illegal instruction:

use std::marker;

trait Async {
    type Cancel;
}

struct Receipt<A: ?Sized + Async> {
    marker: marker::PhantomData<A>,
}

struct Complete<B: ?Sized> {
    core: marker::PhantomData<B>,
}

impl<B: ?Sized> Async for Complete<B> {
    type Cancel = Receipt<Complete<Option<B>>>;
}

fn foo(r: Receipt<Complete<()>>) { }

fn main() { }

The overflow is weird, because you can have impl<T> Foo for S<T> { type Ty = S<Option<T>>; }

@frewsxcv
Copy link
Member

This is still an issue using Rust 1.0 beta 2

Full error message (compiling @nikomatsakis's latest code block): https://gist.githubusercontent.com/frewsxcv/c74c589b42675c5e5df7/raw/f0d4224d119fcf51db9d48c83fa266c0e955f4bd/gistfile1.txt

~/Downloads $ rustc --version
rustc 1.0.0-beta.2 (e9080ec39 2015-04-16) (built 2015-04-16)

@frewsxcv
Copy link
Member

The I-ICE label can probably be removed, unless there is still an ICE issue here

@apasel422
Copy link
Contributor

It appears that all example code in this issue now compiles on nightly.

@steveklabnik steveklabnik added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. and removed I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ labels Oct 24, 2015
@steveklabnik
Copy link
Member

Okay! As such, let's add e-needstest, and let's get a test for @nikomatsakis 's last code block as a representatitve of this issue. I'd be happy to help anyone who wants to tackle this.

@apasel422
Copy link
Contributor

This actually has a test already in src/test/run-pass/traits-issue-23003.rs and can be closed.

@arielb1 arielb1 closed this as completed Oct 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

No branches or pull requests

7 participants