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

RFC: Can we make drop glue for recursive types not stack overflow? #8399

Closed
bblum opened this issue Aug 8, 2013 · 13 comments

Comments

@bblum
Copy link
Contributor

commented Aug 8, 2013

Example 2014-02-23

struct Foo { data: int, next: Option<~Foo> }

fn main() {
    let mut foo = Some(~Foo { data: 0, next: None });
    for i in range(0, 17_000_000) {
        if i % 1000000 == 0 {
            error!("{}", i);
        }
        foo = Some(~Foo { data: i, next: foo.take() });
    }
}

Original example

use std::cell::Cell;

struct Foo { data: int, next: Option<~Foo> }

fn main() {
    let foo = Cell::new(Foo { data: 0, next: None });
    let mut i = 0;
    do 17_000_000.times {
        i += 1;
        if i % 1000000 == 0 {
            error!("%d", i);
        }
        foo.put_back(Foo { data: i, next: Some(~foo.take()) });
    }
}
@bblum

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2013

This can be avoided by hand with the following destructor:

impl Drop for Foo {
    fn drop(&self) {
        let this: &mut Foo = unsafe { use std::cast; cast::transmute(self) };
        while this.next.is_some() {
            do this.next.take().map_consume |mut next| {
                this.next = next.next.take();
            };
        }
    }
}
@bblum

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2013

Unfortunately we can't even rely on tail-call optimization in some cases, for example, a tree where each stack frame has to recurse once on each branch. I wonder if the above hand-coded destructor can be generalized.

Nominating well-covered.

@bluss

This comment has been minimized.

Copy link
Member

commented Aug 8, 2013

Previous issue (solved) for DList in particular #8295

@bblum

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2013

Oh yeah thanks for the link. I suppose I should tag this an RFC to see if we can solve it in the general case.

@thestinger

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2013

At least with optimizations enabled this would already be a non-issue in many cases if we weren't making so many possibly invalid casts.

@bblum

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2013

With optimizations disabled we should have well-defined behaviour too :P

@thestinger

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2013

The behaviour would still be well-defined, it would just use a lot more memory than necessary. That's really no different than how we use a ridiculous amount of stack space everywhere else without optimizations.

@bblum

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2013

It occurred to me on the way in this morning that this would be very hard if not impossible to do for tree-like types, struct Foo { left: Option<~Foo>, right: Option<~Foo> }.

@thestinger

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2013

@bblum: yeah, there are cases where you fundamentally require a stack of some sort - luckily, for self-balancing binary search trees the depth is log2(length) so it's a non-issue

@catamorphism

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2013

Just a bug (this may never get fixed)

@huonw

This comment has been minimized.

Copy link
Member

commented Feb 23, 2014

Triage: no change. (I updated the example.)

@thestinger thestinger added I-wishlist and removed I-crash labels May 18, 2014

@steveklabnik

This comment has been minimized.

Copy link
Member

commented Jan 20, 2015

Triage:

#![feature(box_syntax)]

struct Foo { data: int, next: Option<Box<Foo>> }

fn main() {
    let mut foo = Some(box Foo { data: 0, next: None });
    for i in range(0, 17_000_000) {
        if#![feature(box_syntax)]

struct Foo { data: int, next: Option<Box<Foo>> }

fn main() {
    let mut foo = Some(box Foo { data: 0, next: None });
    for i in range(0, 17_000_000) {
        if i % 1000000 == 0 {
            println!("{}", i);
        }
        foo = Some(box Foo { data: i, next: foo.take() });
    }
}
 i % 1000000 == 0 {
            println!("{}", i);
        }
        foo = Some(box Foo { data: i, next: foo.take() });
    }
}

Running:

$ ./hello 
0
1000000
2000000
3000000
4000000
5000000
6000000
7000000
8000000
9000000
10000000
11000000
12000000
13000000
14000000
15000000
16000000

thread '<main>' has overflowed its stack
Illegal instruction

@steveklabnik

This comment has been minimized.

Copy link
Member

commented Jan 21, 2015

I'm pulling a massive triage effort to get us ready for 1.0. As part of this, I'm moving stuff that's wishlist-like to the RFCs repo, as that's where major new things should get discussed/prioritized.

This issue has been moved to the RFCs repo: rust-lang/rfcs#686

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.