Auto merge of #83772 - jhpratt:revamp-step-trait, r=Mark-Simulacrum

Make `Step` trait safe to implement

This PR makes a few modifications to the `Step` trait that I believe better position it for stabilization in the short term. In particular,

1. `unsafe trait TrustedStep` is introduced, indicating that the implementation of `Step` for a given type upholds all stated invariants (which have remained unchanged). This is gated behind a new `trusted_step` feature, as stabilization is realistically blocked on min_specialization.
2. The `Step` trait is internally specialized on the `TrustedStep` trait, which avoids a serious performance regression.
3. `TrustedLen` is implemented for `T: TrustedStep` as the latter's invariants subsume the former's.
4. The `Step` trait is no longer `unsafe`, as the invariants must not be relied upon by unsafe code (unless the type implements `TrustedStep`).
5. `TrustedStep` is implemented for all types that implement `Step` in the standard library and compiler.
6. The `step_trait_ext` feature is merged into the `step_trait` feature. I was unable to find any reasoning for the features being split; the `_unchecked` methods need not necessarily be stabilized at the same time, but I think it is useful to have them under the same feature flag.

All existing implementations of `Step` will be broken, as it is not possible to `unsafe impl` a safe trait. Given this trait only exists on nightly, I feel this breakage is acceptable. The blanket `impl<T: Step> TrustedLen for T` will likely cause some minor breakage, but this should be covered by the equivalent impl for `TrustedStep`.

Hopefully these changes are sufficient to place `Step` in decent position for stabilization, which would allow user-defined types to be used with `a..b` syntax.

Author: bors

compiler/rustc_ast/src/lib.rs

@@ -17,6 +17,8 @@
 #![feature(iter_zip)]
 #![feature(label_break_value)]
 #![feature(nll)]
+#![feature(min_specialization)]
+#![feature(trusted_step)]
 #![recursion_limit = "256"]
 
 #[macro_use]

compiler/rustc_hir/src/lib.rs

@@ -7,6 +7,8 @@
 #![cfg_attr(bootstrap, feature(extended_key_value_attributes))]
 #![feature(in_band_lifetimes)]
 #![feature(once_cell)]
+#![feature(min_specialization)]
+#![feature(trusted_step)]
 #![recursion_limit = "256"]
 
 #[macro_use]

compiler/rustc_index/src/lib.rs

@@ -6,6 +6,7 @@
 #![feature(unboxed_closures)]
 #![feature(test)]
 #![feature(fn_traits)]
+#![feature(trusted_step)]
 
 pub mod bit_set;
 pub mod vec;

compiler/rustc_index/src/vec.rs

@@ -65,7 +65,7 @@ impl Idx for u32 {
 /// `u32::MAX`. You can also customize things like the `Debug` impl,
 /// what traits are derived, and so forth via the macro.
 #[macro_export]
-#[allow_internal_unstable(step_trait, step_trait_ext, rustc_attrs)]
+#[allow_internal_unstable(step_trait, rustc_attrs)]
 macro_rules! newtype_index {
     // ---- public rules ----
 
@@ -184,7 +184,7 @@ macro_rules! newtype_index {
             }
         }
 
-        unsafe impl ::std::iter::Step for $type {
+        impl ::std::iter::Step for $type {
             #[inline]
             fn steps_between(start: &Self, end: &Self) -> Option<usize> {
                 <usize as ::std::iter::Step>::steps_between(
@@ -204,6 +204,9 @@ macro_rules! newtype_index {
             }
         }
 
+        // Safety: The implementation of `Step` upholds all invariants.
+        unsafe impl ::std::iter::TrustedStep for $type {}
+
         impl From<$type> for u32 {
             #[inline]
             fn from(v: $type) -> u32 {

compiler/rustc_infer/src/lib.rs

@@ -22,6 +22,8 @@
 #![feature(never_type)]
 #![feature(in_band_lifetimes)]
 #![feature(control_flow_enum)]
+#![feature(min_specialization)]
+#![feature(trusted_step)]
 #![recursion_limit = "512"] // For rustdoc
 
 #[macro_use]

compiler/rustc_middle/src/lib.rs

@@ -50,6 +50,7 @@
 #![feature(associated_type_defaults)]
 #![feature(iter_zip)]
 #![feature(thread_local_const_init)]
+#![feature(trusted_step)]
 #![recursion_limit = "512"]
 
 #[macro_use]

compiler/rustc_mir/src/lib.rs

@@ -31,6 +31,7 @@ Rust MIR: a lowered representation of Rust.
 #![feature(option_get_or_insert_default)]
 #![feature(once_cell)]
 #![feature(control_flow_enum)]
+#![feature(trusted_step)]
 #![recursion_limit = "256"]
 
 #[macro_use]

compiler/rustc_mir_build/src/lib.rs

@@ -10,6 +10,8 @@
 #![feature(bool_to_option)]
 #![feature(iter_zip)]
 #![feature(once_cell)]
+#![feature(min_specialization)]
+#![feature(trusted_step)]
 #![recursion_limit = "256"]
 
 #[macro_use]

compiler/rustc_passes/src/lib.rs

@@ -10,6 +10,8 @@
 #![feature(in_band_lifetimes)]
 #![feature(iter_zip)]
 #![feature(nll)]
+#![feature(min_specialization)]
+#![feature(trusted_step)]
 #![recursion_limit = "256"]
 
 #[macro_use]

compiler/rustc_query_system/src/lib.rs

@@ -6,6 +6,7 @@
 #![feature(iter_zip)]
 #![feature(min_specialization)]
 #![feature(stmt_expr_attributes)]
+#![feature(trusted_step)]
 
 #[macro_use]
 extern crate tracing;

compiler/rustc_span/src/lib.rs

@@ -21,6 +21,7 @@
 #![feature(nll)]
 #![feature(min_specialization)]
 #![feature(thread_local_const_init)]
+#![feature(trusted_step)]
 
 #[macro_use]
 extern crate rustc_macros;

compiler/rustc_target/src/lib.rs

@@ -14,6 +14,8 @@
 #![feature(never_type)]
 #![feature(associated_type_bounds)]
 #![feature(exhaustive_patterns)]
+#![feature(min_specialization)]
+#![feature(trusted_step)]
 
 use std::path::{Path, PathBuf};
 

compiler/rustc_type_ir/src/lib.rs

@@ -1,3 +1,6 @@
+#![feature(min_specialization)]
+#![feature(trusted_step)]
+
 #[macro_use]
 extern crate bitflags;
 #[macro_use]

library/core/src/iter/mod.rs

@@ -384,6 +384,8 @@ pub use self::traits::FusedIterator;
 pub use self::traits::InPlaceIterable;
 #[unstable(feature = "trusted_len", issue = "37572")]
 pub use self::traits::TrustedLen;
+#[unstable(feature = "trusted_step", issue = "85731")]
+pub use self::traits::TrustedStep;
 #[stable(feature = "rust1", since = "1.0.0")]
 pub use self::traits::{
     DoubleEndedIterator, ExactSizeIterator, Extend, FromIterator, IntoIterator, Product, Sum,

library/core/src/iter/range.rs

@@ -1,3 +1,5 @@
+use crate::iter::Step;
+
 /// An iterator that always continues to yield `None` when exhausted.
 ///
 /// Calling next on a fused iterator that has returned `None` once is guaranteed
@@ -55,3 +57,18 @@ unsafe impl<I: TrustedLen + ?Sized> TrustedLen for &mut I {}
 #[unstable(issue = "none", feature = "inplace_iteration")]
 #[doc(hidden)]
 pub unsafe trait InPlaceIterable: Iterator {}
+
+/// A type that upholds all invariants of [`Step`].
+///
+/// The invariants of [`Step::steps_between()`] are a superset of the invariants
+/// of [`TrustedLen`]. As such, [`TrustedLen`] is implemented for all range
+/// types with the same generic type argument.
+///
+/// # Safety
+///
+/// The implementation of [`Step`] for the given type must guarantee all
+/// invariants of all methods are upheld. See the [`Step`] trait's documentation
+/// for details. Consumers are free to rely on the invariants in unsafe code.
+#[unstable(feature = "trusted_step", issue = "85731")]
+#[rustc_specialization_trait]
+pub unsafe trait TrustedStep: Step {}

library/core/src/iter/traits/marker.rs

@@ -13,5 +13,7 @@ pub use self::exact_size::ExactSizeIterator;
 pub use self::iterator::Iterator;
 #[unstable(issue = "none", feature = "inplace_iteration")]
 pub use self::marker::InPlaceIterable;
+#[unstable(feature = "trusted_step", issue = "85731")]
+pub use self::marker::TrustedStep;
 #[stable(feature = "rust1", since = "1.0.0")]
 pub use self::marker::{FusedIterator, TrustedLen};

library/core/src/iter/traits/mod.rs

@@ -40,7 +40,6 @@
 #![feature(maybe_uninit_write_slice)]
 #![feature(min_specialization)]
 #![feature(step_trait)]
-#![feature(step_trait_ext)]
 #![feature(str_internals)]
 #![feature(test)]
 #![feature(trusted_len)]

library/core/tests/lib.rs

@@ -32,6 +32,10 @@
                   scope 5 {
                       debug i => _15;      // in scope 5 at $DIR/remove_storage_markers.rs:8:9: 8:10
                   }
+                  scope 7 (inlined iter::range::<impl Iterator for std::ops::Range<i32>>::next) { // at $DIR/remove_storage_markers.rs:8:14: 8:19
+                      debug self => _9;    // in scope 7 at $DIR/remove_storage_markers.rs:8:14: 8:19
+                      let mut _18: &mut std::ops::Range<i32>; // in scope 7 at $DIR/remove_storage_markers.rs:8:14: 8:19
+                  }
               }
           }
           scope 6 (inlined <std::ops::Range<i32> as IntoIterator>::into_iter) { // at $DIR/remove_storage_markers.rs:8:14: 8:19
@@ -61,19 +65,15 @@
 -         StorageLive(_10);                // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19
           _10 = &mut _4;                   // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19
           _9 = &mut (*_10);                // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19
-          _8 = <std::ops::Range<i32> as Iterator>::next(move _9) -> bb2; // scope 3 at $DIR/remove_storage_markers.rs:8:14: 8:19
+-         StorageLive(_18);                // scope 7 at $DIR/remove_storage_markers.rs:8:14: 8:19
+          _18 = &mut (*_9);                // scope 7 at $DIR/remove_storage_markers.rs:8:14: 8:19
+          _8 = <std::ops::Range<i32> as iter::range::RangeIteratorImpl>::spec_next(move _18) -> bb4; // scope 7 at $DIR/remove_storage_markers.rs:8:14: 8:19
                                            // mir::Constant
                                            // + span: $DIR/remove_storage_markers.rs:8:14: 8:19
-                                           // + literal: Const { ty: for<'r> fn(&'r mut std::ops::Range<i32>) -> std::option::Option<<std::ops::Range<i32> as std::iter::Iterator>::Item> {<std::ops::Range<i32> as std::iter::Iterator>::next}, val: Value(Scalar(<ZST>)) }
+                                           // + literal: Const { ty: for<'r> fn(&'r mut std::ops::Range<i32>) -> std::option::Option<<std::ops::Range<i32> as std::iter::range::RangeIteratorImpl>::Item> {<std::ops::Range<i32> as std::iter::range::RangeIteratorImpl>::spec_next}, val: Value(Scalar(<ZST>)) }
       }
 
       bb2: {
--         StorageDead(_9);                 // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19
-          _11 = discriminant(_8);          // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10
-          switchInt(move _11) -> [0_isize: bb3, otherwise: bb4]; // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10
-      }
-  
-      bb3: {
           _0 = const ();                   // scope 3 at $DIR/remove_storage_markers.rs:8:5: 10:6
 -         StorageDead(_10);                // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19
 -         StorageDead(_8);                 // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19
@@ -85,7 +85,7 @@
           return;                          // scope 0 at $DIR/remove_storage_markers.rs:11:2: 11:2
       }
 
-      bb4: {
+      bb3: {
 -         StorageLive(_12);                // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10
           _12 = ((_8 as Some).0: i32);     // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10
 -         StorageLive(_13);                // scope 4 at $DIR/remove_storage_markers.rs:8:9: 8:10
@@ -111,5 +111,12 @@
 -         StorageDead(_6);                 // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6
           goto -> bb1;                     // scope 2 at $DIR/remove_storage_markers.rs:8:5: 10:6
       }
+  
+      bb4: {
+-         StorageDead(_18);                // scope 7 at $DIR/remove_storage_markers.rs:8:14: 8:19
+-         StorageDead(_9);                 // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19
+          _11 = discriminant(_8);          // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10
+          switchInt(move _11) -> [0_isize: bb2, otherwise: bb3]; // scope 3 at $DIR/remove_storage_markers.rs:8:9: 8:10
+      }
   }
 

src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff

@@ -3,7 +3,6 @@
 
 #![feature(fn_traits,
            step_trait,
-           step_trait_ext,
            unboxed_closures,
 )]
 
@@ -157,7 +156,7 @@ impl<'a, 'b> std::ops::Add<&'b NaiveDate> for &'a NaiveDate {
     }
 }
 
-unsafe impl std::iter::Step for NaiveDate {
+impl std::iter::Step for NaiveDate {
     fn steps_between(_: &Self, _: &Self) -> Option<usize> {
         unimplemented!()
     }