Too lenient #[may_dangle] on BTreeMap; misses PhantomData for dropck

[steffahn]

struct PrintOnDrop<'a>(&'a str);

impl Drop for PrintOnDrop<'_> {
    fn drop(&mut self) {
        println!("printint: {}", self.0);
    }
}

use std::collections::BTreeMap;

fn main() {
    let s = String::from("Hello World!");
    let map = BTreeMap::from_iter([((), PrintOnDrop(&s))]);
    drop(s);
}

(playground)

printing: ��J��U��J�

@rustbot label T-libs, I-unsound
@rustbot claim

[steffahn]

This is actually also a (not too recent) regression (in nightly-2019-10-18).

********************************************************************************
Regression in nightly-2019-10-18
********************************************************************************

fetching https://static.rust-lang.org/dist/2019-10-17/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2019-10-17: 40 B / 40 B [===================================================================] 100.00 % 737.29 KB/s converted 2019-10-17 to 0e8a4b441c5da21a2cb19448728ade5baa299c66
fetching https://static.rust-lang.org/dist/2019-10-18/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2019-10-18: 40 B / 40 B [===================================================================] 100.00 % 772.54 KB/s converted 2019-10-18 to fa0f7d0080d8e7e9eb20aa9cbf8013f96c81287f
looking for regression commit between 2019-10-17 and 2019-10-18
fetching (via remote github) commits from max(0e8a4b441c5da21a2cb19448728ade5baa299c66, 2019-10-15) to fa0f7d0080d8e7e9eb20aa9cbf8013f96c81287f
ending github query because we found starting sha: 0e8a4b441c5da21a2cb19448728ade5baa299c66
get_commits_between returning commits, len: 7
  commit[0] 2019-10-16UTC: Auto merge of #65445 - ehuss:update-cargo-books, r=alexcrichton
  commit[1] 2019-10-16UTC: Auto merge of #63756 - Zoxc:sharded-dep-graph-1, r=nikomatsakis
  commit[2] 2019-10-17UTC: Auto merge of #65234 - GuillaumeGomez:long-err-explanation-E0573, r=kinnison
  commit[3] 2019-10-17UTC: Auto merge of #65251 - tlively:emscripten-upstream-upgrade, r=tlively
  commit[4] 2019-10-17UTC: Auto merge of #59953 - eddyb:soa-metadata, r=michaelwoerister
  commit[5] 2019-10-17UTC: Auto merge of #64595 - Mark-Simulacrum:trivial-query, r=pnkfelix
  commit[6] 2019-10-17UTC: Auto merge of #65495 - Centril:rollup-tguwjt5, r=Centril

So maybe #64595 made a difference?

---

@rustbot label regression-from-stable-to-stable

[steffahn]

This is what changed apparently, the following compiles successfully since nightly 2019-10-18:

#![feature(dropck_eyepatch)]

use std::mem::ManuallyDrop;

struct S<T>(ManuallyDrop<T>);

unsafe impl<#[may_dangle] T> Drop for S<T> {
    fn drop(&mut self) {}
}

struct NonTrivialDrop<'a>(&'a str);
impl<'a> Drop for NonTrivialDrop<'a> {
    fn drop(&mut self) {}
}

fn main() {
    let s = String::from("string");
    let _t = S(ManuallyDrop::new(NonTrivialDrop(&s)));
    drop(s);
}

I mean, yeah, this change was good IMO, so it is just the library that needs fixing, dropck itself seems fine.

[m-ou-se]

It's probably time we move https://forge.rust-lang.org/libs/maintaining-std.html#is-there-a-manual-drop-implementation to the std-dev-guide.

[RalfJung]

So maybe #64595 made a difference?

That PR sounds like it was meant to be an optimization, not changing behavior. There are no new tests. And yet it turns out that it did change behavior!
Cc @Mark-Simulacrum @pnkfelix

Specifically, I think this is when dropck started exploiting that ManuallyDrop does not drop its fields. I will add a corresponding testcase to #103413.

[Mark-Simulacrum]

Yes, I seem to recall us fixing fallout from that PR a while back - it did indeed (unintentionally) change behavior, though I don't have in cache what went wrong with the changes.

[RalfJung]

By "fixing" you mean bring back the original behavior? That does not seem to be entirely the case; as this issue shows, one change in behavior did remain in place.