Skip to content

Drop impls ignore actual type parameters when running drop glue #16491

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
lilyball opened this issue Aug 14, 2014 · 4 comments
Closed

Drop impls ignore actual type parameters when running drop glue #16491

lilyball opened this issue Aug 14, 2014 · 4 comments
Labels
A-destructors Area: Destructors (`Drop`, …)

Comments

@lilyball
Copy link
Contributor

When implementing Drop on a type that has type parameters (which requires #[unsafe_destructor]), if the Drop impl is specialized to some concrete type parameter, the specialization is ignored in the drop glue and the Drop impl is instead run on all values of the outer type regardless of type parameters.

Example:

#![feature(unsafe_destructor)]

extern crate debug;

struct DropMe<T> {
    _x: T
}

struct ImplDrop;

#[unsafe_destructor]
impl Drop for DropMe<ImplDrop> {
    fn drop(&mut self) {
        println!("DropMe<ImplDrop>.Drop: {:?}", *self);
    }
}

fn main() {
    let x = DropMe::<ImplDrop> { _x: ImplDrop };
    println!("Dropping DropMe<ImplDrop>");
    drop(x);

    let y = DropMe::<int> { _x: 32 };
    println!("Dropping DropMe<int>");
    drop(y);
}

This should print

Dropping DropMe<int>
Dropping DropMe<ImplDrop>
DropMe<ImplDrop>.Drop: DropMe<ImplDrop>{_x: ImplDrop}

Instead it prints

Dropping DropMe<int>
DropMe<ImplDrop>.Drop: DropMe<ImplDrop>{_x: ImplDrop}
Dropping DropMe<ImplDrop>
DropMe<ImplDrop>.Drop: DropMe<ImplDrop>{_x: ImplDrop}

Note that it's calling the Drop impl on DropMe<int>, even though the Drop impl itself believes that the receiver type is DropMe<ImplDrop>.


It gets worse. If you add a second specialized Drop impl, that completely overrides the first:

#![feature(unsafe_destructor)]

extern crate debug;

struct DropMe<T> {
    _x: T
}

struct ImplDrop;

#[unsafe_destructor]
impl Drop for DropMe<ImplDrop> {
    fn drop(&mut self) {
        println!("DropMe<ImplDrop>.Drop: {:?}", *self);
    }
}

#[unsafe_destructor]
impl Drop for DropMe<int> {
    fn drop(&mut self) {
        println!("DropMe<int>.Drop: {:?}", *self);
    }
}

fn main() {
    let x = DropMe::<int> { _x: 32 };
    println!("Dropping DropMe<int>");
    drop(x);

    let y = DropMe::<ImplDrop> { _x: ImplDrop };
    println!("Dropping DropMe<ImplDrop>");
    drop(y);
}

This prints:

Dropping DropMe<int>
DropMe<int>.Drop: DropMe<int>{_x: 32}
Dropping DropMe<ImplDrop>
DropMe<int>.Drop: DropMe<int>{_x: 140734783283201}

It gets worse. If you use a type parameter with bounds, it incorrectly believes that it needs to call the Drop impl regardless of parameterization (as it did before), even though it knows that the type doesn't conform to the bounds. At least this time it's a compile-time error:

#![feature(unsafe_destructor)]

extern crate debug;

struct DropMe<T> {
    _x: T
}

struct ImplDrop;
trait Foo {}
impl Foo for ImplDrop {}

#[unsafe_destructor]
impl<T: Foo> Drop for DropMe<T> {
    fn drop(&mut self) {
        println!("DropMe<T: Foo>.Drop: {:?}", *self);
    }
}

fn main() {
    let x = DropMe::<int> { _x: 32 };
    println!("Dropping DropMe<int>");
    drop(x);

    let y = DropMe::<ImplDrop> { _x: ImplDrop };
    println!("Dropping DropMe<ImplDrop>");
    drop(y);
}
unnamed.rs:15:5: 17:6 error: failed to find an implementation of trait Foo for int
unnamed.rs:15     fn drop(&mut self) {
unnamed.rs:16         println!("DropMe<T: Foo>.Drop: {:?}", *self);
unnamed.rs:17     }

And of course if you add an impl Drop for DropMe<int> then it complains about duplicate implementations.

@huonw
Copy link
Member

huonw commented Aug 14, 2014

This is a dupe of #8142, which was closed by #14988 (putting #[unsafe_destructor] behind a feature gate).

@lilyball
Copy link
Contributor Author

Hrm, no wonder I couldn't find it, I only looked for open issues.

I was under the impression that there was some plan that would fix the lifetime issue and thus cause #[unsafe_destructor] to no longer be necessary?

In any case, if this doesn't work, then either a ticket should remain open to document it (such as this one), or the compiler should actually throw an error upon encountering the unsupported Drop impls.

@pnkfelix
Copy link
Member

@kballard I believe our planned fix is outlined on the updated description on #8142. (Note that it e.g. requires the ability to put bounds on type parameters in structs themselves, so that instead of having type-specialized Drop impls, you instead embed the desired constraints in the struct definition itself.)

Perhaps instead of closing that ticket after the addition of #14988, we instead should have just taken it off the P-backcompat-lang list and 1.0 milestone, but still left it open. I will go address that there.

@lilyball
Copy link
Contributor Author

@pnkfelix Works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-destructors Area: Destructors (`Drop`, …)
Projects
None yet
Development

No branches or pull requests

3 participants