Skip to content

Fix overconstrained Send impls in btree internals #102680

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

Merged
merged 2 commits into from
Oct 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions library/alloc/src/collections/btree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,9 @@ impl<'a, K: 'a, V: 'a, Type> Clone for NodeRef<marker::Immut<'a>, K, V, Type> {

unsafe impl<BorrowType, K: Sync, V: Sync, Type> Sync for NodeRef<BorrowType, K, V, Type> {}

unsafe impl<'a, K: Sync + 'a, V: Sync + 'a, Type> Send for NodeRef<marker::Immut<'a>, K, V, Type> {}
unsafe impl<'a, K: Send + 'a, V: Send + 'a, Type> Send for NodeRef<marker::Mut<'a>, K, V, Type> {}
unsafe impl<'a, K: Send + 'a, V: Send + 'a, Type> Send for NodeRef<marker::ValMut<'a>, K, V, Type> {}
unsafe impl<K: Sync, V: Sync, Type> Send for NodeRef<marker::Immut<'_>, K, V, Type> {}
unsafe impl<K: Send, V: Send, Type> Send for NodeRef<marker::Mut<'_>, K, V, Type> {}
unsafe impl<K: Send, V: Send, Type> Send for NodeRef<marker::ValMut<'_>, K, V, Type> {}
unsafe impl<K: Send, V: Send, Type> Send for NodeRef<marker::Owned, K, V, Type> {}
unsafe impl<K: Send, V: Send, Type> Send for NodeRef<marker::Dying, K, V, Type> {}

Expand Down
293 changes: 293 additions & 0 deletions library/alloc/tests/autotraits.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
fn require_sync<T: Sync>(_: T) {}
fn require_send_sync<T: Send + Sync>(_: T) {}

struct NotSend(*const ());
unsafe impl Sync for NotSend {}

#[test]
fn test_btree_map() {
// Tests of this form are prone to https://github.com/rust-lang/rust/issues/64552.
//
// In theory the async block's future would be Send if the value we hold
// across the await point is Send, and Sync if the value we hold across the
// await point is Sync.
//
// We test autotraits in this convoluted way, instead of a straightforward
// `require_send_sync::<TypeIWantToTest>()`, because the interaction with
// generators exposes some current limitations in rustc's ability to prove a
// lifetime bound on the erased generator witness types. See the above link.
//
// A typical way this would surface in real code is:
//
// fn spawn<T: Future + Send>(_: T) {}
//
// async fn f() {
// let map = BTreeMap::<u32, Box<dyn Send + Sync>>::new();
// for _ in &map {
// async {}.await;
// }
// }
//
// fn main() {
// spawn(f());
// }
//
// where with some unintentionally overconstrained Send impls in liballoc's
// internals, the future might incorrectly not be Send even though every
// single type involved in the program is Send and Sync.
require_send_sync(async {
let _v = None::<alloc::collections::btree_map::Iter<'_, &u32, &u32>>;
async {}.await;
});

// Testing like this would not catch all issues that the above form catches.
require_send_sync(None::<alloc::collections::btree_map::Iter<'_, &u32, &u32>>);

require_sync(async {
let _v = None::<alloc::collections::btree_map::Iter<'_, u32, NotSend>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::BTreeMap<&u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<
alloc::collections::btree_map::DrainFilter<
'_,
&u32,
&u32,
fn(&&u32, &mut &u32) -> bool,
>,
>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::Entry<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::IntoIter<&u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::IntoKeys<&u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::IntoValues<&u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::Iter<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::IterMut<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::Keys<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::OccupiedEntry<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::OccupiedError<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::Range<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::RangeMut<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::VacantEntry<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::Values<'_, &u32, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_map::ValuesMut<'_, &u32, &u32>>;
async {}.await;
});
}

#[test]
fn test_btree_set() {
require_send_sync(async {
let _v = None::<alloc::collections::btree_set::BTreeSet<&u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_set::Difference<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_set::DrainFilter<'_, &u32, fn(&&u32) -> bool>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_set::Intersection<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_set::IntoIter<&u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_set::Iter<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_set::Range<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_set::SymmetricDifference<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::btree_set::Union<'_, &u32>>;
async {}.await;
});
}

#[test]
fn test_binary_heap() {
require_send_sync(async {
let _v = None::<alloc::collections::binary_heap::BinaryHeap<&u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::binary_heap::Drain<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::binary_heap::DrainSorted<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::binary_heap::IntoIter<&u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::binary_heap::IntoIterSorted<&u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::binary_heap::Iter<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::binary_heap::PeekMut<'_, &u32>>;
async {}.await;
});
}

#[test]
fn test_linked_list() {
require_send_sync(async {
let _v = None::<alloc::collections::linked_list::Cursor<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::linked_list::CursorMut<'_, &u32>>;
async {}.await;
});

// FIXME
/*
require_send_sync(async {
let _v =
None::<alloc::collections::linked_list::DrainFilter<'_, &u32, fn(&mut &u32) -> bool>>;
async {}.await;
});
*/

require_send_sync(async {
let _v = None::<alloc::collections::linked_list::IntoIter<&u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::linked_list::Iter<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::linked_list::IterMut<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::linked_list::LinkedList<&u32>>;
async {}.await;
});
}

#[test]
fn test_vec_deque() {
require_send_sync(async {
let _v = None::<alloc::collections::vec_deque::Drain<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::vec_deque::IntoIter<&u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::vec_deque::Iter<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::vec_deque::IterMut<'_, &u32>>;
async {}.await;
});

require_send_sync(async {
let _v = None::<alloc::collections::vec_deque::VecDeque<&u32>>;
async {}.await;
});
}
4 changes: 4 additions & 0 deletions library/alloc/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![feature(alloc_layout_extra)]
#![feature(assert_matches)]
#![feature(box_syntax)]
#![feature(btree_drain_filter)]
#![feature(cow_is_borrowed)]
#![feature(const_box)]
#![feature(const_convert)]
Expand All @@ -14,6 +15,8 @@
#![feature(core_intrinsics)]
#![feature(drain_filter)]
#![feature(exact_size_is_empty)]
#![feature(linked_list_cursors)]
#![feature(map_try_insert)]
#![feature(new_uninit)]
#![feature(pattern)]
#![feature(trusted_len)]
Expand Down Expand Up @@ -49,6 +52,7 @@ use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};

mod arc;
mod autotraits;
mod borrow;
mod boxed;
mod btree_set_hash;
Expand Down