consider removing/revising test/run-pass/zero-size-type-destructors.rs

[pnkfelix]

(spawned off of #rustc IRC discussion)

This test:

https://github.com/rust-lang/rust/blob/master/src/test/run-pass/zero-size-type-destructors.rs

leverages #[unsafe_no_drop_flag], coupled with a Drop impl that deliberately does not attempt to observe whether the value has already been dropped, to count the number of actual drops are executed by the generated code.

The problem with this is that small variations on the same program exhibit semantics that probably should not be considered stable.

For example (playpen: http://is.gd/BwiZ17):

#![feature(unsafe_no_drop_flag)]

static mut destructions : isize = 3;

pub fn foo() {
    #[unsafe_no_drop_flag]
    struct Foo(u32);

    impl Drop for Foo {
        fn drop(&mut self) {
            let p = self as *mut _;
            println!("dropping Foo(0x{:08x}) at {:?}", self.0, p);
            unsafe { destructions -= 1 };
        }
    };

    let x = Foo(1);
    println!("The location of  `&x` is at {:?}",  &x as *const _);
    let _x = [x, Foo(2), Foo(3)];
    println!("The location of `&_x` is at {:?}", &_x as *const _);
}

pub fn main() {
  foo();
  assert_eq!(unsafe { destructions }, 0);
}

prints:

The location of  `&x` is at 0x7fff1bfb0308
The location of `&_x` is at 0x7fff1bfb0258
dropping Foo(0x00000001) at 0x7fff1bfb0258
dropping Foo(0x00000002) at 0x7fff1bfb025c
dropping Foo(0x00000003) at 0x7fff1bfb0260
dropping Foo(0x1d1d1d1d) at 0x7fff1bfb0308
thread '<main>' panicked at 'assertion failed: `(left == right)` (left: `-1`, right: `0`)', <anon>:25

We continue to assert that there are three drops of Foo, since x is moved into _x[0]. But due to unsafe_no_drop_flag, there is no dynamic tracking of whether x is dropped or not, and therefore we end up executing the destructor four times. (The code above prints out the addresses of the local variables and of the &mut self in the destructor to make this very explicit; note that the 0x1d bit pattern is the way we mark a value as dropped/uninitialized.)

---

I am filing this issue to point out that the test in the respository may be overly-conservative, in that it is explicitly testing the behavior of unsafe_no_drop_flag with code for which we do not guarantee the current behavior.

In particular, this test is one of the things that is "blocking" MIR i.e. -Z orbit in its current state, but I would argue that we do not guarantee the current behavior, at least not to this degree of specificity.

[pnkfelix]

discussed a bit with lang team.

@nikomatsakis pointed out that a #[unsafe_no_drop_flag]'ed zero-sized type can have an idempotent destructor and be sane. (I had been thinking that no destructors would ever be sane and have any visible effects, but an idempotent one that e.g. set a global bit to true seems like an exception.)

[nikomatsakis]

That said, my feeling is that the #[unsafe_no_drop_flag] should go away, and that we don't generally support idempotent destructors. For example, it is not allowed to have a Copy type that also implementes Drop. So I am fine with making it illegal to have #[unsafe_no_drop_flag] on a ZST. It'd probably be prudent to check how much code breaks and consider a warning period, though of course these features are unstable. We can also update the test, but of course changing semantics here might break people's code, so making it a hard error is probably doing them a favor.

[Mark-Simulacrum]

The test no longer uses #[unsafe_no_drop_flag], so I'm going to close this--but please reopen if I'm wrong.