From 1aff08010d6b35f2c7cef685b77ff1245c45a523 Mon Sep 17 00:00:00 2001 From: Waffle Date: Fri, 24 Jan 2020 14:49:34 +0300 Subject: [PATCH 01/10] Add `Iterator::map_while` method and corresponding `MapWhile` adapter --- src/libcore/iter/adapters/mod.rs | 89 ++++++++++++++++++++++++ src/libcore/iter/mod.rs | 2 + src/libcore/iter/traits/iterator.rs | 102 +++++++++++++++++++++++++++- src/libcore/tests/iter.rs | 2 + src/libcore/tests/lib.rs | 1 + 5 files changed, 195 insertions(+), 1 deletion(-) diff --git a/src/libcore/iter/adapters/mod.rs b/src/libcore/iter/adapters/mod.rs index 5787b9174edab..6ef4a49434002 100644 --- a/src/libcore/iter/adapters/mod.rs +++ b/src/libcore/iter/adapters/mod.rs @@ -1752,6 +1752,95 @@ where } } +/// An iterator that only accepts elements while `predicate` returns `Some(_)`. +/// +/// This `struct` is created by the [`map_while`] method on [`Iterator`]. See its +/// documentation for more. +/// +/// [`map_while`]: trait.Iterator.html#method.map_while +/// [`Iterator`]: trait.Iterator.html +#[must_use = "iterators are lazy and do nothing unless consumed"] +#[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] +#[derive(Clone)] +pub struct MapWhile { + iter: I, + finished: bool, + predicate: P, +} + +impl MapWhile { + pub(super) fn new(iter: I, predicate: P) -> MapWhile { + MapWhile { iter, finished: false, predicate } + } +} + +#[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] +impl fmt::Debug for MapWhile { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("MapWhile").field("iter", &self.iter).field("flag", &self.finished).finish() + } +} + +#[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] +impl Iterator for MapWhile +where + P: FnMut(I::Item) -> Option, +{ + type Item = B; + + #[inline] + fn next(&mut self) -> Option { + if self.finished { + None + } else { + let x = self.iter.next()?; + let ret = (self.predicate)(x); + self.finished = ret.is_none(); + ret + } + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + if self.finished { + (0, Some(0)) + } else { + let (_, upper) = self.iter.size_hint(); + (0, upper) // can't know a lower bound, due to the predicate + } + } + + #[inline] + fn try_fold(&mut self, init: Acc, fold: Fold) -> R + where + Self: Sized, + Fold: FnMut(Acc, Self::Item) -> R, + R: Try, + { + fn check<'a, B, T, Acc, R: Try>( + flag: &'a mut bool, + p: &'a mut impl FnMut(T) -> Option, + mut fold: impl FnMut(Acc, B) -> R + 'a, + ) -> impl FnMut(Acc, T) -> LoopState + 'a { + move |acc, x| match p(x) { + Some(item) => LoopState::from_try(fold(acc, item)), + None => { + *flag = true; + LoopState::Break(Try::from_ok(acc)) + } + } + } + + if self.finished { + Try::from_ok(init) + } else { + let flag = &mut self.finished; + let p = &mut self.predicate; + self.iter.try_fold(init, check(flag, p, fold)).into_try() + } + } +} + #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for TakeWhile where diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs index 0d5af3986fbff..c9f5e77d14b3e 100644 --- a/src/libcore/iter/mod.rs +++ b/src/libcore/iter/mod.rs @@ -351,6 +351,8 @@ pub use self::adapters::Cloned; pub use self::adapters::Copied; #[stable(feature = "iterator_flatten", since = "1.29.0")] pub use self::adapters::Flatten; +#[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] +pub use self::adapters::MapWhile; #[stable(feature = "iterator_step_by", since = "1.28.0")] pub use self::adapters::StepBy; #[stable(feature = "rust1", since = "1.0.0")] diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 21a569867b178..6bb92099ced51 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1,4 +1,6 @@ // ignore-tidy-filelength +// This file almost exclusively consists of the definition of `Iterator`. We +// can't split that into multiple files. use crate::cmp::{self, Ordering}; use crate::ops::{Add, Try}; @@ -7,7 +9,9 @@ use super::super::LoopState; use super::super::{Chain, Cloned, Copied, Cycle, Enumerate, Filter, FilterMap, Fuse}; use super::super::{FlatMap, Flatten}; use super::super::{FromIterator, Product, Sum, Zip}; -use super::super::{Inspect, Map, Peekable, Rev, Scan, Skip, SkipWhile, StepBy, Take, TakeWhile}; +use super::super::{ + Inspect, Map, MapWhile, Peekable, Rev, Scan, Skip, SkipWhile, StepBy, Take, TakeWhile, +}; fn _assert_is_object_safe(_: &dyn Iterator) {} @@ -1026,6 +1030,102 @@ pub trait Iterator { TakeWhile::new(self, predicate) } + /// Creates an iterator that both yields elements based on a predicate and maps. + /// + /// `map_while()` takes a closure as an argument. It will call this + /// closure on each element of the iterator, and yield elements + /// while it returns [`Some(_)`][`Some`]. + /// + /// After [`None`] is returned, `map_while()`'s job is over, and the + /// rest of the elements are ignored. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ``` + /// #![feature(iter_map_while)] + /// let a = [-1i32, 4, 0, 1]; + /// + /// let mut iter = a.iter().map_while(|x| 16i32.checked_div(*x)); + /// + /// assert_eq!(iter.next(), Some(-16)); + /// assert_eq!(iter.next(), Some(4)); + /// assert_eq!(iter.next(), None); + /// ``` + /// + /// Here's the same example, but with [`take_while`] and [`map`]: + /// + /// [`take_while`]: #method.take_while + /// [`map`]: #method.map + /// + /// ``` + /// let a = [-1i32, 4, 0, 1]; + /// + /// let mut iter = a.iter() + /// .map(|x| 16i32.checked_div(*x)) + /// .take_while(|x| x.is_some()) + /// .map(|x| x.unwrap()); + /// + /// assert_eq!(iter.next(), Some(-16)); + /// assert_eq!(iter.next(), Some(4)); + /// assert_eq!(iter.next(), None); + /// ``` + /// + /// Stopping after an initial [`None`]: + /// + /// ``` + /// #![feature(iter_map_while)] + /// use std::convert::TryFrom; + /// + /// let a = [0, -1, 1, -2]; + /// + /// let mut iter = a.iter().map_while(|x| u32::try_from(*x).ok()); + /// + /// assert_eq!(iter.next(), Some(0u32)); + /// + /// // We have more elements that are fit in u32, but since we already + /// // got a None, map_while() isn't used any more + /// assert_eq!(iter.next(), None); + /// ``` + /// + /// Because `map_while()` needs to look at the value in order to see if it + /// should be included or not, consuming iterators will see that it is + /// removed: + /// + /// ``` + /// #![feature(iter_map_while)] + /// use std::convert::TryFrom; + /// + /// let a = [1, 2, -3, 4]; + /// let mut iter = a.iter(); + /// + /// let result: Vec = iter.by_ref() + /// .map_while(|n| u32::try_from(*n).ok()) + /// .collect(); + /// + /// assert_eq!(result, &[1, 2]); + /// + /// let result: Vec = iter.cloned().collect(); + /// + /// assert_eq!(result, &[4]); + /// ``` + /// + /// The `-3` is no longer there, because it was consumed in order to see if + /// the iteration should stop, but wasn't placed back into the iterator. + /// + /// [`Some`]: ../../std/option/enum.Option.html#variant.Some + /// [`None`]: ../../std/option/enum.Option.html#variant.None + #[inline] + #[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] + fn map_while(self, predicate: P) -> MapWhile + where + Self: Sized, + P: FnMut(Self::Item) -> Option, + { + MapWhile::new(self, predicate) + } + /// Creates an iterator that skips the first `n` elements. /// /// After they have been consumed, the rest of the elements are yielded. diff --git a/src/libcore/tests/iter.rs b/src/libcore/tests/iter.rs index 8b8dc941534ee..bd3218ec27f32 100644 --- a/src/libcore/tests/iter.rs +++ b/src/libcore/tests/iter.rs @@ -1477,6 +1477,7 @@ fn test_iterator_size_hint() { assert_eq!(c.clone().take(5).size_hint(), (5, Some(5))); assert_eq!(c.clone().skip(5).size_hint().1, None); assert_eq!(c.clone().take_while(|_| false).size_hint(), (0, None)); + assert_eq!(c.clone().map_while(|_| None::<()>).size_hint(), (0, None)); assert_eq!(c.clone().skip_while(|_| false).size_hint(), (0, None)); assert_eq!(c.clone().enumerate().size_hint(), (usize::MAX, None)); assert_eq!(c.clone().chain(vi.clone().cloned()).size_hint(), (usize::MAX, None)); @@ -1491,6 +1492,7 @@ fn test_iterator_size_hint() { assert_eq!(vi.clone().skip(3).size_hint(), (7, Some(7))); assert_eq!(vi.clone().skip(12).size_hint(), (0, Some(0))); assert_eq!(vi.clone().take_while(|_| false).size_hint(), (0, Some(10))); + assert_eq!(vi.clone().map_while(|_| None::<()>).size_hint(), (0, Some(10))); assert_eq!(vi.clone().skip_while(|_| false).size_hint(), (0, Some(10))); assert_eq!(vi.clone().enumerate().size_hint(), (10, Some(10))); assert_eq!(vi.clone().chain(v2).size_hint(), (13, Some(13))); diff --git a/src/libcore/tests/lib.rs b/src/libcore/tests/lib.rs index 567c840b01ade..8fd19ef67fccf 100644 --- a/src/libcore/tests/lib.rs +++ b/src/libcore/tests/lib.rs @@ -36,6 +36,7 @@ #![feature(iter_is_partitioned)] #![feature(iter_order_by)] #![feature(cmp_min_max_by)] +#![feature(iter_map_while)] #![feature(const_slice_from_raw_parts)] #![feature(const_raw_ptr_deref)] #![feature(never_type)] From db1a107b3f920637dc785fcc6d6bbe247a271e7b Mon Sep 17 00:00:00 2001 From: Waffle Date: Sat, 25 Jan 2020 21:23:22 +0300 Subject: [PATCH 02/10] Fill tracking issue for `iter_map_while` feature --- src/libcore/iter/adapters/mod.rs | 6 +++--- src/libcore/iter/mod.rs | 2 +- src/libcore/iter/traits/iterator.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libcore/iter/adapters/mod.rs b/src/libcore/iter/adapters/mod.rs index 6ef4a49434002..7d10ef3d28219 100644 --- a/src/libcore/iter/adapters/mod.rs +++ b/src/libcore/iter/adapters/mod.rs @@ -1760,7 +1760,7 @@ where /// [`map_while`]: trait.Iterator.html#method.map_while /// [`Iterator`]: trait.Iterator.html #[must_use = "iterators are lazy and do nothing unless consumed"] -#[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] +#[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] #[derive(Clone)] pub struct MapWhile { iter: I, @@ -1774,14 +1774,14 @@ impl MapWhile { } } -#[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] +#[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] impl fmt::Debug for MapWhile { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("MapWhile").field("iter", &self.iter).field("flag", &self.finished).finish() } } -#[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] +#[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] impl Iterator for MapWhile where P: FnMut(I::Item) -> Option, diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs index c9f5e77d14b3e..d8a56cb3ae515 100644 --- a/src/libcore/iter/mod.rs +++ b/src/libcore/iter/mod.rs @@ -351,7 +351,7 @@ pub use self::adapters::Cloned; pub use self::adapters::Copied; #[stable(feature = "iterator_flatten", since = "1.29.0")] pub use self::adapters::Flatten; -#[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] +#[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] pub use self::adapters::MapWhile; #[stable(feature = "iterator_step_by", since = "1.28.0")] pub use self::adapters::StepBy; diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 6bb92099ced51..1d055676c7708 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1117,7 +1117,7 @@ pub trait Iterator { /// [`Some`]: ../../std/option/enum.Option.html#variant.Some /// [`None`]: ../../std/option/enum.Option.html#variant.None #[inline] - #[unstable(feature = "iter_map_while", reason = "recently added", issue = "none")] + #[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] fn map_while(self, predicate: P) -> MapWhile where Self: Sized, From 50ed6cbf9836a48c938e74bb28501ffbbe59585e Mon Sep 17 00:00:00 2001 From: Hiroki Noda Date: Thu, 30 Jan 2020 10:07:36 +0900 Subject: [PATCH 03/10] Fix typo. --- src/libstd/sys/unix/thread.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index a5b34eeec289e..58b4839f9028f 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -426,7 +426,7 @@ pub mod guard { } // glibc >= 2.15 has a __pthread_get_minstack() function that returns -// PTHREAD_STACK_MIN plus however many bytes are needed for thread-local +// PTHREAD_STACK_MIN plus how many bytes are needed for thread-local // storage. We need that information to avoid blowing up when a small stack // is created in an application with big thread-local storage requirements. // See #6233 for rationale and details. From c870ca6217038e80138b69a88b2340b62859f52b Mon Sep 17 00:00:00 2001 From: Hiroki Noda Date: Thu, 30 Jan 2020 10:31:12 +0900 Subject: [PATCH 04/10] Update --- src/libstd/sys/unix/thread.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index 58b4839f9028f..3ca778354e413 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -426,8 +426,8 @@ pub mod guard { } // glibc >= 2.15 has a __pthread_get_minstack() function that returns -// PTHREAD_STACK_MIN plus how many bytes are needed for thread-local -// storage. We need that information to avoid blowing up when a small stack +// PTHREAD_STACK_MIN plus bytes needed for thread-local storage. +// We need that information to avoid blowing up when a small stack // is created in an application with big thread-local storage requirements. // See #6233 for rationale and details. #[cfg(target_os = "linux")] From ad0e4a167d950607430ff95022219671ab7054db Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 29 Jan 2020 23:47:01 -0800 Subject: [PATCH 05/10] Update jobserver crate to 0.1.21 Brings in a fix for alexcrichton/jobserver-rs#23 which could cause Cargo to unnecessarily hang in some situations. --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2c4063a5e565e..c081f2fbf20ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1642,9 +1642,9 @@ dependencies = [ [[package]] name = "jobserver" -version = "0.1.19" +version = "0.1.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67b06c1b455f1cf4269a8cfc320ab930a810e2375a42af5075eb8a8b36405ce0" +checksum = "5c71313ebb9439f74b00d9d2dcec36440beaf57a6aa0623068441dd7cd81a7f2" dependencies = [ "libc", ] From 2f575dab3030467525acae204e47f7a9a8311530 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 29 Jan 2020 13:32:37 +0100 Subject: [PATCH 06/10] Add missing links for cmp traits --- src/libcore/cmp.rs | 51 ++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/src/libcore/cmp.rs b/src/libcore/cmp.rs index 3ea4baa57b49e..e41a7afd3e223 100644 --- a/src/libcore/cmp.rs +++ b/src/libcore/cmp.rs @@ -35,7 +35,7 @@ use self::Ordering::*; /// /// This trait allows for partial equality, for types that do not have a full /// equivalence relation. For example, in floating point numbers `NaN != NaN`, -/// so floating point types implement `PartialEq` but not `Eq`. +/// so floating point types implement `PartialEq` but not [`Eq`]. /// /// Formally, the equality must be (for all `a`, `b` and `c`): /// @@ -55,12 +55,12 @@ use self::Ordering::*; /// /// ## How can I implement `PartialEq`? /// -/// PartialEq only requires the `eq` method to be implemented; `ne` is defined -/// in terms of it by default. Any manual implementation of `ne` *must* respect -/// the rule that `eq` is a strict inverse of `ne`; that is, `!(a == b)` if and +/// `PartialEq` only requires the [`eq`] method to be implemented; [`ne`] is defined +/// in terms of it by default. Any manual implementation of [`ne`] *must* respect +/// the rule that [`eq`] is a strict inverse of [`ne`]; that is, `!(a == b)` if and /// only if `a != b`. /// -/// Implementations of `PartialEq`, `PartialOrd`, and `Ord` *must* agree with +/// Implementations of `PartialEq`, [`PartialOrd`], and [`Ord`] *must* agree with /// each other. It's easy to accidentally make them disagree by deriving some /// of the traits and manually implementing others. /// @@ -190,6 +190,9 @@ use self::Ordering::*; /// assert_eq!(x == y, false); /// assert_eq!(x.eq(&y), false); /// ``` +/// +/// [`eq`]: PartialEq::eq +/// [`ne`]: PartialEq::ne #[lang = "eq"] #[stable(feature = "rust1", since = "1.0.0")] #[doc(alias = "==")] @@ -233,7 +236,7 @@ pub macro PartialEq($item:item) { /// - transitive: `a == b` and `b == c` implies `a == c`. /// /// This property cannot be checked by the compiler, and therefore `Eq` implies -/// `PartialEq`, and has no extra methods. +/// [`PartialEq`], and has no extra methods. /// /// ## Derivable /// @@ -370,6 +373,7 @@ impl Ordering { /// Chains two orderings. /// /// Returns `self` when it's not `Equal`. Otherwise returns `other`. + /// /// # Examples /// /// ``` @@ -442,10 +446,12 @@ impl Ordering { /// A helper struct for reverse ordering. /// -/// This struct is a helper to be used with functions like `Vec::sort_by_key` and +/// This struct is a helper to be used with functions like [`Vec::sort_by_key`] and /// can be used to reverse order a part of a key. /// -/// Example usage: +/// [`Vec::sort_by_key`]: ../../std/vec/struct.Vec.html#method.sort_by_key +/// +/// # Examples /// /// ``` /// use std::cmp::Reverse; @@ -506,12 +512,12 @@ impl Ord for Reverse { /// /// ## How can I implement `Ord`? /// -/// `Ord` requires that the type also be `PartialOrd` and `Eq` (which requires `PartialEq`). +/// `Ord` requires that the type also be [`PartialOrd`] and [`Eq`] (which requires [`PartialEq`]). /// -/// Then you must define an implementation for `cmp()`. You may find it useful to use -/// `cmp()` on your type's fields. +/// Then you must define an implementation for [`cmp`]. You may find it useful to use +/// [`cmp`] on your type's fields. /// -/// Implementations of `PartialEq`, `PartialOrd`, and `Ord` *must* +/// Implementations of [`PartialEq`], [`PartialOrd`], and `Ord` *must* /// agree with each other. That is, `a.cmp(b) == Ordering::Equal` if /// and only if `a == b` and `Some(a.cmp(b)) == a.partial_cmp(b)` for /// all `a` and `b`. It's easy to accidentally make them disagree by @@ -548,13 +554,15 @@ impl Ord for Reverse { /// } /// } /// ``` +/// +/// [`cmp`]: Ord::cmp #[doc(alias = "<")] #[doc(alias = ">")] #[doc(alias = "<=")] #[doc(alias = ">=")] #[stable(feature = "rust1", since = "1.0.0")] pub trait Ord: Eq + PartialOrd { - /// This method returns an `Ordering` between `self` and `other`. + /// This method returns an [`Ordering`] between `self` and `other`. /// /// By convention, `self.cmp(&other)` returns the ordering matching the expression /// `self other` if true. @@ -689,20 +697,20 @@ impl PartialOrd for Ordering { /// /// ## How can I implement `PartialOrd`? /// -/// `PartialOrd` only requires implementation of the `partial_cmp` method, with the others +/// `PartialOrd` only requires implementation of the [`partial_cmp`] method, with the others /// generated from default implementations. /// /// However it remains possible to implement the others separately for types which do not have a /// total order. For example, for floating point numbers, `NaN < 0 == false` and `NaN >= 0 == /// false` (cf. IEEE 754-2008 section 5.11). /// -/// `PartialOrd` requires your type to be `PartialEq`. +/// `PartialOrd` requires your type to be [`PartialEq`]. /// -/// Implementations of `PartialEq`, `PartialOrd`, and `Ord` *must* agree with each other. It's +/// Implementations of [`PartialEq`], `PartialOrd`, and [`Ord`] *must* agree with each other. It's /// easy to accidentally make them disagree by deriving some of the traits and manually /// implementing others. /// -/// If your type is `Ord`, you can implement `partial_cmp()` by using `cmp()`: +/// If your type is [`Ord`], you can implement [`partial_cmp`] by using [`cmp`]: /// /// ``` /// use std::cmp::Ordering; @@ -733,7 +741,7 @@ impl PartialOrd for Ordering { /// } /// ``` /// -/// You may also find it useful to use `partial_cmp()` on your type's fields. Here +/// You may also find it useful to use [`partial_cmp`] on your type's fields. Here /// is an example of `Person` types who have a floating-point `height` field that /// is the only field to be used for sorting: /// @@ -768,6 +776,9 @@ impl PartialOrd for Ordering { /// assert_eq!(x < y, true); /// assert_eq!(x.lt(&y), true); /// ``` +/// +/// [`partial_cmp`]: PartialOrd::partial_cmp +/// [`cmp`]: Ord::cmp #[lang = "partial_ord"] #[stable(feature = "rust1", since = "1.0.0")] #[doc(alias = ">")] @@ -893,7 +904,7 @@ pub macro PartialOrd($item:item) { /// /// Returns the first argument if the comparison determines them to be equal. /// -/// Internally uses an alias to `Ord::min`. +/// Internally uses an alias to [`Ord::min`]. /// /// # Examples /// @@ -956,7 +967,7 @@ pub fn min_by_key K, K: Ord>(v1: T, v2: T, mut f: F) -> T { /// /// Returns the second argument if the comparison determines them to be equal. /// -/// Internally uses an alias to `Ord::max`. +/// Internally uses an alias to [`Ord::max`]. /// /// # Examples /// From d404d218aa2d4e81c5bb5276bfd149981ffb9803 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 22 Jan 2020 18:54:04 -0500 Subject: [PATCH 07/10] Add `#[repr(no_niche)]`. This repr-hint makes a struct/enum hide any niche within from its surrounding type-construction context. It is meant (at least initially) as an implementation detail for resolving issue 68303. We will not stabilize the repr-hint unless someone finds motivation for doing so. (So, declaration of `no_niche` feature lives in section of file where other internal implementation details are grouped, and deliberately leaves out the tracking issue number.) incorporated review feedback, and fixed post-rebase. --- src/librustc/ty/layout.rs | 25 +++++++++++++------ src/librustc/ty/mod.rs | 8 +++++- .../deriving/generic/mod.rs | 3 ++- src/librustc_feature/active.rs | 4 +++ src/librustc_passes/check_attr.rs | 24 +++++++++++++++--- src/librustc_span/symbol.rs | 2 ++ src/libsyntax/attr/builtin.rs | 2 ++ 7 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index acaa4eec9410d..ea1f7066c7562 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -356,12 +356,14 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { debug!("univariant offset: {:?} field: {:#?}", offset, field); offsets[i as usize] = offset; - if let Some(mut niche) = field.largest_niche.clone() { - let available = niche.available(dl); - if available > largest_niche_available { - largest_niche_available = available; - niche.offset += offset; - largest_niche = Some(niche); + if !repr.hide_niche() { + if let Some(mut niche) = field.largest_niche.clone() { + let available = niche.available(dl); + if available > largest_niche_available { + largest_niche_available = available; + niche.offset += offset; + largest_niche = Some(niche); + } } } @@ -838,7 +840,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { } // Update `largest_niche` if we have introduced a larger niche. - let niche = Niche::from_scalar(dl, Size::ZERO, scalar.clone()); + let niche = if def.repr.hide_niche() { + None + } else { + Niche::from_scalar(dl, Size::ZERO, scalar.clone()) + }; if let Some(niche) = niche { match &st.largest_niche { Some(largest_niche) => { @@ -863,6 +869,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { return Ok(tcx.intern_layout(st)); } + // At this point, we have handled all unions and + // structs. (We have also handled univariant enums + // that allow representation optimization.) + assert!(def.is_enum()); + // The current code for niche-filling relies on variant indices // instead of actual discriminants, so dataful enums with // explicit discriminants (RFC #2363) would misbehave. diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 4889f751f601b..3579db777d793 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2036,7 +2036,8 @@ bitflags! { const IS_TRANSPARENT = 1 << 2; // Internal only for now. If true, don't reorder fields. const IS_LINEAR = 1 << 3; - + // If true, don't expose any niche to type's context. + const HIDE_NICHE = 1 << 4; // Any of these flags being set prevent field reordering optimisation. const IS_UNOPTIMISABLE = ReprFlags::IS_C.bits | ReprFlags::IS_SIMD.bits | @@ -2073,6 +2074,7 @@ impl ReprOptions { ReprFlags::empty() } attr::ReprTransparent => ReprFlags::IS_TRANSPARENT, + attr::ReprNoNiche => ReprFlags::HIDE_NICHE, attr::ReprSimd => ReprFlags::IS_SIMD, attr::ReprInt(i) => { size = Some(i); @@ -2113,6 +2115,10 @@ impl ReprOptions { pub fn linear(&self) -> bool { self.flags.contains(ReprFlags::IS_LINEAR) } + #[inline] + pub fn hide_niche(&self) -> bool { + self.flags.contains(ReprFlags::HIDE_NICHE) + } pub fn discr_type(&self) -> attr::IntType { self.int.unwrap_or(attr::SignedInt(ast::IntTy::Isize)) diff --git a/src/librustc_builtin_macros/deriving/generic/mod.rs b/src/librustc_builtin_macros/deriving/generic/mod.rs index f8918016c1b98..76e4841d7ce21 100644 --- a/src/librustc_builtin_macros/deriving/generic/mod.rs +++ b/src/librustc_builtin_macros/deriving/generic/mod.rs @@ -824,7 +824,8 @@ fn find_repr_type_name(sess: &ParseSess, type_attrs: &[ast::Attribute]) -> &'sta attr::ReprPacked(_) | attr::ReprSimd | attr::ReprAlign(_) - | attr::ReprTransparent => continue, + | attr::ReprTransparent + | attr::ReprNoNiche => continue, attr::ReprC => "i32", diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index f20a57ea61c42..04d861418a8ad 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -204,6 +204,10 @@ declare_features! ( /// Added for testing E0705; perma-unstable. (active, test_2018_feature, "1.31.0", None, Some(Edition::Edition2018)), + /// Allows `#[repr(no_niche)]` (an implementation detail of `rustc`, + /// it is not on path for eventual stabilization). + (active, no_niche, "1.42.0", None, None), + // no-tracking-issue-end // ------------------------------------------------------------------------- diff --git a/src/librustc_passes/check_attr.rs b/src/librustc_passes/check_attr.rs index 3ff1ba3bbfc8c..81c5c3412b1db 100644 --- a/src/librustc_passes/check_attr.rs +++ b/src/librustc_passes/check_attr.rs @@ -16,9 +16,10 @@ use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::DUMMY_HIR_ID; use rustc_hir::{self, HirId, Item, ItemKind, TraitItem}; use rustc_session::lint::builtin::{CONFLICTING_REPR_HINTS, UNUSED_ATTRIBUTES}; +use rustc_session::parse::feature_err; use rustc_span::symbol::sym; use rustc_span::Span; -use syntax::ast::Attribute; +use syntax::ast::{Attribute, NestedMetaItem}; use syntax::attr; fn target_from_impl_item<'tcx>(tcx: TyCtxt<'tcx>, impl_item: &hir::ImplItem<'_>) -> Target { @@ -287,6 +288,21 @@ impl CheckAttrVisitor<'tcx> { _ => ("a", "struct, enum, or union"), } } + sym::no_niche => { + if !self.tcx.features().enabled(sym::no_niche) { + feature_err( + &self.tcx.sess.parse_sess, + sym::no_niche, + hint.span(), + "the attribute `repr(no_niche)` is currently unstable", + ) + .emit(); + } + match target { + Target::Struct | Target::Enum => continue, + _ => ("a", "struct or enum"), + } + } sym::i8 | sym::u8 | sym::i16 @@ -314,8 +330,10 @@ impl CheckAttrVisitor<'tcx> { // This is not ideal, but tracking precisely which ones are at fault is a huge hassle. let hint_spans = hints.iter().map(|hint| hint.span()); - // Error on repr(transparent, ). - if is_transparent && hints.len() > 1 { + // Error on repr(transparent, ). + let non_no_niche = |hint: &&NestedMetaItem| hint.name_or_empty() != sym::no_niche; + let non_no_niche_count = hints.iter().filter(non_no_niche).count(); + if is_transparent && non_no_niche_count > 1 { let hint_spans: Vec<_> = hint_spans.clone().collect(); struct_span_err!( self.tcx.sess, diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index e4f8b5a014389..f1bf746207a93 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -487,6 +487,7 @@ symbols! { None, non_exhaustive, non_modrs_mods, + no_niche, no_stack_check, no_start, no_std, @@ -583,6 +584,7 @@ symbols! { repr128, repr_align, repr_align_enum, + repr_no_niche, repr_packed, repr_simd, repr_transparent, diff --git a/src/libsyntax/attr/builtin.rs b/src/libsyntax/attr/builtin.rs index 6cfe4f2de1e96..401e57b9a3c3f 100644 --- a/src/libsyntax/attr/builtin.rs +++ b/src/libsyntax/attr/builtin.rs @@ -820,6 +820,7 @@ pub enum ReprAttr { ReprSimd, ReprTransparent, ReprAlign(u32), + ReprNoNiche, } #[derive(Eq, PartialEq, Debug, RustcEncodable, RustcDecodable, Copy, Clone, HashStable_Generic)] @@ -875,6 +876,7 @@ pub fn find_repr_attrs(sess: &ParseSess, attr: &Attribute) -> Vec { sym::packed => Some(ReprPacked(1)), sym::simd => Some(ReprSimd), sym::transparent => Some(ReprTransparent), + sym::no_niche => Some(ReprNoNiche), name => int_type_of_word(name).map(ReprInt), }; From 712c4336892cf79c72ff9c59d2488282000edd1e Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 22 Jan 2020 18:50:38 -0500 Subject: [PATCH 08/10] tests for `#[repr(no_niche)]`. --- src/test/ui/repr/feature-gate-no-niche.rs | 20 ++ src/test/ui/repr/feature-gate-no-niche.stderr | 35 ++ .../repr-no-niche-inapplicable-to-unions.rs | 14 + ...epr-no-niche-inapplicable-to-unions.stderr | 19 + src/test/ui/repr/repr-no-niche.rs | 329 ++++++++++++++++++ 5 files changed, 417 insertions(+) create mode 100644 src/test/ui/repr/feature-gate-no-niche.rs create mode 100644 src/test/ui/repr/feature-gate-no-niche.stderr create mode 100644 src/test/ui/repr/repr-no-niche-inapplicable-to-unions.rs create mode 100644 src/test/ui/repr/repr-no-niche-inapplicable-to-unions.stderr create mode 100644 src/test/ui/repr/repr-no-niche.rs diff --git a/src/test/ui/repr/feature-gate-no-niche.rs b/src/test/ui/repr/feature-gate-no-niche.rs new file mode 100644 index 0000000000000..8872ee7119e4a --- /dev/null +++ b/src/test/ui/repr/feature-gate-no-niche.rs @@ -0,0 +1,20 @@ +use std::num::NonZeroU8 as N8; +use std::num::NonZeroU16 as N16; + +#[repr(no_niche)] +pub struct Cloaked(N16); +//~^^ ERROR the attribute `repr(no_niche)` is currently unstable [E0658] + +#[repr(transparent, no_niche)] +pub struct Shadowy(N16); +//~^^ ERROR the attribute `repr(no_niche)` is currently unstable [E0658] + +#[repr(no_niche)] +pub enum Cloaked1 { _A(N16), } +//~^^ ERROR the attribute `repr(no_niche)` is currently unstable [E0658] + +#[repr(no_niche)] +pub enum Cloaked2 { _A(N16), _B(u8, N8) } +//~^^ ERROR the attribute `repr(no_niche)` is currently unstable [E0658] + +fn main() { } diff --git a/src/test/ui/repr/feature-gate-no-niche.stderr b/src/test/ui/repr/feature-gate-no-niche.stderr new file mode 100644 index 0000000000000..34fd417cc99a2 --- /dev/null +++ b/src/test/ui/repr/feature-gate-no-niche.stderr @@ -0,0 +1,35 @@ +error[E0658]: the attribute `repr(no_niche)` is currently unstable + --> $DIR/feature-gate-no-niche.rs:4:8 + | +LL | #[repr(no_niche)] + | ^^^^^^^^ + | + = help: add `#![feature(no_niche)]` to the crate attributes to enable + +error[E0658]: the attribute `repr(no_niche)` is currently unstable + --> $DIR/feature-gate-no-niche.rs:8:21 + | +LL | #[repr(transparent, no_niche)] + | ^^^^^^^^ + | + = help: add `#![feature(no_niche)]` to the crate attributes to enable + +error[E0658]: the attribute `repr(no_niche)` is currently unstable + --> $DIR/feature-gate-no-niche.rs:12:8 + | +LL | #[repr(no_niche)] + | ^^^^^^^^ + | + = help: add `#![feature(no_niche)]` to the crate attributes to enable + +error[E0658]: the attribute `repr(no_niche)` is currently unstable + --> $DIR/feature-gate-no-niche.rs:16:8 + | +LL | #[repr(no_niche)] + | ^^^^^^^^ + | + = help: add `#![feature(no_niche)]` to the crate attributes to enable + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/repr/repr-no-niche-inapplicable-to-unions.rs b/src/test/ui/repr/repr-no-niche-inapplicable-to-unions.rs new file mode 100644 index 0000000000000..308634651a384 --- /dev/null +++ b/src/test/ui/repr/repr-no-niche-inapplicable-to-unions.rs @@ -0,0 +1,14 @@ +#![feature(no_niche)] + +use std::num::NonZeroU8 as N8; +use std::num::NonZeroU16 as N16; + +#[repr(no_niche)] +pub union Cloaked1 { _A: N16 } +//~^^ ERROR attribute should be applied to struct or enum [E0517] + +#[repr(no_niche)] +pub union Cloaked2 { _A: N16, _B: (u8, N8) } +//~^^ ERROR attribute should be applied to struct or enum [E0517] + +fn main() { } diff --git a/src/test/ui/repr/repr-no-niche-inapplicable-to-unions.stderr b/src/test/ui/repr/repr-no-niche-inapplicable-to-unions.stderr new file mode 100644 index 0000000000000..4c542c5f0da64 --- /dev/null +++ b/src/test/ui/repr/repr-no-niche-inapplicable-to-unions.stderr @@ -0,0 +1,19 @@ +error[E0517]: attribute should be applied to struct or enum + --> $DIR/repr-no-niche-inapplicable-to-unions.rs:6:8 + | +LL | #[repr(no_niche)] + | ^^^^^^^^ +LL | pub union Cloaked1 { _A: N16 } + | ------------------------------ not a struct or enum + +error[E0517]: attribute should be applied to struct or enum + --> $DIR/repr-no-niche-inapplicable-to-unions.rs:10:8 + | +LL | #[repr(no_niche)] + | ^^^^^^^^ +LL | pub union Cloaked2 { _A: N16, _B: (u8, N8) } + | -------------------------------------------- not a struct or enum + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0517`. diff --git a/src/test/ui/repr/repr-no-niche.rs b/src/test/ui/repr/repr-no-niche.rs new file mode 100644 index 0000000000000..a7f0d509af521 --- /dev/null +++ b/src/test/ui/repr/repr-no-niche.rs @@ -0,0 +1,329 @@ +// run-pass + +// This file tests repr(no_niche), which causes an struct/enum to hide +// any niche space that may exist in its internal state from the +// context it appears in. + +// Here are the axes this test is seeking to cover: +// +// repr annotation: +// visible: (); cloaked: (no_niche); transparent: (transparent); shadowy: (transparent, no_niche) +// +// enum vs struct +// +// niche-type via type-parameter vs inline declaration + +#![feature(decl_macro)] +#![feature(no_niche)] + +use std::mem::size_of; +use std::num::{NonZeroU8, NonZeroU16}; + +mod struct_inline { + use std::num::NonZeroU16 as N16; + + #[derive(Debug)] pub struct Visible(N16); + + #[repr(no_niche)] + #[derive(Debug)] pub struct Cloaked(N16); + + #[repr(transparent)] + #[derive(Debug)] pub struct Transparent(N16); + + #[repr(transparent, no_niche)] + #[derive(Debug)] pub struct Shadowy(N16); +} + +mod struct_param { + #[derive(Debug)] pub struct Visible(T); + + #[repr(no_niche)] + #[derive(Debug)] pub struct Cloaked(T); + + #[repr(transparent)] + #[derive(Debug)] pub struct Transparent(T); + + #[repr(transparent, no_niche)] + #[derive(Debug)] pub struct Shadowy(T); +} + +mod enum_inline { + use crate::two_fifty_six_variant_enum; + use std::num::{NonZeroU8 as N8, NonZeroU16 as N16}; + + #[derive(Debug)] pub enum Visible1 { _A(N16), } + + #[repr(no_niche)] + #[derive(Debug)] pub enum Cloaked1 { _A(N16), } + + // (N.B.: transparent enums must be univariant) + #[repr(transparent)] + #[derive(Debug)] pub enum Transparent { _A(N16), } + + #[repr(transparent, no_niche)] + #[derive(Debug)] pub enum Shadowy { _A(N16), } + + // including multivariant enums for completeness. Payload and + // number of variants (i.e. discriminant size) have been chosen so + // that layout including discriminant is 4 bytes, with no space in + // padding to hide another discrimnant from the surrounding + // context. + // + // (Note that multivariant enums cannot usefully expose a niche in + // general; this test is relying on that.) + two_fifty_six_variant_enum!(Visible2, N8); + + #[repr(no_niche)] + two_fifty_six_variant_enum!(Cloaked2, N8); +} + +mod enum_param { + use super::two_fifty_six_variant_enum; + + #[derive(Debug)] pub enum Visible1 { _A(T), } + + #[repr(no_niche)] + #[derive(Debug)] pub enum Cloaked1 { _A(T), } + + // (N.B.: transparent enums must be univariant) + #[repr(transparent)] + #[derive(Debug)] pub enum Transparent { _A(T), } + + #[repr(transparent, no_niche)] + #[derive(Debug)] pub enum Shadowy { _A(T), } + + // including multivariant enums for completeness. Same notes apply + // here as above (assuming `T` is instantiated with `NonZeroU8`). + two_fifty_six_variant_enum!(Visible2); + + #[repr(no_niche)] + two_fifty_six_variant_enum!(Cloaked2); +} + +fn main() { + // sanity-checks + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 2); + + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2); + + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 2); // transparent enums are univariant + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 4); + assert_eq!(size_of::(), 4); + + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 4); + assert_eq!(size_of::>(), 4); + + // now the actual tests of no_niche: how do inputs above compose + // with `Option` type constructor. The cases with a `_+2` are the + // ones where no_niche fires. + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2+2); + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2+2); + + assert_eq!(size_of::>>(), 2); + assert_eq!(size_of::>>(), 2+2); + assert_eq!(size_of::>>(), 2); + assert_eq!(size_of::>>(), 2+2); + + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2+2); + assert_eq!(size_of::>(), 2); + assert_eq!(size_of::>(), 2+2); + // cannot use niche of multivariant payload + assert_eq!(size_of::>(), 4+2); + assert_eq!(size_of::>(), 4+2); + + assert_eq!(size_of::>>(), 2); + assert_eq!(size_of::>>(), 2+2); + assert_eq!(size_of::>>(), 2); + assert_eq!(size_of::>>(), 2+2); + // cannot use niche of multivariant payload + assert_eq!(size_of::>>(), 4+2); + assert_eq!(size_of::>>(), 4+2); +} + +macro two_fifty_six_variant_enum { + ($name:ident<$param:ident>) => { + #[derive(Debug)] + pub enum $name<$param> { + _V00($param, u16), _V01(u16, $param), _V02($param, u16), _V03(u16, $param), + _V04($param, u16), _V05(u16, $param), _V06($param, u16), _V07(u16, $param), + _V08($param, u16), _V09(u16, $param), _V0a($param, u16), _V0b(u16, $param), + _V0c($param, u16), _V0d(u16, $param), _V0e($param, u16), _V0f(u16, $param), + + _V10($param, u16), _V11(u16, $param), _V12($param, u16), _V13(u16, $param), + _V14($param, u16), _V15(u16, $param), _V16($param, u16), _V17(u16, $param), + _V18($param, u16), _V19(u16, $param), _V1a($param, u16), _V1b(u16, $param), + _V1c($param, u16), _V1d(u16, $param), _V1e($param, u16), _V1f(u16, $param), + + _V20($param, u16), _V21(u16, $param), _V22($param, u16), _V23(u16, $param), + _V24($param, u16), _V25(u16, $param), _V26($param, u16), _V27(u16, $param), + _V28($param, u16), _V29(u16, $param), _V2a($param, u16), _V2b(u16, $param), + _V2c($param, u16), _V2d(u16, $param), _V2e($param, u16), _V2f(u16, $param), + + _V30($param, u16), _V31(u16, $param), _V32($param, u16), _V33(u16, $param), + _V34($param, u16), _V35(u16, $param), _V36($param, u16), _V37(u16, $param), + _V38($param, u16), _V39(u16, $param), _V3a($param, u16), _V3b(u16, $param), + _V3c($param, u16), _V3d(u16, $param), _V3e($param, u16), _V3f(u16, $param), + + _V40($param, u16), _V41(u16, $param), _V42($param, u16), _V43(u16, $param), + _V44($param, u16), _V45(u16, $param), _V46($param, u16), _V47(u16, $param), + _V48($param, u16), _V49(u16, $param), _V4a($param, u16), _V4b(u16, $param), + _V4c($param, u16), _V4d(u16, $param), _V4e($param, u16), _V4f(u16, $param), + + _V50($param, u16), _V51(u16, $param), _V52($param, u16), _V53(u16, $param), + _V54($param, u16), _V55(u16, $param), _V56($param, u16), _V57(u16, $param), + _V58($param, u16), _V59(u16, $param), _V5a($param, u16), _V5b(u16, $param), + _V5c($param, u16), _V5d(u16, $param), _V5e($param, u16), _V5f(u16, $param), + + _V60($param, u16), _V61(u16, $param), _V62($param, u16), _V63(u16, $param), + _V64($param, u16), _V65(u16, $param), _V66($param, u16), _V67(u16, $param), + _V68($param, u16), _V69(u16, $param), _V6a($param, u16), _V6b(u16, $param), + _V6c($param, u16), _V6d(u16, $param), _V6e($param, u16), _V6f(u16, $param), + + _V70($param, u16), _V71(u16, $param), _V72($param, u16), _V73(u16, $param), + _V74($param, u16), _V75(u16, $param), _V76($param, u16), _V77(u16, $param), + _V78($param, u16), _V79(u16, $param), _V7a($param, u16), _V7b(u16, $param), + _V7c($param, u16), _V7d(u16, $param), _V7e($param, u16), _V7f(u16, $param), + + _V80($param, u16), _V81(u16, $param), _V82($param, u16), _V83(u16, $param), + _V84($param, u16), _V85(u16, $param), _V86($param, u16), _V87(u16, $param), + _V88($param, u16), _V89(u16, $param), _V8a($param, u16), _V8b(u16, $param), + _V8c($param, u16), _V8d(u16, $param), _V8e($param, u16), _V8f(u16, $param), + + _V90($param, u16), _V91(u16, $param), _V92($param, u16), _V93(u16, $param), + _V94($param, u16), _V95(u16, $param), _V96($param, u16), _V97(u16, $param), + _V98($param, u16), _V99(u16, $param), _V9a($param, u16), _V9b(u16, $param), + _V9c($param, u16), _V9d(u16, $param), _V9e($param, u16), _V9f(u16, $param), + + _Va0($param, u16), _Va1(u16, $param), _Va2($param, u16), _Va3(u16, $param), + _Va4($param, u16), _Va5(u16, $param), _Va6($param, u16), _Va7(u16, $param), + _Va8($param, u16), _Va9(u16, $param), _Vaa($param, u16), _Vab(u16, $param), + _Vac($param, u16), _Vad(u16, $param), _Vae($param, u16), _Vaf(u16, $param), + + _Vb0($param, u16), _Vb1(u16, $param), _Vb2($param, u16), _Vb3(u16, $param), + _Vb4($param, u16), _Vb5(u16, $param), _Vb6($param, u16), _Vb7(u16, $param), + _Vb8($param, u16), _Vb9(u16, $param), _Vba($param, u16), _Vbb(u16, $param), + _Vbc($param, u16), _Vbd(u16, $param), _Vbe($param, u16), _Vbf(u16, $param), + + _Vc0($param, u16), _Vc1(u16, $param), _Vc2($param, u16), _Vc3(u16, $param), + _Vc4($param, u16), _Vc5(u16, $param), _Vc6($param, u16), _Vc7(u16, $param), + _Vc8($param, u16), _Vc9(u16, $param), _Vca($param, u16), _Vcb(u16, $param), + _Vcc($param, u16), _Vcd(u16, $param), _Vce($param, u16), _Vcf(u16, $param), + + _Vd0($param, u16), _Vd1(u16, $param), _Vd2($param, u16), _Vd3(u16, $param), + _Vd4($param, u16), _Vd5(u16, $param), _Vd6($param, u16), _Vd7(u16, $param), + _Vd8($param, u16), _Vd9(u16, $param), _Vda($param, u16), _Vdb(u16, $param), + _Vdc($param, u16), _Vdd(u16, $param), _Vde($param, u16), _Vdf(u16, $param), + + _Ve0($param, u16), _Ve1(u16, $param), _Ve2($param, u16), _Ve3(u16, $param), + _Ve4($param, u16), _Ve5(u16, $param), _Ve6($param, u16), _Ve7(u16, $param), + _Ve8($param, u16), _Ve9(u16, $param), _Vea($param, u16), _Veb(u16, $param), + _Vec($param, u16), _Ved(u16, $param), _Vee($param, u16), _Vef(u16, $param), + + _Vf0($param, u16), _Vf1(u16, $param), _Vf2($param, u16), _Vf3(u16, $param), + _Vf4($param, u16), _Vf5(u16, $param), _Vf6($param, u16), _Vf7(u16, $param), + _Vf8($param, u16), _Vf9(u16, $param), _Vfa($param, u16), _Vfb(u16, $param), + _Vfc($param, u16), _Vfd(u16, $param), _Vfe($param, u16), _Vff(u16, $param), + } + }, + + ($name:ident, $param:ty) => { + #[derive(Debug)] + pub enum $name { + _V00($param, u16), _V01(u16, $param), _V02($param, u16), _V03(u16, $param), + _V04($param, u16), _V05(u16, $param), _V06($param, u16), _V07(u16, $param), + _V08($param, u16), _V09(u16, $param), _V0a($param, u16), _V0b(u16, $param), + _V0c($param, u16), _V0d(u16, $param), _V0e($param, u16), _V0f(u16, $param), + + _V10($param, u16), _V11(u16, $param), _V12($param, u16), _V13(u16, $param), + _V14($param, u16), _V15(u16, $param), _V16($param, u16), _V17(u16, $param), + _V18($param, u16), _V19(u16, $param), _V1a($param, u16), _V1b(u16, $param), + _V1c($param, u16), _V1d(u16, $param), _V1e($param, u16), _V1f(u16, $param), + + _V20($param, u16), _V21(u16, $param), _V22($param, u16), _V23(u16, $param), + _V24($param, u16), _V25(u16, $param), _V26($param, u16), _V27(u16, $param), + _V28($param, u16), _V29(u16, $param), _V2a($param, u16), _V2b(u16, $param), + _V2c($param, u16), _V2d(u16, $param), _V2e($param, u16), _V2f(u16, $param), + + _V30($param, u16), _V31(u16, $param), _V32($param, u16), _V33(u16, $param), + _V34($param, u16), _V35(u16, $param), _V36($param, u16), _V37(u16, $param), + _V38($param, u16), _V39(u16, $param), _V3a($param, u16), _V3b(u16, $param), + _V3c($param, u16), _V3d(u16, $param), _V3e($param, u16), _V3f(u16, $param), + + _V40($param, u16), _V41(u16, $param), _V42($param, u16), _V43(u16, $param), + _V44($param, u16), _V45(u16, $param), _V46($param, u16), _V47(u16, $param), + _V48($param, u16), _V49(u16, $param), _V4a($param, u16), _V4b(u16, $param), + _V4c($param, u16), _V4d(u16, $param), _V4e($param, u16), _V4f(u16, $param), + + _V50($param, u16), _V51(u16, $param), _V52($param, u16), _V53(u16, $param), + _V54($param, u16), _V55(u16, $param), _V56($param, u16), _V57(u16, $param), + _V58($param, u16), _V59(u16, $param), _V5a($param, u16), _V5b(u16, $param), + _V5c($param, u16), _V5d(u16, $param), _V5e($param, u16), _V5f(u16, $param), + + _V60($param, u16), _V61(u16, $param), _V62($param, u16), _V63(u16, $param), + _V64($param, u16), _V65(u16, $param), _V66($param, u16), _V67(u16, $param), + _V68($param, u16), _V69(u16, $param), _V6a($param, u16), _V6b(u16, $param), + _V6c($param, u16), _V6d(u16, $param), _V6e($param, u16), _V6f(u16, $param), + + _V70($param, u16), _V71(u16, $param), _V72($param, u16), _V73(u16, $param), + _V74($param, u16), _V75(u16, $param), _V76($param, u16), _V77(u16, $param), + _V78($param, u16), _V79(u16, $param), _V7a($param, u16), _V7b(u16, $param), + _V7c($param, u16), _V7d(u16, $param), _V7e($param, u16), _V7f(u16, $param), + + _V80($param, u16), _V81(u16, $param), _V82($param, u16), _V83(u16, $param), + _V84($param, u16), _V85(u16, $param), _V86($param, u16), _V87(u16, $param), + _V88($param, u16), _V89(u16, $param), _V8a($param, u16), _V8b(u16, $param), + _V8c($param, u16), _V8d(u16, $param), _V8e($param, u16), _V8f(u16, $param), + + _V90($param, u16), _V91(u16, $param), _V92($param, u16), _V93(u16, $param), + _V94($param, u16), _V95(u16, $param), _V96($param, u16), _V97(u16, $param), + _V98($param, u16), _V99(u16, $param), _V9a($param, u16), _V9b(u16, $param), + _V9c($param, u16), _V9d(u16, $param), _V9e($param, u16), _V9f(u16, $param), + + _Va0($param, u16), _Va1(u16, $param), _Va2($param, u16), _Va3(u16, $param), + _Va4($param, u16), _Va5(u16, $param), _Va6($param, u16), _Va7(u16, $param), + _Va8($param, u16), _Va9(u16, $param), _Vaa($param, u16), _Vab(u16, $param), + _Vac($param, u16), _Vad(u16, $param), _Vae($param, u16), _Vaf(u16, $param), + + _Vb0($param, u16), _Vb1(u16, $param), _Vb2($param, u16), _Vb3(u16, $param), + _Vb4($param, u16), _Vb5(u16, $param), _Vb6($param, u16), _Vb7(u16, $param), + _Vb8($param, u16), _Vb9(u16, $param), _Vba($param, u16), _Vbb(u16, $param), + _Vbc($param, u16), _Vbd(u16, $param), _Vbe($param, u16), _Vbf(u16, $param), + + _Vc0($param, u16), _Vc1(u16, $param), _Vc2($param, u16), _Vc3(u16, $param), + _Vc4($param, u16), _Vc5(u16, $param), _Vc6($param, u16), _Vc7(u16, $param), + _Vc8($param, u16), _Vc9(u16, $param), _Vca($param, u16), _Vcb(u16, $param), + _Vcc($param, u16), _Vcd(u16, $param), _Vce($param, u16), _Vcf(u16, $param), + + _Vd0($param, u16), _Vd1(u16, $param), _Vd2($param, u16), _Vd3(u16, $param), + _Vd4($param, u16), _Vd5(u16, $param), _Vd6($param, u16), _Vd7(u16, $param), + _Vd8($param, u16), _Vd9(u16, $param), _Vda($param, u16), _Vdb(u16, $param), + _Vdc($param, u16), _Vdd(u16, $param), _Vde($param, u16), _Vdf(u16, $param), + + _Ve0($param, u16), _Ve1(u16, $param), _Ve2($param, u16), _Ve3(u16, $param), + _Ve4($param, u16), _Ve5(u16, $param), _Ve6($param, u16), _Ve7(u16, $param), + _Ve8($param, u16), _Ve9(u16, $param), _Vea($param, u16), _Veb(u16, $param), + _Vec($param, u16), _Ved(u16, $param), _Vee($param, u16), _Vef(u16, $param), + + _Vf0($param, u16), _Vf1(u16, $param), _Vf2($param, u16), _Vf3(u16, $param), + _Vf4($param, u16), _Vf5(u16, $param), _Vf6($param, u16), _Vf7(u16, $param), + _Vf8($param, u16), _Vf9(u16, $param), _Vfa($param, u16), _Vfb(u16, $param), + _Vfc($param, u16), _Vfd(u16, $param), _Vfe($param, u16), _Vff(u16, $param), + } + } +} From ee633e821f68a1c44097d1f7549e1ada46c53b48 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 23 Jan 2020 14:33:55 -0500 Subject: [PATCH 09/10] Add `repr(no_niche)` to `UnsafeCell`. Fix #68303. --- src/libcore/cell.rs | 1 + src/libcore/lib.rs | 1 + src/test/ui/layout/unsafe-cell-hides-niche.rs | 32 +++++++++++++++++++ 3 files changed, 34 insertions(+) create mode 100644 src/test/ui/layout/unsafe-cell-hides-niche.rs diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index e7eecf7540ad7..8b8bda2e6b44f 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1475,6 +1475,7 @@ impl fmt::Display for RefMut<'_, T> { #[lang = "unsafe_cell"] #[stable(feature = "rust1", since = "1.0.0")] #[repr(transparent)] +#[cfg_attr(not(bootstrap), repr(no_niche))] // rust-lang/rust#68303. pub struct UnsafeCell { value: T, } diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index ce7ddffd82584..bc3509b9b6507 100644 --- a/src/libcore/lib.rs +++ b/src/libcore/lib.rs @@ -135,6 +135,7 @@ #![feature(const_caller_location)] #![cfg_attr(bootstrap, feature(slice_patterns))] #![feature(assoc_int_consts)] +#![cfg_attr(not(bootstrap), feature(no_niche))] // rust-lang/rust#68303 #[prelude_import] #[allow(unused)] diff --git a/src/test/ui/layout/unsafe-cell-hides-niche.rs b/src/test/ui/layout/unsafe-cell-hides-niche.rs new file mode 100644 index 0000000000000..4ca3f7a1aad94 --- /dev/null +++ b/src/test/ui/layout/unsafe-cell-hides-niche.rs @@ -0,0 +1,32 @@ +// For rust-lang/rust#68303: the contents of `UnsafeCell` cannot +// participate in the niche-optimization for enum discriminants. This +// test checks that an `Option>` has the same +// size in memory as an `Option>` (namely, 8 bytes). + +// run-pass + +#![feature(no_niche)] + +use std::cell::UnsafeCell; +use std::mem::size_of; +use std::num::NonZeroU32 as N32; + +struct Wrapper(T); + +#[repr(transparent)] +struct Transparent(T); + +#[repr(no_niche)] +struct NoNiche(T); + +fn main() { + assert_eq!(size_of::>>(), 8); + assert_eq!(size_of::>>(), 4); + assert_eq!(size_of::>>(), 8); + assert_eq!(size_of::>>(), 4); + assert_eq!(size_of::>>(), 8); + assert_eq!(size_of::>>(), 8); + + assert_eq!(size_of::>>(), 8); + assert_eq!(size_of::>>(), 8); +} From db319a8fb0126b84e0a0abbac83d4e1adeca6a95 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Wed, 22 Jan 2020 11:24:32 -0500 Subject: [PATCH 10/10] suggest adding space in accidental doc comments --- src/librustc_parse/parser/stmt.rs | 21 +++++++- src/test/ui/parser/doc-comment-in-stmt.rs | 20 ++++++++ src/test/ui/parser/doc-comment-in-stmt.stderr | 50 +++++++++++++++++++ 3 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/parser/doc-comment-in-stmt.rs create mode 100644 src/test/ui/parser/doc-comment-in-stmt.stderr diff --git a/src/librustc_parse/parser/stmt.rs b/src/librustc_parse/parser/stmt.rs index a94d8228bbe85..ae8f1e4db1b38 100644 --- a/src/librustc_parse/parser/stmt.rs +++ b/src/librustc_parse/parser/stmt.rs @@ -7,13 +7,13 @@ use crate::maybe_whole; use crate::DirectoryOwnership; use rustc_errors::{Applicability, PResult}; -use rustc_span::source_map::{respan, Span}; +use rustc_span::source_map::{respan, BytePos, Span}; use rustc_span::symbol::{kw, sym, Symbol}; use syntax::ast; use syntax::ast::{AttrStyle, AttrVec, Attribute, Mac, MacStmtStyle, VisibilityKind}; use syntax::ast::{Block, BlockCheckMode, Expr, ExprKind, Local, Stmt, StmtKind, DUMMY_NODE_ID}; use syntax::ptr::P; -use syntax::token; +use syntax::token::{self, TokenKind}; use syntax::util::classify; use std::mem; @@ -431,6 +431,23 @@ impl<'a> Parser<'a> { if let Err(mut e) = self.expect_one_of(&[], &[token::Semi, token::CloseDelim(token::Brace)]) { + if let TokenKind::DocComment(..) = self.token.kind { + if let Ok(snippet) = self.span_to_snippet(self.token.span) { + let sp = self.token.span; + let marker = &snippet[..3]; + let (comment_marker, doc_comment_marker) = marker.split_at(2); + + e.span_suggestion( + sp.with_hi(sp.lo() + BytePos(marker.len() as u32)), + &format!( + "add a space before `{}` to use a regular comment", + doc_comment_marker, + ), + format!("{} {}", comment_marker, doc_comment_marker), + Applicability::MaybeIncorrect, + ); + } + } e.emit(); self.recover_stmt(); // Don't complain about type errors in body tail after parse error (#57383). diff --git a/src/test/ui/parser/doc-comment-in-stmt.rs b/src/test/ui/parser/doc-comment-in-stmt.rs new file mode 100644 index 0000000000000..b02df13213f20 --- /dev/null +++ b/src/test/ui/parser/doc-comment-in-stmt.rs @@ -0,0 +1,20 @@ +fn foo() -> bool { + false + //!self.allow_ty_infer() + //~^ ERROR found doc comment +} + +fn bar() -> bool { + false + /*! bar */ //~ ERROR found doc comment +} + +fn baz() -> i32 { + 1 /** baz */ //~ ERROR found doc comment +} + +fn quux() -> i32 { + 2 /*! quux */ //~ ERROR found doc comment +} + +fn main() {} diff --git a/src/test/ui/parser/doc-comment-in-stmt.stderr b/src/test/ui/parser/doc-comment-in-stmt.stderr new file mode 100644 index 0000000000000..5d94d6fe69b59 --- /dev/null +++ b/src/test/ui/parser/doc-comment-in-stmt.stderr @@ -0,0 +1,50 @@ +error: expected one of `.`, `;`, `?`, `}`, or an operator, found doc comment `//!self.allow_ty_infer()` + --> $DIR/doc-comment-in-stmt.rs:3:5 + | +LL | false + | - expected one of `.`, `;`, `?`, `}`, or an operator +LL | //!self.allow_ty_infer() + | ^^^^^^^^^^^^^^^^^^^^^^^^ unexpected token + | +help: add a space before `!` to use a regular comment + | +LL | // !self.allow_ty_infer() + | ^^^^ + +error: expected one of `.`, `;`, `?`, `}`, or an operator, found doc comment `/*! bar */` + --> $DIR/doc-comment-in-stmt.rs:9:5 + | +LL | false + | - expected one of `.`, `;`, `?`, `}`, or an operator +LL | /*! bar */ + | ^^^^^^^^^^ unexpected token + | +help: add a space before `!` to use a regular comment + | +LL | /* ! bar */ + | ^^^^ + +error: expected one of `.`, `;`, `?`, `}`, or an operator, found doc comment `/** baz */` + --> $DIR/doc-comment-in-stmt.rs:13:7 + | +LL | 1 /** baz */ + | ^^^^^^^^^^ expected one of `.`, `;`, `?`, `}`, or an operator + | +help: add a space before `*` to use a regular comment + | +LL | 1 /* * baz */ + | ^^^^ + +error: expected one of `.`, `;`, `?`, `}`, or an operator, found doc comment `/*! quux */` + --> $DIR/doc-comment-in-stmt.rs:17:7 + | +LL | 2 /*! quux */ + | ^^^^^^^^^^^ expected one of `.`, `;`, `?`, `}`, or an operator + | +help: add a space before `!` to use a regular comment + | +LL | 2 /* ! quux */ + | ^^^^ + +error: aborting due to 4 previous errors +