Skip to content

Commit

Permalink
remove TrustedRandomAccess vs. TrustedRandomAccessNoCoerce distinction
Browse files Browse the repository at this point in the history
TRA is no longer used for single-step (`next()`) specializations, only loops and after each
loop we bring the iterator back into a safe state, meaning coercions are now safe because
the user code will not have access to an iterator instance that would be unsafe to coerce.
  • Loading branch information
the8472 committed Apr 19, 2022
1 parent a66cd5b commit aec6846
Show file tree
Hide file tree
Showing 17 changed files with 61 additions and 202 deletions.
8 changes: 2 additions & 6 deletions library/alloc/src/collections/vec_deque/iter.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use core::fmt;
use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce};
use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess};
use core::mem::MaybeUninit;
use core::ops::Try;

Expand Down Expand Up @@ -205,11 +205,7 @@ unsafe impl<T> TrustedLen for Iter<'_, T> {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<T> TrustedRandomAccess for Iter<'_, T> {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<T> TrustedRandomAccessNoCoerce for Iter<'_, T> {
unsafe impl<T> TrustedRandomAccess for Iter<'_, T> {
const NEEDS_CLEANUP: bool = false;

fn cleanup(&mut self, num: usize, forward: bool) {
Expand Down
8 changes: 2 additions & 6 deletions library/alloc/src/collections/vec_deque/iter_mut.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use core::fmt;
use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce};
use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess};
use core::marker::PhantomData;

use super::{count, wrap_index, RingSlices};
Expand Down Expand Up @@ -154,11 +154,7 @@ unsafe impl<T> TrustedLen for IterMut<'_, T> {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<T> TrustedRandomAccess for IterMut<'_, T> {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<T> TrustedRandomAccessNoCoerce for IterMut<'_, T> {
unsafe impl<T> TrustedRandomAccess for IterMut<'_, T> {
const NEEDS_CLEANUP: bool = false;

fn cleanup(&mut self, num: usize, forward: bool) {
Expand Down
12 changes: 6 additions & 6 deletions library/alloc/src/vec/in_place_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
//! # O(1) collect
//!
//! The main iteration itself is further specialized when the iterator implements
//! [`TrustedRandomAccessNoCoerce`] to let the optimizer see that it is a counted loop with a single
//! [`TrustedRandomAccess`] to let the optimizer see that it is a counted loop with a single
//! [induction variable]. This can turn some iterators into a noop, i.e. it reduces them from O(n) to
//! O(1). This particular optimization is quite fickle and doesn't always work, see [#79308]
//!
Expand All @@ -70,7 +70,7 @@
//! Since unchecked accesses through that trait do not advance the read pointer of `IntoIter`
//! this would interact unsoundly with the requirements about dropping the tail described above.
//! But since the normal `Drop` implementation of `IntoIter` would suffer from the same problem it
//! is only correct for `TrustedRandomAccessNoCoerce` to be implemented when the items don't
//! is only correct for `TrustedRandomAccess` to be implemented when the items don't
//! have a destructor. Thus that implicit requirement also makes the specialization safe to use for
//! in-place collection.
//! Note that this safety concern is about the correctness of `impl Drop for IntoIter`,
Expand Down Expand Up @@ -134,7 +134,7 @@
//! }
//! vec.truncate(write_idx);
//! ```
use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccessNoCoerce};
use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccess};
use core::mem::{self, ManuallyDrop};
use core::ptr::{self};

Expand Down Expand Up @@ -195,7 +195,7 @@ where
// itself once IntoIter goes out of scope.
// If the drop panics then we also leak any elements collected into dst_buf.
//
// Note: This access to the source wouldn't be allowed by the TrustedRandomIteratorNoCoerce
// Note: This access to the source wouldn't be allowed by the TrustedRandomIterator
// contract (used by SpecInPlaceCollect below). But see the "O(1) collect" section in the
// module documenttation why this is ok anyway.
src.forget_allocation_drop_remaining();
Expand Down Expand Up @@ -230,7 +230,7 @@ trait SpecInPlaceCollect<T, I>: Iterator<Item = T> {
/// collected. `end` is the last writable element of the allocation and used for bounds checks.
///
/// This method is specialized and one of its implementations makes use of
/// `Iterator::__iterator_get_unchecked` calls with a `TrustedRandomAccessNoCoerce` bound
/// `Iterator::__iterator_get_unchecked` calls with a `TrustedRandomAccess` bound
/// on `I` which means the caller of this method must take the safety conditions
/// of that trait into consideration.
fn collect_in_place(&mut self, dst: *mut T, end: *const T) -> usize;
Expand All @@ -256,7 +256,7 @@ where

impl<T, I> SpecInPlaceCollect<T, I> for I
where
I: Iterator<Item = T> + TrustedRandomAccessNoCoerce,
I: Iterator<Item = T> + TrustedRandomAccess,
{
#[inline]
fn collect_in_place(&mut self, dst_buf: *mut T, end: *const T) -> usize {
Expand Down
10 changes: 3 additions & 7 deletions library/alloc/src/vec/into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use crate::alloc::{Allocator, Global};
use crate::raw_vec::RawVec;
use core::fmt;
use core::intrinsics::arith_offset;
use core::iter::{
FusedIterator, InPlaceIterable, SourceIter, TrustedLen, TrustedRandomAccessNoCoerce,
};
use core::iter::{FusedIterator, InPlaceIterable, SourceIter, TrustedLen, TrustedRandomAccess};
use core::marker::PhantomData;
use core::mem::{self, ManuallyDrop};
use core::ops::Deref;
Expand Down Expand Up @@ -200,7 +198,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
#[doc(hidden)]
unsafe fn __iterator_get_unchecked(&mut self, i: usize) -> Self::Item
where
Self: TrustedRandomAccessNoCoerce,
Self: TrustedRandomAccess,
{
// SAFETY: the caller must guarantee that `i` is in bounds of the
// `Vec<T>`, so `i` cannot overflow an `isize`, and the `self.ptr.add(i)`
Expand Down Expand Up @@ -284,9 +282,7 @@ impl<T: Copy> NonDrop for T {}

#[doc(hidden)]
#[unstable(issue = "none", feature = "std_internals")]
// TrustedRandomAccess (without NoCoerce) must not be implemented because
// subtypes/supertypes of `T` might not be `NonDrop`
unsafe impl<T, A: Allocator> TrustedRandomAccessNoCoerce for IntoIter<T, A>
unsafe impl<T, A: Allocator> TrustedRandomAccess for IntoIter<T, A>
where
T: NonDrop,
{
Expand Down
14 changes: 4 additions & 10 deletions library/core/src/iter/adapters/cloned.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::iter::adapters::{
zip::try_get_unchecked, TrustedRandomAccess, TrustedRandomAccessNoCoerce,
};
use crate::iter::adapters::{zip::try_get_unchecked, TrustedRandomAccess};
use crate::iter::{FusedIterator, TrustedLen};
use crate::ops::Try;

Expand Down Expand Up @@ -63,7 +61,7 @@ where
#[doc(hidden)]
unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> T
where
Self: TrustedRandomAccessNoCoerce,
Self: TrustedRandomAccess,
{
// SAFETY: the caller must uphold the contract for
// `Iterator::__iterator_get_unchecked`.
Expand Down Expand Up @@ -123,13 +121,9 @@ where

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I> TrustedRandomAccess for Cloned<I> where I: TrustedRandomAccess {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I> TrustedRandomAccessNoCoerce for Cloned<I>
unsafe impl<I> TrustedRandomAccess for Cloned<I>
where
I: TrustedRandomAccessNoCoerce,
I: TrustedRandomAccess,
{
const NEEDS_CLEANUP: bool = I::NEEDS_CLEANUP;

Expand Down
14 changes: 4 additions & 10 deletions library/core/src/iter/adapters/copied.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::iter::adapters::{
zip::try_get_unchecked, TrustedRandomAccess, TrustedRandomAccessNoCoerce,
};
use crate::iter::adapters::{zip::try_get_unchecked, TrustedRandomAccess};
use crate::iter::{FusedIterator, TrustedLen};
use crate::ops::Try;

Expand Down Expand Up @@ -84,7 +82,7 @@ where
#[doc(hidden)]
unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> T
where
Self: TrustedRandomAccessNoCoerce,
Self: TrustedRandomAccess,
{
// SAFETY: the caller must uphold the contract for
// `Iterator::__iterator_get_unchecked`.
Expand Down Expand Up @@ -149,13 +147,9 @@ where

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I> TrustedRandomAccess for Copied<I> where I: TrustedRandomAccess {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I> TrustedRandomAccessNoCoerce for Copied<I>
unsafe impl<I> TrustedRandomAccess for Copied<I>
where
I: TrustedRandomAccessNoCoerce,
I: TrustedRandomAccess,
{
const NEEDS_CLEANUP: bool = I::NEEDS_CLEANUP;

Expand Down
14 changes: 4 additions & 10 deletions library/core/src/iter/adapters/enumerate.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::iter::adapters::{
zip::try_get_unchecked, SourceIter, TrustedRandomAccess, TrustedRandomAccessNoCoerce,
};
use crate::iter::adapters::{zip::try_get_unchecked, SourceIter, TrustedRandomAccess};
use crate::iter::{FusedIterator, InPlaceIterable, TrustedLen};
use crate::ops::Try;

Expand Down Expand Up @@ -132,7 +130,7 @@ where
#[inline]
unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> <Self as Iterator>::Item
where
Self: TrustedRandomAccessNoCoerce,
Self: TrustedRandomAccess,
{
// SAFETY: the caller must uphold the contract for
// `Iterator::__iterator_get_unchecked`.
Expand Down Expand Up @@ -232,13 +230,9 @@ where

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I> TrustedRandomAccess for Enumerate<I> where I: TrustedRandomAccess {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I> TrustedRandomAccessNoCoerce for Enumerate<I>
unsafe impl<I> TrustedRandomAccess for Enumerate<I>
where
I: TrustedRandomAccessNoCoerce,
I: TrustedRandomAccess,
{
const NEEDS_CLEANUP: bool = I::NEEDS_CLEANUP;

Expand Down
11 changes: 3 additions & 8 deletions library/core/src/iter/adapters/fuse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::intrinsics;
use crate::iter::adapters::zip::try_get_unchecked;
use crate::iter::{
DoubleEndedIterator, ExactSizeIterator, FusedIterator, TrustedLen, TrustedRandomAccess,
TrustedRandomAccessNoCoerce,
};
use crate::ops::Try;

Expand Down Expand Up @@ -132,7 +131,7 @@ where
#[doc(hidden)]
unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item
where
Self: TrustedRandomAccessNoCoerce,
Self: TrustedRandomAccess,
{
match self.iter {
// SAFETY: the caller must uphold the contract for
Expand Down Expand Up @@ -222,13 +221,9 @@ unsafe impl<I> TrustedLen for Fuse<I> where I: TrustedLen {}
//
// This is safe to implement as `Fuse` is just forwarding these to the wrapped iterator `I`, which
// preserves these properties.
unsafe impl<I> TrustedRandomAccess for Fuse<I> where I: TrustedRandomAccess {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I> TrustedRandomAccessNoCoerce for Fuse<I>
unsafe impl<I> TrustedRandomAccess for Fuse<I>
where
I: TrustedRandomAccessNoCoerce,
I: TrustedRandomAccess,
{
const NEEDS_CLEANUP: bool = I::NEEDS_CLEANUP;

Expand Down
14 changes: 4 additions & 10 deletions library/core/src/iter/adapters/map.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use crate::fmt;
use crate::iter::adapters::{
zip::try_get_unchecked, SourceIter, TrustedRandomAccess, TrustedRandomAccessNoCoerce,
};
use crate::iter::adapters::{zip::try_get_unchecked, SourceIter, TrustedRandomAccess};
use crate::iter::{FusedIterator, InPlaceIterable, TrustedLen};
use crate::ops::Try;

Expand Down Expand Up @@ -128,7 +126,7 @@ where
#[inline]
unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> B
where
Self: TrustedRandomAccessNoCoerce,
Self: TrustedRandomAccess,
{
// SAFETY: the caller must uphold the contract for
// `Iterator::__iterator_get_unchecked`.
Expand Down Expand Up @@ -190,13 +188,9 @@ where

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I, F> TrustedRandomAccess for Map<I, F> where I: TrustedRandomAccess {}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<I, F> TrustedRandomAccessNoCoerce for Map<I, F>
unsafe impl<I, F> TrustedRandomAccess for Map<I, F>
where
I: TrustedRandomAccessNoCoerce,
I: TrustedRandomAccess,
{
const NEEDS_CLEANUP: bool = I::NEEDS_CLEANUP;

Expand Down
3 changes: 0 additions & 3 deletions library/core/src/iter/adapters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ pub use self::map_while::MapWhile;
#[unstable(feature = "trusted_random_access", issue = "none")]
pub use self::zip::TrustedRandomAccess;

#[unstable(feature = "trusted_random_access", issue = "none")]
pub use self::zip::TrustedRandomAccessNoCoerce;

#[stable(feature = "iter_zip", since = "1.59.0")]
pub use self::zip::zip;

Expand Down
44 changes: 6 additions & 38 deletions library/core/src/iter/adapters/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ where
#[doc(hidden)]
unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item
where
Self: TrustedRandomAccessNoCoerce,
Self: TrustedRandomAccess,
{
// SAFETY: the caller must uphold the contract for
// `Iterator::__iterator_get_unchecked`.
Expand Down Expand Up @@ -257,15 +257,6 @@ unsafe impl<A, B> TrustedRandomAccess for Zip<A, B>
where
A: TrustedRandomAccess,
B: TrustedRandomAccess,
{
}

#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
unsafe impl<A, B> TrustedRandomAccessNoCoerce for Zip<A, B>
where
A: TrustedRandomAccessNoCoerce,
B: TrustedRandomAccessNoCoerce,
{
const NEEDS_CLEANUP: bool = A::NEEDS_CLEANUP || B::NEEDS_CLEANUP;

Expand Down Expand Up @@ -328,9 +319,7 @@ impl<A: Debug, B: Debug> ZipFmt<A, B> for Zip<A, B> {
}
}

impl<A: Debug + TrustedRandomAccessNoCoerce, B: Debug + TrustedRandomAccessNoCoerce> ZipFmt<A, B>
for Zip<A, B>
{
impl<A: Debug + TrustedRandomAccess, B: Debug + TrustedRandomAccess> ZipFmt<A, B> for Zip<A, B> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// It's *not safe* to call fmt on the contained iterators, since once
// we start iterating they're in strange, potentially unsafe, states.
Expand All @@ -344,7 +333,7 @@ impl<A: Debug + TrustedRandomAccessNoCoerce, B: Debug + TrustedRandomAccessNoCoe
///
/// The iterator's `size_hint` must be exact and cheap to call.
///
/// `TrustedRandomAccessNoCoerce::size` may not be overridden.
/// `TrustedRandomAccess::size` may not be overridden.
///
/// All subtypes and all supertypes of `Self` must also implement `TrustedRandomAccess`.
/// In particular, this means that types with non-invariant parameters usually can not have
Expand Down Expand Up @@ -372,42 +361,21 @@ impl<A: Debug + TrustedRandomAccessNoCoerce, B: Debug + TrustedRandomAccessNoCoe
/// * `std::iter::DoubleEndedIterator::next_back`
/// * `std::iter::ExactSizeIterator::len`
/// * `std::iter::Iterator::__iterator_get_unchecked`
/// * `std::iter::TrustedRandomAccessNoCoerce::size`
/// 5. If `T` is a subtype of `Self`, then `self` is allowed to be coerced
/// to `T`. If `self` is coerced to `T` after `self.__iterator_get_unchecked(idx)` has already
/// been called, then no methods except for the ones listed under 4. are allowed to be called
/// on the resulting value of type `T`, either. Multiple such coercion steps are allowed.
/// Regarding 2. and 3., the number of times `__iterator_get_unchecked(idx)` or `next_back()` is
/// called on `self` and the resulting value of type `T` (and on further coercion results with
/// sub-subtypes) are added together and their sums must not exceed the specified bounds.
/// * `std::iter::TrustedRandomAccess::size`
///
/// Further, given that these conditions are met, it must guarantee that:
///
/// * It does not change the value returned from `size_hint`
/// * It must be safe to call the methods listed above on `self` after calling
/// `self.__iterator_get_unchecked(idx)`, assuming that the required traits are implemented.
/// * It must also be safe to drop `self` after calling `self.__iterator_get_unchecked(idx)`.
/// * If `T` is a subtype of `Self`, then it must be safe to coerce `self` to `T`.
//
// FIXME: Clarify interaction with SourceIter/InPlaceIterable. Calling `SourceIter::as_inner`
// after `__iterator_get_unchecked` is supposed to be allowed.
#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
#[rustc_specialization_trait]
pub unsafe trait TrustedRandomAccess: TrustedRandomAccessNoCoerce {}

/// Like [`TrustedRandomAccess`] but without any of the requirements / guarantees around
/// coercions to subtypes after `__iterator_get_unchecked` (they aren’t allowed here!), and
/// without the requirement that subtypes / supertypes implement `TrustedRandomAccessNoCoerce`.
///
/// This trait was created in PR #85874 to fix soundness issue #85873 without performance regressions.
/// It is subject to change as we might want to build a more generally useful (for performance
/// optimizations) and more sophisticated trait or trait hierarchy that replaces or extends
/// [`TrustedRandomAccess`] and `TrustedRandomAccessNoCoerce`.
#[doc(hidden)]
#[unstable(feature = "trusted_random_access", issue = "none")]
#[rustc_specialization_trait]
pub unsafe trait TrustedRandomAccessNoCoerce: Sized {
pub unsafe trait TrustedRandomAccess: Sized {
// Convenience method.
fn size(&self) -> usize
where
Expand Down Expand Up @@ -450,7 +418,7 @@ unsafe impl<I: Iterator> SpecTrustedRandomAccess for I {
}
}

unsafe impl<I: Iterator + TrustedRandomAccessNoCoerce> SpecTrustedRandomAccess for I {
unsafe impl<I: Iterator + TrustedRandomAccess> SpecTrustedRandomAccess for I {
#[inline]
unsafe fn try_get_unchecked(&mut self, index: usize) -> Self::Item {
// SAFETY: the caller must uphold the contract for
Expand Down
Loading

0 comments on commit aec6846

Please sign in to comment.