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

ICE if generic destructor invokes a method on self ("vtables missing where they are needed") #4252

Closed
tychosci opened this issue Dec 22, 2012 · 10 comments · Fixed by #12403
Closed
Labels
A-destructors Area: Destructors (`Drop`, …) A-traits Area: Trait system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority
Milestone

Comments

@tychosci
Copy link
Contributor

xyz.rs:

trait X {
    fn call(&self);
}

struct Y;
struct Z<T: X> {
    x: T
}

impl Y: X {
    fn call(&self) {
    }
}

impl<T: X> Z<T>: Drop {
    fn finalize(&self) {
        self.x.call(); // Adding this statement causes an ICE.
    }
}

fn main() {
    let y = Y;
    let _z = Z{x: y};
}

$ RUST_LOG=rustc=1,::rt::backtrace rustc xyz.rs
rust: task failed at 'option::get none', /Users/tychosci/pkg/rust/src/librustc/rustc.rc:1
error: internal compiler error: unexpected failure
note: the compiler hit an unexpected failure path. this is a bug
note: try running with RUST_LOG=rustc=1,::rt::backtrace to get further details and report the results to github.com/mozilla/rust/issues
rust: task failed at 'explicit failure', /Users/tychosci/pkg/rust/src/librustc/rustc.rc:425
rust: domain main @0x7feecb000010 root task failed
rust: task failed at 'killed', /Users/tychosci/pkg/rust/src/libcore/task/mod.rs:570
@ghost ghost assigned catamorphism Feb 20, 2013
@catamorphism
Copy link
Contributor

Bounds on struct params are no longer supported, so I had to edit a bit, but I still got it to ICE. Will check in the new test case, xfailed. Not critical for 0.6, though, de-milestoning.

@catamorphism
Copy link
Contributor

Still ICEs if I add #[unsafe_destructor]. Nominating for milestone 5, production-ready.

@msullivan
Copy link
Contributor

Still ICEs on this updated test:

trait X {
    fn call(&self);
}

struct Y;
struct Z<T> {
    x: T
}

impl X for Y {
    fn call(&self) {
    }
}

#[unsafe_destructor]
impl<T: X> Drop for Z<T> {
    fn drop(&self) {
        self.x.call(); // Adding this statement causes an ICE.
    }
}

fn main() {
    let y = Y;
    let _z = Z{x: y};
}

It fails because it doesn't have the vtables around... Interesting.

@msullivan
Copy link
Contributor

Ah, so, destructors seem to be generated from a strange codepath such that they never get vtables. See get_res_dtor which calls monomorphic_fn to generate the destructor, and calls it None as the vtables.

@msullivan
Copy link
Contributor

I updated the title on this.

@graydon
Copy link
Contributor

graydon commented Aug 1, 2013

accepted for production-ready milestone

@panzi
Copy link

panzi commented Aug 5, 2013

Will this be fixed some when or will it be impossible to write a generic destructor?

sfackler added a commit to sfackler/rust that referenced this issue Sep 11, 2013
The default buffer size is the same as the one in Java's BufferedWriter.

We may want BufferedWriter to have a Drop impl that flushes, but that
isn't possible right now due to rust-lang#4252/rust-lang#4430. This would be a bit
awkward due to the possibility of the inner flush failing. For what it's
worth, Java's BufferedReader doesn't have a flushing finalizer, but that
may just be because Java's finalizer support is awful.

Closes rust-lang#8953
bors added a commit that referenced this issue Sep 11, 2013
The default buffer size is the same as the one in Java's BufferedWriter.

We may want BufferedWriter to have a Drop impl that flushes, but that
isn't possible right now due to #4252/#4430. This would be a bit
awkward due to the possibility of the inner flush failing. For what it's
worth, Java's BufferedReader doesn't have a flushing finalizer, but that
may just be because Java's finalizer support is awful.

The current implementation of BufferedStream is weird in my opinion, but
it's what the discussion in #8953 settled on.

I wrote a custom copy function since vec::copy_from doesn't optimize as
well as I would like.

Closes #8953
@thestinger
Copy link
Contributor

This bug is quite hard to fix. The typeck part of compilation outputs the necessary vtable information based on the context of method calls. Since destructor calls are not part of the AST, there is no metadata generated for them so there's nothing for the get_res_dtor function to ask for.

pczarn referenced this issue in wbthomason/ironkernel Dec 9, 2013
@pnkfelix
Copy link
Member

Its pretty hard to trace back from the ICE's that one gets to this cause. E.g. messages like this (see #8286):

task 'rustc' failed at 'vtables missing where they are needed', 
   /Users/rustbuild/src/rust-buildbot/slave/snap3-mac/build/src/libstd/option.rs:111

do not tell the user that they should revise the code of their destructor. (Admittedly, generic destructors are already unsafe ... but to me, unsafe does not imply "may cause the compiler to ICE" -- just the generated code!)

We should either figure out a way to allow generic destructors to invoke methods on self, or we should emit an error or warning earlier on in the compilation so that someone has a prayer of dissecting this problem without hunting through our issue database.

Nominating for 1.0, P-backcompat-lang.

@pnkfelix
Copy link
Member

Definitely something we should resolve one way or another for 1.0, even if its just about getting the error message better (but ideally, since RAII is part of our story, we'd do better than that.)

P-high

@bors bors closed this as completed in 06e1281 Feb 20, 2014
alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 21, 2014
This contains the fix for rust-lang#4252 so we can start using methods in destructors.
bors added a commit that referenced this issue Feb 22, 2014
This contains the fix for #4252 so we can start using methods in destructors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-destructors Area: Destructors (`Drop`, …) A-traits Area: Trait system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants