Skip to content

Commit

Permalink
Add PhantomData marker for dropck to BTreeMap
Browse files Browse the repository at this point in the history
  • Loading branch information
steffahn committed Jul 18, 2022
1 parent 8804161 commit 110fdb6
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 7 deletions.
30 changes: 26 additions & 4 deletions library/alloc/src/collections/btree/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ pub struct BTreeMap<
length: usize,
/// `ManuallyDrop` to control drop order (needs to be dropped after all the nodes).
pub(super) alloc: ManuallyDrop<A>,
// For dropck; the `Box` avoids making the `Unpin` impl more strict than before
_marker: PhantomData<crate::boxed::Box<(K, V)>>,
}

#[stable(feature = "btree_drop", since = "1.7.0")]
Expand All @@ -187,6 +189,19 @@ unsafe impl<#[may_dangle] K, #[may_dangle] V, A: Allocator + Clone> Drop for BTr
}
}

// FIXME: This implementation is "wrong", but changing it would be a breaking change.
// (The bounds of the automatic `UnwindSafe` implementation have been like this since Rust 1.50.)
// Maybe we can fix it nonetheless with a crater run, or if the `UnwindSafe`
// traits are deprecated, or disarmed (no longer causing hard errors) in the future.
#[stable(feature = "btree_unwindsafe", since = "1.64.0")]
impl<K, V, A: Allocator + Clone> core::panic::UnwindSafe for BTreeMap<K, V, A>
where
A: core::panic::UnwindSafe,
K: core::panic::RefUnwindSafe,
V: core::panic::RefUnwindSafe,
{
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<K: Clone, V: Clone, A: Allocator + Clone> Clone for BTreeMap<K, V, A> {
fn clone(&self) -> BTreeMap<K, V, A> {
Expand All @@ -204,6 +219,7 @@ impl<K: Clone, V: Clone, A: Allocator + Clone> Clone for BTreeMap<K, V, A> {
root: Some(Root::new(alloc.clone())),
length: 0,
alloc: ManuallyDrop::new(alloc),
_marker: PhantomData,
};

{
Expand Down Expand Up @@ -567,7 +583,7 @@ impl<K, V> BTreeMap<K, V> {
#[rustc_const_unstable(feature = "const_btree_new", issue = "71835")]
#[must_use]
pub const fn new() -> BTreeMap<K, V> {
BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(Global) }
BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(Global), _marker: PhantomData }
}
}

Expand All @@ -593,6 +609,7 @@ impl<K, V, A: Allocator + Clone> BTreeMap<K, V, A> {
root: mem::replace(&mut self.root, None),
length: mem::replace(&mut self.length, 0),
alloc: self.alloc.clone(),
_marker: PhantomData,
});
}

Expand All @@ -615,7 +632,7 @@ impl<K, V, A: Allocator + Clone> BTreeMap<K, V, A> {
/// ```
#[unstable(feature = "btreemap_alloc", issue = "32838")]
pub fn new_in(alloc: A) -> BTreeMap<K, V, A> {
BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(alloc) }
BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(alloc), _marker: PhantomData }
}
}

Expand Down Expand Up @@ -1320,7 +1337,12 @@ impl<K, V, A: Allocator + Clone> BTreeMap<K, V, A> {
let (new_left_len, right_len) = Root::calc_split_length(total_num, &left_root, &right_root);
self.length = new_left_len;

BTreeMap { root: Some(right_root), length: right_len, alloc: self.alloc.clone() }
BTreeMap {
root: Some(right_root),
length: right_len,
alloc: self.alloc.clone(),
_marker: PhantomData,
}
}

/// Creates an iterator that visits all elements (key-value pairs) in
Expand Down Expand Up @@ -1445,7 +1467,7 @@ impl<K, V, A: Allocator + Clone> BTreeMap<K, V, A> {
let mut root = Root::new(alloc.clone());
let mut length = 0;
root.bulk_push(DedupSortedIter::new(iter.into_iter()), &mut length, alloc.clone());
BTreeMap { root: Some(root), length, alloc: ManuallyDrop::new(alloc) }
BTreeMap { root: Some(root), length, alloc: ManuallyDrop::new(alloc), _marker: PhantomData }
}
}

Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/btreemap/btreemap_dropck.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
struct PrintOnDrop<'a>(&'a str);

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

use std::collections::BTreeMap;
use std::iter::FromIterator;

fn main() {
let s = String::from("Hello World!");
let _map = BTreeMap::from_iter([((), PrintOnDrop(&s))]);
drop(s); //~ ERROR cannot move out of `s` because it is borrowed
}
13 changes: 13 additions & 0 deletions src/test/ui/btreemap/btreemap_dropck.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error[E0505]: cannot move out of `s` because it is borrowed
--> $DIR/btreemap_dropck.rs:15:10
|
LL | let _map = BTreeMap::from_iter([((), PrintOnDrop(&s))]);
| -- borrow of `s` occurs here
LL | drop(s);
| ^ move out of `s` occurs here
LL | }
| - borrow might be used here, when `_map` is dropped and runs the `Drop` code for type `BTreeMap`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0505`.
5 changes: 4 additions & 1 deletion src/test/ui/issues/issue-72554.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use std::collections::BTreeSet;

#[derive(Hash)]
pub enum ElemDerived { //~ ERROR recursive type `ElemDerived` has infinite size
pub enum ElemDerived {
//~^ ERROR recursive type `ElemDerived` has infinite size
//~| ERROR cycle detected when computing drop-check constraints for `ElemDerived`
A(ElemDerived)
}


pub enum Elem {
Derived(ElemDerived)
}
Expand Down
19 changes: 17 additions & 2 deletions src/test/ui/issues/issue-72554.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ error[E0072]: recursive type `ElemDerived` has infinite size
|
LL | pub enum ElemDerived {
| ^^^^^^^^^^^^^^^^^^^^ recursive type has infinite size
...
LL | A(ElemDerived)
| ----------- recursive without indirection
|
Expand All @@ -11,6 +12,20 @@ help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `ElemDerived
LL | A(Box<ElemDerived>)
| ++++ +

error: aborting due to previous error
error[E0391]: cycle detected when computing drop-check constraints for `ElemDerived`
--> $DIR/issue-72554.rs:4:1
|
LL | pub enum ElemDerived {
| ^^^^^^^^^^^^^^^^^^^^
|
= note: ...which immediately requires computing drop-check constraints for `ElemDerived` again
note: cycle used when computing drop-check constraints for `Elem`
--> $DIR/issue-72554.rs:11:1
|
LL | pub enum Elem {
| ^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0072`.
Some errors have detailed explanations: E0072, E0391.
For more information about an error, try `rustc --explain E0072`.

0 comments on commit 110fdb6

Please sign in to comment.