Ban #[unsafe_no_drop_flag] on zero-sized types.

[eddyb]

The only existing test we have relies on the simple in-place initialization behavior in old trans, e.g.:

    // Runs Foo's Drop impl 3 times.
    let _x = [x, Foo, Foo];

    // Runs Foo's Drop impl 4 times.
    let x = Foo;
    let _x = [x, Foo, Foo];

With MIR, there are more temporaries, and the test can't work without extra move tracking.

This is a [breaking-change], but #[unsafe_no_drop_flag] is unstable, and only works for ZSTs in very simple cases, so I doubt there are real uses in the wild. Fixes #33237.

[rust-highfive]

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

[pnkfelix]

I would think that to truly resolve #33237, we should also have a run-pass test that explicitly shows how one would write a similar test (on a non-zero-sized type) that properly consults the drop-flag state to decide whether to run the destructor or not.

(That, or point to an existing unit test that is similarly demonstrating how to write such code.)

---

Update (hopefully a clarification): the point I am trying to make here is that the alternative code I wrote in #33237 is an example of behavior that we probably do not want to consider specified/stable, and it is on a type that is explicitly not zero sized.

I now understand why @eddyb is pointing out that part of the story here is to disallow #[unsafe_no_drop_flag] on ZST's (namely, because if you have a ZST and no drop flag attached, there there is quite simply no way you can hope to properly implement Drop for such a type, and thus @eddyb is flagging such types ahead of time).

[pnkfelix]

(I want to discuss this with either T-compiler or T-lang before merging.)

[pnkfelix]

@eddyb (if we do decide to land a check like this, it should be downgraded to a warning first, following something like RFC rust-lang/rfcs#1589 )

[pnkfelix]

@eddyb (but first lets see what a crater run says)

[eddyb]

@pnkfelix Does that apply to changes in unstable features (which this is)?

[pnkfelix]

@eddyb yes I think this rule applies even for unstable features.

I can understand how that might sound odd, since someone who is using unstable features is already putting up with using the nightly channel (or a snapshot of nightly, or a locally built rustc).

But my thinking is that we want to make the runway for breaking changes a smooth incline, even when the breakage is solely applicable to unstable things.

---

@nikomatsakis may want to chime in further, or contradict me entirely for that matter. :) (rust-lang/rfcs#1589 doesn't draw a distinction AFAICT in terms of what kinds of features it is meant to apply to)

[DemiMarie]

@eddyb I think part of the reason for this is that some crates require nightly Rust, as does Servo.

[eddyb]

@drbo Sure, and they deal with fallout on virtually every single manual update of the Rust version they support (Servo in particular has a set of custom lints which use the unstable rustc API).

[bors]

☔ The latest upstream changes (presumably #33425) made this pull request unmergeable. Please resolve the merge conflicts.

[eddyb]

Results of crater run: https://gist.github.com/83e72ed67d7424a37a1fa8e6937b245a.
Effectively 0 regressions (and a timeout).
However, I don't care enough, given that proper non-zeroing/filling dynamic drop could land in the following weeks.

[pnkfelix]

@eddyb okay, well, r=me (just fix the tidy failures).

[pnkfelix]

(I said "r=me" but maybe this needs to actually be addressed first?)

[eddyb]

That's a long term goal that is blocked on removing old trans.

[eddyb]

With non-zeroing drop here, there's no need for this.