From 13ffbee1735c7329d6e7b3521712996c914c4ef9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:07:58 +0100 Subject: [PATCH 01/54] Add MaybeUninit::read_uninitialized Also remove a no-longer accurate comments --- src/libcore/mem.rs | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 43afc9a522a30..3955839ac3625 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1185,6 +1185,61 @@ impl MaybeUninit { ManuallyDrop::into_inner(self.value) } + /// Reads the value from the `MaybeUninit` container. The resulting `T` is subject + /// to the usual drop handling. + /// + /// # Unsafety + /// + /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// state. Calling this when the content is not yet fully initialized causes undefined + /// behavior. + /// + /// Moreover, this leaves a copy of the same data behind in the `MaybeUninit`. When using + /// multiple copies of the data (by calling `read_initialized` multiple times, or first + /// calling `read_initialized` and then [`into_initialized`]), it is your responsibility + /// to ensure that that data may indeed be duplicated. + /// + /// # Examples + /// + /// Correct usage of this method: + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::::uninitialized(); + /// x.set(13); + /// let x1 = unsafe { x.read_initialized() }; + /// // `u32` is `Copy`, so we may read multiple times. + /// let x2 = unsafe { x.read_initialized() }; + /// + /// let mut x = MaybeUninit::>>::uninitialized(); + /// x.set(None); + /// let x1 = unsafe { x.read_initialized() }; + /// // Duplicating a `None` value is okay, so we may read multiple times. + /// let x2 = unsafe { x.read_initialized() }; + /// ``` + /// + /// *Incorrect* usafe of this method: + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>>::uninitialized(); + /// x.set(Some(vec![0,1,2])); + /// let x1 = unsafe { x.read_initialized() }; + /// let x2 = unsafe { x.read_initialized() }; + /// // We now created two copies of the same vector, leading to a double-free when + /// // they both get dropped! + /// ``` + #[unstable(feature = "maybe_uninit", issue = "53491")] + #[inline(always)] + pub unsafe fn read_initialized(&self) -> T { + intrinsics::panic_if_uninhabited::(); + self.as_ptr().read() + } + /// Gets a reference to the contained value. /// /// # Unsafety From dc570fb3f26b45d101440bb4fe57121ca06884d3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:11:37 +0100 Subject: [PATCH 02/54] also add examples to MaybeUninit::into_initialized --- src/libcore/mem.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 3955839ac3625..42f30c1dd6d72 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1178,6 +1178,31 @@ impl MaybeUninit { /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state. Calling this when the content is not yet fully initialized causes undefined /// behavior. + /// + /// # Examples + /// + /// Correct usage of this method: + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::::uninitialized(); + /// x.set(true); + /// let x_init = unsafe { x.into_initialized() }; + /// assert_eq!(x_init, true); + /// ``` + /// + /// *Incorrect* usage of this method: + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let x = MaybeUninit::>::uninitialized(); + /// let x_init = unsafe { x.into_initialized() }; + /// // `x` had not been initialized yet, so this last line causes undefined behavior. + /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub unsafe fn into_initialized(self) -> T { @@ -1212,15 +1237,17 @@ impl MaybeUninit { /// let x1 = unsafe { x.read_initialized() }; /// // `u32` is `Copy`, so we may read multiple times. /// let x2 = unsafe { x.read_initialized() }; + /// assert_eq!(x1, x2); /// /// let mut x = MaybeUninit::>>::uninitialized(); /// x.set(None); /// let x1 = unsafe { x.read_initialized() }; /// // Duplicating a `None` value is okay, so we may read multiple times. /// let x2 = unsafe { x.read_initialized() }; + /// assert_eq!(x1, x2); /// ``` /// - /// *Incorrect* usafe of this method: + /// *Incorrect* usage of this method: /// /// ```rust,no_run /// #![feature(maybe_uninit)] From 10f511daa01d98e4c8f524cbdf38e9dd6c3ea9e3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:17:04 +0100 Subject: [PATCH 03/54] misc tweaks --- src/libcore/mem.rs | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 42f30c1dd6d72..556f8a9e8a2af 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1056,12 +1056,22 @@ impl DerefMut for ManuallyDrop { /// This is exploited by the compiler for various optimizations, such as eliding /// run-time checks and optimizing `enum` layout. /// -/// Not initializing memory at all (instead of zero-initializing it) causes the same -/// issue: after all, the initial value of the variable might just happen to be -/// one that violates the invariant. Moreover, uninitialized memory is special -/// in that the compiler knows that it does not have a fixed value. This makes -/// it undefined behavior to have uninitialized data in a variable even if that -/// variable has otherwise no restrictions about which values are valid: +/// Similarly, entirely uninitialized memory may have any content, while a `bool` must +/// always be `true` or `false`. Hence, creating an uninitialized `bool` is undefined behavior: +/// +/// ```rust,no_run +/// #![feature(maybe_uninit)] +/// use std::mem::{self, MaybeUninit}; +/// +/// let b: bool = unsafe { mem::uninitialized() }; // undefined behavior! +/// // equivalent code with `MaybeUninit` +/// let b: bool = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! +/// ``` +/// +/// Moreover, uninitialized memory is special in that the compiler knows that +/// it does not have a fixed value. This makes it undefined behavior to have +/// uninitialized data in a variable even if that variable has integer type, +/// which otherwise can hold any bit pattern: /// /// ```rust,no_run /// #![feature(maybe_uninit)] @@ -1074,8 +1084,8 @@ impl DerefMut for ManuallyDrop { /// (Notice that the rules around uninitialized integers are not finalized yet, but /// until they are, it is advisable to avoid them.) /// -/// `MaybeUninit` serves to enable unsafe code to deal with uninitialized data: -/// it is a signal to the compiler indicating that the data here might *not* +/// `MaybeUninit` serves to enable unsafe code to deal with uninitialized data. +/// It is a signal to the compiler indicating that the data here might *not* /// be initialized: /// /// ```rust @@ -1092,11 +1102,11 @@ impl DerefMut for ManuallyDrop { /// let x = unsafe { x.into_initialized() }; /// ``` /// -/// The compiler then knows to not optimize this code. +/// The compiler then knows to not make any incorrect assumptions or optimizations on this code. // FIXME before stabilizing, explain how to initialize a struct field-by-field. #[allow(missing_debug_implementations)] #[unstable(feature = "maybe_uninit", issue = "53491")] -// NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::{uninitialized,zeroed}` +// NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::uninitialized` pub union MaybeUninit { uninit: (), value: ManuallyDrop, @@ -1154,7 +1164,7 @@ impl MaybeUninit { } /// Gets a pointer to the contained value. Reading from this pointer or turning it - /// into a reference will be undefined behavior unless the `MaybeUninit` is initialized. + /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_ptr(&self) -> *const T { @@ -1162,7 +1172,7 @@ impl MaybeUninit { } /// Gets a mutable pointer to the contained value. Reading from this pointer or turning it - /// into a reference will be undefined behavior unless the `MaybeUninit` is initialized. + /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_mut_ptr(&mut self) -> *mut T { From 48aa59e74d6a2b3fea5162eaed902798dc0f95f8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:23:31 +0100 Subject: [PATCH 04/54] examples for as[_mut]_ptr --- src/libcore/mem.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 556f8a9e8a2af..d586f45534ea0 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1165,6 +1165,32 @@ impl MaybeUninit { /// Gets a pointer to the contained value. Reading from this pointer or turning it /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. + /// + /// # Examples + /// + /// Correct usage of this method: + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>::uninitialized(); + /// x.set(vec![0,1,2]); + /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. + /// let x_vec = unsafe { &*x.as_ptr() }; + /// assert_eq!(x_vec.len(), 3); + /// ``` + /// + /// *Incorrect* usage of this method: + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let x = MaybeUninit::>::uninitialized(); + /// let x_vec = unsafe { &*x.as_ptr() }; + /// // We have created a reference to an uninitialized vector! This is undefined behavior. + /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_ptr(&self) -> *const T { @@ -1173,6 +1199,33 @@ impl MaybeUninit { /// Gets a mutable pointer to the contained value. Reading from this pointer or turning it /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. + /// + /// # Examples + /// + /// Correct usage of this method: + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>::uninitialized(); + /// x.set(vec![0,1,2]); + /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. + /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; + /// x_vec.push(3); + /// assert_eq!(x_vec.len(), 4); + /// ``` + /// + /// *Incorrect* usage of this method: + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let mut x = MaybeUninit::>::uninitialized(); + /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; + /// // We have created a reference to an uninitialized vector! This is undefined behavior. + /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_mut_ptr(&mut self) -> *mut T { From 084ee7a875cb02c0f0c0c6d1ffaff81d69cec74e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:35:18 +0100 Subject: [PATCH 05/54] examples for MaybeUninit::zeroed --- src/libcore/mem.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index d586f45534ea0..8f6798e0f6e61 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1141,6 +1141,35 @@ impl MaybeUninit { /// /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. + /// + /// # Example + /// + /// Correct usage of this method: initializing a struct with zero, where all + /// fields of the struct can hold 0 as a valid value. + /// + /// ```rust + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// let x = MaybeUninit::<(u8, bool)>::zeroed(); + /// let x = unsafe { x.into_initialized() }; + /// assert_eq!(x, (0, false)); + /// ``` + /// + /// *Incorrect* usage of this method: initializing a struct with zero, where some fields + /// cannot hold 0 as a valid value. + /// + /// ```rust,no_run + /// #![feature(maybe_uninit)] + /// use std::mem::MaybeUninit; + /// + /// enum NotZero { One = 1, Two = 2 }; + /// + /// let x = MaybeUninit::<(u8, NotZero)>::zeroed(); + /// let x = unsafe { x.into_initialized() }; + /// // We create a `NotZero` (inside a pair) that does not have a valid discriminant. + /// // This is undefined behavior. + /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline] pub fn zeroed() -> MaybeUninit { From d10366fe27bf1a1e6ab67076bdc268f486abeb88 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:47:30 +0100 Subject: [PATCH 06/54] avoid unnecessary use of MaybeUninit::get_ref, and expand comment on the others --- src/libcore/fmt/float.rs | 4 ++++ src/libcore/ptr.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcore/fmt/float.rs b/src/libcore/fmt/float.rs index 20c626cef1b16..edeb65afd67b2 100644 --- a/src/libcore/fmt/float.rs +++ b/src/libcore/fmt/float.rs @@ -15,6 +15,7 @@ fn float_to_decimal_common_exact(fmt: &mut Formatter, num: &T, // FIXME(#53491): Technically, this is calling `get_mut` on an uninitialized // `MaybeUninit` (here and elsewhere in this file). Revisit this once // we decided whether that is valid or not. + // Using `freeze` is *not enough*; `flt2dec::Part` is an enum! let formatted = flt2dec::to_exact_fixed_str(flt2dec::strategy::grisu::format_exact, *num, sign, precision, false, buf.get_mut(), parts.get_mut()); @@ -33,6 +34,7 @@ fn float_to_decimal_common_shortest(fmt: &mut Formatter, num: &T, // enough for f32 and f64 let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninitialized(); let mut parts = MaybeUninit::<[flt2dec::Part; 4]>::uninitialized(); + // FIXME(#53491) let formatted = flt2dec::to_shortest_str(flt2dec::strategy::grisu::format_shortest, *num, sign, precision, false, buf.get_mut(), parts.get_mut()); @@ -71,6 +73,7 @@ fn float_to_exponential_common_exact(fmt: &mut Formatter, num: &T, unsafe { let mut buf = MaybeUninit::<[u8; 1024]>::uninitialized(); // enough for f32 and f64 let mut parts = MaybeUninit::<[flt2dec::Part; 6]>::uninitialized(); + // FIXME(#53491) let formatted = flt2dec::to_exact_exp_str(flt2dec::strategy::grisu::format_exact, *num, sign, precision, upper, buf.get_mut(), parts.get_mut()); @@ -90,6 +93,7 @@ fn float_to_exponential_common_shortest(fmt: &mut Formatter, // enough for f32 and f64 let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninitialized(); let mut parts = MaybeUninit::<[flt2dec::Part; 6]>::uninitialized(); + // FIXME(#53491) let formatted = flt2dec::to_shortest_exp_str(flt2dec::strategy::grisu::format_shortest, *num, sign, (0, 0), upper, buf.get_mut(), parts.get_mut()); diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 866c8d0896b3c..a2599ae834c69 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -301,7 +301,7 @@ pub unsafe fn swap(x: *mut T, y: *mut T) { // Perform the swap copy_nonoverlapping(x, tmp.as_mut_ptr(), 1); copy(y, x, 1); // `x` and `y` may overlap - copy_nonoverlapping(tmp.get_ref(), y, 1); + copy_nonoverlapping(tmp.as_ptr(), y, 1); } /// Swaps `count * size_of::()` bytes between the two regions of memory From aa4a9b0827f09efa8a06d99df6cae07b21e6729c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Feb 2019 22:58:53 +0100 Subject: [PATCH 07/54] make MaybeUninit Copy --- src/libcore/mem.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 8f6798e0f6e61..296f15d830302 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1106,12 +1106,22 @@ impl DerefMut for ManuallyDrop { // FIXME before stabilizing, explain how to initialize a struct field-by-field. #[allow(missing_debug_implementations)] #[unstable(feature = "maybe_uninit", issue = "53491")] +#[derive(Copy)] // NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::uninitialized` pub union MaybeUninit { uninit: (), value: ManuallyDrop, } +#[unstable(feature = "maybe_uninit", issue = "53491")] +impl Clone for MaybeUninit { + #[inline(always)] + fn clone(&self) -> Self { + // Not calling T::clone(), we cannot know if we are initialized enough for that. + *self + } +} + impl MaybeUninit { /// Create a new `MaybeUninit` initialized with the given value. /// From 53c027588263abf69bf0124f15eb6d261cd43316 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 23 Feb 2019 16:17:07 +0100 Subject: [PATCH 08/54] Apply suggestions from code review Co-Authored-By: RalfJung --- src/libcore/mem.rs | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 296f15d830302..46ba523c7722b 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1064,13 +1064,13 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let b: bool = unsafe { mem::uninitialized() }; // undefined behavior! -/// // equivalent code with `MaybeUninit` +/// // The equivalent code with `MaybeUninit`: /// let b: bool = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! /// ``` /// /// Moreover, uninitialized memory is special in that the compiler knows that /// it does not have a fixed value. This makes it undefined behavior to have -/// uninitialized data in a variable even if that variable has integer type, +/// uninitialized data in a variable even if that variable has an integer type, /// which otherwise can hold any bit pattern: /// /// ```rust,no_run @@ -1084,7 +1084,7 @@ impl DerefMut for ManuallyDrop { /// (Notice that the rules around uninitialized integers are not finalized yet, but /// until they are, it is advisable to avoid them.) /// -/// `MaybeUninit` serves to enable unsafe code to deal with uninitialized data. +/// `MaybeUninit` serves to enable unsafe code to deal with uninitialized data. /// It is a signal to the compiler indicating that the data here might *not* /// be initialized: /// @@ -1107,7 +1107,7 @@ impl DerefMut for ManuallyDrop { #[allow(missing_debug_implementations)] #[unstable(feature = "maybe_uninit", issue = "53491")] #[derive(Copy)] -// NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::uninitialized` +// NOTE after stabilizing `MaybeUninit` proceed to deprecate `mem::uninitialized`. pub union MaybeUninit { uninit: (), value: ManuallyDrop, @@ -1123,7 +1123,7 @@ impl Clone for MaybeUninit { } impl MaybeUninit { - /// Create a new `MaybeUninit` initialized with the given value. + /// Create a new `MaybeUninit` initialized with the given value. /// /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. @@ -1149,13 +1149,13 @@ impl MaybeUninit { /// but `MaybeUninit<&'static i32>::zeroed()` is not because references must not /// be null. /// - /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. + /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. /// /// # Example /// - /// Correct usage of this method: initializing a struct with zero, where all - /// fields of the struct can hold 0 as a valid value. + /// Correct usage of this function: initializing a struct with zero, where all + /// fields of the struct can hold the bit-pattern 0 as a valid value. /// /// ```rust /// #![feature(maybe_uninit)] @@ -1166,7 +1166,7 @@ impl MaybeUninit { /// assert_eq!(x, (0, false)); /// ``` /// - /// *Incorrect* usage of this method: initializing a struct with zero, where some fields + /// *Incorrect* usage of this function: initializing a struct with zero, where some fields /// cannot hold 0 as a valid value. /// /// ```rust,no_run @@ -1177,7 +1177,7 @@ impl MaybeUninit { /// /// let x = MaybeUninit::<(u8, NotZero)>::zeroed(); /// let x = unsafe { x.into_initialized() }; - /// // We create a `NotZero` (inside a pair) that does not have a valid discriminant. + /// // Inside a pair, we create a `NotZero` that does not have a valid discriminant. /// // This is undefined behavior. /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] @@ -1203,7 +1203,7 @@ impl MaybeUninit { } /// Gets a pointer to the contained value. Reading from this pointer or turning it - /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. + /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. /// /// # Examples /// @@ -1237,7 +1237,7 @@ impl MaybeUninit { } /// Gets a mutable pointer to the contained value. Reading from this pointer or turning it - /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. + /// into a reference is undefined behavior unless the `MaybeUninit` is initialized. /// /// # Examples /// @@ -1249,7 +1249,8 @@ impl MaybeUninit { /// /// let mut x = MaybeUninit::>::uninitialized(); /// x.set(vec![0,1,2]); - /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. + /// // Create a reference into the `MaybeUninit>`. + /// // This is okay because we initialized it. /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; /// x_vec.push(3); /// assert_eq!(x_vec.len(), 4); @@ -1303,7 +1304,7 @@ impl MaybeUninit { /// /// let x = MaybeUninit::>::uninitialized(); /// let x_init = unsafe { x.into_initialized() }; - /// // `x` had not been initialized yet, so this last line causes undefined behavior. + /// // `x` had not been initialized yet, so this last line caused undefined behavior. /// ``` #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] @@ -1312,16 +1313,16 @@ impl MaybeUninit { ManuallyDrop::into_inner(self.value) } - /// Reads the value from the `MaybeUninit` container. The resulting `T` is subject + /// Reads the value from the `MaybeUninit` container. The resulting `T` is subject /// to the usual drop handling. /// - /// # Unsafety + /// # Safety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state. Calling this when the content is not yet fully initialized causes undefined /// behavior. /// - /// Moreover, this leaves a copy of the same data behind in the `MaybeUninit`. When using + /// Moreover, this leaves a copy of the same data behind in the `MaybeUninit`. When using /// multiple copies of the data (by calling `read_initialized` multiple times, or first /// calling `read_initialized` and then [`into_initialized`]), it is your responsibility /// to ensure that that data may indeed be duplicated. From ac2284b80b5bf50702af2289c2ac4f04efa573ec Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Feb 2019 16:18:50 +0100 Subject: [PATCH 09/54] expand type name --- src/libcore/mem.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 46ba523c7722b..b354815a3a142 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1035,7 +1035,7 @@ impl DerefMut for ManuallyDrop { } } -/// A newtype to construct uninitialized instances of `T`. +/// A wrapper to construct uninitialized instances of `T`. /// /// The compiler, in general, assumes that variables are properly initialized /// at their respective type. For example, a variable of reference type must @@ -1049,7 +1049,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let x: &i32 = unsafe { mem::zeroed() }; // undefined behavior! -/// // equivalent code with `MaybeUninit` +/// // equivalent code with `MaybeUninit<&i32>` /// let x: &i32 = unsafe { MaybeUninit::zeroed().into_initialized() }; // undefined behavior! /// ``` /// @@ -1064,7 +1064,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let b: bool = unsafe { mem::uninitialized() }; // undefined behavior! -/// // The equivalent code with `MaybeUninit`: +/// // The equivalent code with `MaybeUninit`: /// let b: bool = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! /// ``` /// @@ -1078,7 +1078,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let x: i32 = unsafe { mem::uninitialized() }; // undefined behavior! -/// // equivalent code with `MaybeUninit` +/// // equivalent code with `MaybeUninit` /// let x: i32 = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! /// ``` /// (Notice that the rules around uninitialized integers are not finalized yet, but @@ -1093,7 +1093,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::MaybeUninit; /// /// // Create an explicitly uninitialized reference. The compiler knows that data inside -/// // a `MaybeUninit` may be invalid, and hence this is not UB: +/// // a `MaybeUninit` may be invalid, and hence this is not UB: /// let mut x = MaybeUninit::<&i32>::uninitialized(); /// // Set it to a valid value. /// x.set(&0); @@ -1125,7 +1125,7 @@ impl Clone for MaybeUninit { impl MaybeUninit { /// Create a new `MaybeUninit` initialized with the given value. /// - /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. + /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] @@ -1133,9 +1133,9 @@ impl MaybeUninit { MaybeUninit { value: ManuallyDrop::new(val) } } - /// Creates a new `MaybeUninit` in an uninitialized state. + /// Creates a new `MaybeUninit` in an uninitialized state. /// - /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. + /// Note that dropping a `MaybeUninit` will never call `T`'s drop code. /// It is your responsibility to make sure `T` gets dropped if it got initialized. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] @@ -1143,7 +1143,7 @@ impl MaybeUninit { MaybeUninit { uninit: () } } - /// Creates a new `MaybeUninit` in an uninitialized state, with the memory being + /// Creates a new `MaybeUninit` in an uninitialized state, with the memory being /// filled with `0` bytes. It depends on `T` whether that already makes for /// proper initialization. For example, `MaybeUninit::zeroed()` is initialized, /// but `MaybeUninit<&'static i32>::zeroed()` is not because references must not @@ -1190,9 +1190,9 @@ impl MaybeUninit { u } - /// Sets the value of the `MaybeUninit`. This overwrites any previous value without dropping it. - /// For your convenience, this also returns a mutable reference to the (now safely initialized) - /// contents of `self`. + /// Sets the value of the `MaybeUninit`. This overwrites any previous value + /// without dropping it. For your convenience, this also returns a mutable + /// reference to the (now safely initialized) contents of `self`. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn set(&mut self, val: T) -> &mut T { @@ -1215,7 +1215,7 @@ impl MaybeUninit { /// /// let mut x = MaybeUninit::>::uninitialized(); /// x.set(vec![0,1,2]); - /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. + /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. /// let x_vec = unsafe { &*x.as_ptr() }; /// assert_eq!(x_vec.len(), 3); /// ``` @@ -1272,13 +1272,13 @@ impl MaybeUninit { unsafe { &mut *self.value as *mut T } } - /// Extracts the value from the `MaybeUninit` container. This is a great way + /// Extracts the value from the `MaybeUninit` container. This is a great way /// to ensure that the data will get dropped, because the resulting `T` is /// subject to the usual drop handling. /// /// # Unsafety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state. Calling this when the content is not yet fully initialized causes undefined /// behavior. /// @@ -1374,7 +1374,7 @@ impl MaybeUninit { /// /// # Unsafety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state. Calling this when the content is not yet fully initialized causes undefined /// behavior. #[unstable(feature = "maybe_uninit_ref", issue = "53491")] @@ -1387,7 +1387,7 @@ impl MaybeUninit { /// /// # Unsafety /// - /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized + /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state. Calling this when the content is not yet fully initialized causes undefined /// behavior. // FIXME(#53491): We currently rely on the above being incorrect, i.e., we have references From 8ce9b8667ac831821729f11b1e7c685af1e9e21c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Feb 2019 16:20:00 +0100 Subject: [PATCH 10/54] fix link --- src/libcore/mem.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index b354815a3a142..30fa904101d27 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1327,6 +1327,8 @@ impl MaybeUninit { /// calling `read_initialized` and then [`into_initialized`]), it is your responsibility /// to ensure that that data may indeed be duplicated. /// + /// [`into_initialized`]: #method.into_initialized + /// /// # Examples /// /// Correct usage of this method: From a5e2d0c4e5f4afe1bd52ed0ebe0be03890d3af62 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 24 Feb 2019 12:25:46 +0100 Subject: [PATCH 11/54] remark that the rules are unfinished --- src/libcore/mem.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 30fa904101d27..d4d51f8eeb76e 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1230,6 +1230,8 @@ impl MaybeUninit { /// let x_vec = unsafe { &*x.as_ptr() }; /// // We have created a reference to an uninitialized vector! This is undefined behavior. /// ``` + /// (Notice that the rules around referenced to uninitialized data are not finalized yet, but + /// until they are, it is advisable to avoid them.) #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_ptr(&self) -> *const T { @@ -1266,6 +1268,8 @@ impl MaybeUninit { /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; /// // We have created a reference to an uninitialized vector! This is undefined behavior. /// ``` + /// (Notice that the rules around referenced to uninitialized data are not finalized yet, but + /// until they are, it is advisable to avoid them.) #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] pub fn as_mut_ptr(&mut self) -> *mut T { From be8d728f3cb1cb79a630c6e6aba6df923dd3e999 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 24 Feb 2019 16:58:04 +0100 Subject: [PATCH 12/54] show how to set with ptr::write --- src/libcore/mem.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index d4d51f8eeb76e..967a36f6f1c39 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1191,7 +1191,8 @@ impl MaybeUninit { } /// Sets the value of the `MaybeUninit`. This overwrites any previous value - /// without dropping it. For your convenience, this also returns a mutable + /// without dropping it, so be careful not to use this twice unless you want to + /// skip running the destructor. For your convenience, this also returns a mutable /// reference to the (now safely initialized) contents of `self`. #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] @@ -1214,7 +1215,7 @@ impl MaybeUninit { /// use std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::>::uninitialized(); - /// x.set(vec![0,1,2]); + /// unsafe { x.as_mut_ptr().write(vec![0,1,2]); } /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. /// let x_vec = unsafe { &*x.as_ptr() }; /// assert_eq!(x_vec.len(), 3); @@ -1250,7 +1251,7 @@ impl MaybeUninit { /// use std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::>::uninitialized(); - /// x.set(vec![0,1,2]); + /// unsafe { x.as_mut_ptr().write(vec![0,1,2]); } /// // Create a reference into the `MaybeUninit>`. /// // This is okay because we initialized it. /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; @@ -1295,7 +1296,7 @@ impl MaybeUninit { /// use std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::::uninitialized(); - /// x.set(true); + /// unsafe { x.as_mut_ptr().write(true); } /// let x_init = unsafe { x.into_initialized() }; /// assert_eq!(x_init, true); /// ``` From 6d32e5ae1a7a6cf7bc28f41d4c99ed81ac6113aa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 24 Feb 2019 20:34:24 +0100 Subject: [PATCH 13/54] prefer into_initialized over read_initialited --- src/libcore/mem.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 967a36f6f1c39..4b5056c5adffa 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1321,6 +1321,9 @@ impl MaybeUninit { /// Reads the value from the `MaybeUninit` container. The resulting `T` is subject /// to the usual drop handling. /// + /// Whenever possible, it is preferrable to use [`into_initialized`] instead, which + /// prevents duplicating the content of the `MaybeUninit`. + /// /// # Safety /// /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized From b11502fbc0cfc251c1b96743562761c9a451d84a Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 26 Feb 2019 22:42:50 +0300 Subject: [PATCH 14/54] Mention `unwind(aborts)` in diagnostics for `#[unwind]` Simplify input validation for `#[unwind]`, add tests --- src/libsyntax/attr/builtin.rs | 48 +++++++------------ src/libsyntax/feature_gate.rs | 2 +- src/test/ui/malformed/malformed-unwind-1.rs | 11 +++++ .../ui/malformed/malformed-unwind-1.stderr | 14 ++++++ src/test/ui/malformed/malformed-unwind-2.rs | 11 +++++ .../ui/malformed/malformed-unwind-2.stderr | 15 ++++++ 6 files changed, 69 insertions(+), 32 deletions(-) create mode 100644 src/test/ui/malformed/malformed-unwind-1.rs create mode 100644 src/test/ui/malformed/malformed-unwind-1.stderr create mode 100644 src/test/ui/malformed/malformed-unwind-2.rs create mode 100644 src/test/ui/malformed/malformed-unwind-2.stderr diff --git a/src/libsyntax/attr/builtin.rs b/src/libsyntax/attr/builtin.rs index e84adc01cf04a..f7a000935caf0 100644 --- a/src/libsyntax/attr/builtin.rs +++ b/src/libsyntax/attr/builtin.rs @@ -7,7 +7,7 @@ use crate::parse::ParseSess; use errors::{Applicability, Handler}; use syntax_pos::{symbol::Symbol, Span}; -use super::{list_contains_name, mark_used, MetaItemKind}; +use super::{mark_used, MetaItemKind}; enum AttrError { MultipleItem(Name), @@ -79,40 +79,26 @@ pub enum UnwindAttr { /// Determine what `#[unwind]` attribute is present in `attrs`, if any. pub fn find_unwind_attr(diagnostic: Option<&Handler>, attrs: &[Attribute]) -> Option { - let syntax_error = |attr: &Attribute| { - mark_used(attr); - diagnostic.map(|d| { - span_err!(d, attr.span, E0633, "malformed `#[unwind]` attribute"); - }); - None - }; - attrs.iter().fold(None, |ia, attr| { - if attr.path != "unwind" { - return ia; - } - let meta = match attr.meta() { - Some(meta) => meta.node, - None => return ia, - }; - match meta { - MetaItemKind::Word => { - syntax_error(attr) - } - MetaItemKind::List(ref items) => { - mark_used(attr); - if items.len() != 1 { - syntax_error(attr) - } else if list_contains_name(&items[..], "allowed") { - Some(UnwindAttr::Allowed) - } else if list_contains_name(&items[..], "aborts") { - Some(UnwindAttr::Aborts) - } else { - syntax_error(attr) + if attr.check_name("unwind") { + if let Some(meta) = attr.meta() { + if let MetaItemKind::List(items) = meta.node { + if items.len() == 1 { + if items[0].check_name("allowed") { + return Some(UnwindAttr::Allowed); + } else if items[0].check_name("aborts") { + return Some(UnwindAttr::Aborts); + } + } + + diagnostic.map(|d| { + span_err!(d, attr.span, E0633, "malformed `#[unwind]` attribute"); + }); } } - _ => ia, } + + ia }) } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index cc1953e69d4ca..93c6de274eeea 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -1173,7 +1173,7 @@ pub const BUILTIN_ATTRIBUTES: &[(&str, AttributeType, AttributeTemplate, Attribu "dropck_eyepatch", "may_dangle has unstable semantics and may be removed in the future", cfg_fn!(dropck_eyepatch))), - ("unwind", Whitelisted, template!(List: "allowed"), Gated(Stability::Unstable, + ("unwind", Whitelisted, template!(List: "allowed|aborts"), Gated(Stability::Unstable, "unwind_attributes", "#[unwind] is experimental", cfg_fn!(unwind_attributes))), diff --git a/src/test/ui/malformed/malformed-unwind-1.rs b/src/test/ui/malformed/malformed-unwind-1.rs new file mode 100644 index 0000000000000..e34c288c027c2 --- /dev/null +++ b/src/test/ui/malformed/malformed-unwind-1.rs @@ -0,0 +1,11 @@ +#![feature(unwind_attributes)] + +#[unwind] +//~^ ERROR attribute must be of the form +extern "C" fn f1() {} + +#[unwind = ""] +//~^ ERROR attribute must be of the form +extern "C" fn f2() {} + +fn main() {} diff --git a/src/test/ui/malformed/malformed-unwind-1.stderr b/src/test/ui/malformed/malformed-unwind-1.stderr new file mode 100644 index 0000000000000..852136eed91bd --- /dev/null +++ b/src/test/ui/malformed/malformed-unwind-1.stderr @@ -0,0 +1,14 @@ +error: attribute must be of the form `#[unwind(allowed|aborts)]` + --> $DIR/malformed-unwind-1.rs:3:1 + | +LL | #[unwind] + | ^^^^^^^^^ + +error: attribute must be of the form `#[unwind(allowed|aborts)]` + --> $DIR/malformed-unwind-1.rs:7:1 + | +LL | #[unwind = ""] + | ^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/malformed/malformed-unwind-2.rs b/src/test/ui/malformed/malformed-unwind-2.rs new file mode 100644 index 0000000000000..d4955b4330930 --- /dev/null +++ b/src/test/ui/malformed/malformed-unwind-2.rs @@ -0,0 +1,11 @@ +#![feature(unwind_attributes)] + +#[unwind(allowed, aborts)] +//~^ ERROR malformed `#[unwind]` attribute +extern "C" fn f1() {} + +#[unwind(unsupported)] +//~^ ERROR malformed `#[unwind]` attribute +extern "C" fn f2() {} + +fn main() {} diff --git a/src/test/ui/malformed/malformed-unwind-2.stderr b/src/test/ui/malformed/malformed-unwind-2.stderr new file mode 100644 index 0000000000000..88fc4e00a2fd3 --- /dev/null +++ b/src/test/ui/malformed/malformed-unwind-2.stderr @@ -0,0 +1,15 @@ +error[E0633]: malformed `#[unwind]` attribute + --> $DIR/malformed-unwind-2.rs:3:1 + | +LL | #[unwind(allowed, aborts)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0633]: malformed `#[unwind]` attribute + --> $DIR/malformed-unwind-2.rs:7:1 + | +LL | #[unwind(unsupported)] + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0633`. From dc00a8adee77f04a450e2f02be97b4e308af863e Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 27 Feb 2019 09:36:49 +0300 Subject: [PATCH 15/54] Validate `#[unwind]` syntax regardless of platform-specific panic strategy --- src/librustc_mir/build/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 19507c900da64..39c84b95f8f5d 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -581,6 +581,10 @@ fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, // Not callable from C, so we can safely unwind through these if abi == Abi::Rust || abi == Abi::RustCall { return false; } + // Validate `#[unwind]` syntax regardless of platform-specific panic strategy + let attrs = &tcx.get_attrs(fn_def_id); + let unwind_attr = attr::find_unwind_attr(Some(tcx.sess.diagnostic()), attrs); + // We never unwind, so it's not relevant to stop an unwind if tcx.sess.panic_strategy() != PanicStrategy::Unwind { return false; } @@ -589,8 +593,7 @@ fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, // This is a special case: some functions have a C abi but are meant to // unwind anyway. Don't stop them. - let attrs = &tcx.get_attrs(fn_def_id); - match attr::find_unwind_attr(Some(tcx.sess.diagnostic()), attrs) { + match unwind_attr { None => true, Some(UnwindAttr::Allowed) => false, Some(UnwindAttr::Aborts) => true, From 797d8ea4789c64bb20869fa7fb0c15e2c09432cf Mon Sep 17 00:00:00 2001 From: Tim Date: Thu, 28 Feb 2019 07:32:13 +0100 Subject: [PATCH 16/54] Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const Make `Unique::as_ptr` const without feature attribute as it's unstable Make `NonNull::dangling` and `NonNull::cast` const with `feature = "const_ptr_nonnull"` --- src/libcore/ptr.rs | 8 +++--- src/test/run-pass/consts/const-ptr-nonnull.rs | 17 +++++++++++++ src/test/run-pass/consts/const-ptr-unique.rs | 15 +++++++++++ src/test/ui/consts/const-ptr-nonnull.rs | 11 ++++++++ src/test/ui/consts/const-ptr-nonnull.stderr | 25 +++++++++++++++++++ src/test/ui/consts/const-ptr-unique.rs | 10 ++++++++ src/test/ui/consts/const-ptr-unique.stderr | 14 +++++++++++ 7 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 src/test/run-pass/consts/const-ptr-nonnull.rs create mode 100644 src/test/run-pass/consts/const-ptr-unique.rs create mode 100644 src/test/ui/consts/const-ptr-nonnull.rs create mode 100644 src/test/ui/consts/const-ptr-nonnull.stderr create mode 100644 src/test/ui/consts/const-ptr-unique.rs create mode 100644 src/test/ui/consts/const-ptr-unique.stderr diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 866c8d0896b3c..3e1773ff9d25c 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -2790,7 +2790,7 @@ impl Unique { } /// Acquires the underlying `*mut` pointer. - pub fn as_ptr(self) -> *mut T { + pub const fn as_ptr(self) -> *mut T { self.pointer as *mut T } @@ -2903,7 +2903,8 @@ impl NonNull { /// some other means. #[stable(feature = "nonnull", since = "1.25.0")] #[inline] - pub fn dangling() -> Self { + #[cfg_attr(not(stage0), rustc_const_unstable(feature = "const_ptr_nonnull"))] + pub const fn dangling() -> Self { unsafe { let ptr = mem::align_of::() as *mut T; NonNull::new_unchecked(ptr) @@ -2966,7 +2967,8 @@ impl NonNull { /// Cast to a pointer of another type #[stable(feature = "nonnull_cast", since = "1.27.0")] #[inline] - pub fn cast(self) -> NonNull { + #[cfg_attr(not(stage0), rustc_const_unstable(feature = "const_ptr_nonnull"))] + pub const fn cast(self) -> NonNull { unsafe { NonNull::new_unchecked(self.as_ptr() as *mut U) } diff --git a/src/test/run-pass/consts/const-ptr-nonnull.rs b/src/test/run-pass/consts/const-ptr-nonnull.rs new file mode 100644 index 0000000000000..91624e92fbe75 --- /dev/null +++ b/src/test/run-pass/consts/const-ptr-nonnull.rs @@ -0,0 +1,17 @@ +// run-pass + +#![feature(const_ptr_nonnull)] + +use std::ptr::NonNull; + +const DANGLING: NonNull = NonNull::dangling(); +const CASTED: NonNull = NonNull::cast(NonNull::::dangling()); + +fn ident(ident: T) -> T { + ident +} + +pub fn main() { + assert_eq!(DANGLING, ident(NonNull::dangling())); + assert_eq!(CASTED, ident(NonNull::dangling())); +} diff --git a/src/test/run-pass/consts/const-ptr-unique.rs b/src/test/run-pass/consts/const-ptr-unique.rs new file mode 100644 index 0000000000000..eb371ab184166 --- /dev/null +++ b/src/test/run-pass/consts/const-ptr-unique.rs @@ -0,0 +1,15 @@ +// run-pass + +#![feature(ptr_internals)] + +use std::ptr::Unique; + +const PTR: *mut u32 = Unique::empty().as_ptr(); + +fn ident(ident: T) -> T { + ident +} + +pub fn main() { + assert_eq!(PTR, ident(Unique::::empty().as_ptr())); +} diff --git a/src/test/ui/consts/const-ptr-nonnull.rs b/src/test/ui/consts/const-ptr-nonnull.rs new file mode 100644 index 0000000000000..54e743aa32e23 --- /dev/null +++ b/src/test/ui/consts/const-ptr-nonnull.rs @@ -0,0 +1,11 @@ +use std::ptr::NonNull; + +fn main() { + let x: &'static NonNull = &(NonNull::dangling()); + //~^ ERROR borrowed value does not live long enough + + let mut i: i32 = 10; + let non_null = NonNull::new(&mut i).unwrap(); + let x: &'static NonNull = &(non_null.cast()); + //~^ ERROR borrowed value does not live long enough +} diff --git a/src/test/ui/consts/const-ptr-nonnull.stderr b/src/test/ui/consts/const-ptr-nonnull.stderr new file mode 100644 index 0000000000000..a9476dda6d320 --- /dev/null +++ b/src/test/ui/consts/const-ptr-nonnull.stderr @@ -0,0 +1,25 @@ +error[E0597]: borrowed value does not live long enough + --> $DIR/const-ptr-nonnull.rs:4:37 + | +LL | let x: &'static NonNull = &(NonNull::dangling()); + | ^^^^^^^^^^^^^^^^^^^^^ temporary value does not live long enough +... +LL | } + | - temporary value only lives until here + | + = note: borrowed value must be valid for the static lifetime... + +error[E0597]: borrowed value does not live long enough + --> $DIR/const-ptr-nonnull.rs:9:37 + | +LL | let x: &'static NonNull = &(non_null.cast()); + | ^^^^^^^^^^^^^^^^^ temporary value does not live long enough +LL | //~^ ERROR borrowed value does not live long enough +LL | } + | - temporary value only lives until here + | + = note: borrowed value must be valid for the static lifetime... + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/consts/const-ptr-unique.rs b/src/test/ui/consts/const-ptr-unique.rs new file mode 100644 index 0000000000000..be44a24181606 --- /dev/null +++ b/src/test/ui/consts/const-ptr-unique.rs @@ -0,0 +1,10 @@ +#![feature(ptr_internals)] + +use std::ptr::Unique; + +fn main() { + let mut i: u32 = 10; + let unique = Unique::new(&mut i).unwrap(); + let x: &'static *mut u32 = &(unique.as_ptr()); + //~^ ERROR borrowed value does not live long enough +} diff --git a/src/test/ui/consts/const-ptr-unique.stderr b/src/test/ui/consts/const-ptr-unique.stderr new file mode 100644 index 0000000000000..141465bf184d0 --- /dev/null +++ b/src/test/ui/consts/const-ptr-unique.stderr @@ -0,0 +1,14 @@ +error[E0597]: borrowed value does not live long enough + --> $DIR/const-ptr-unique.rs:8:33 + | +LL | let x: &'static *mut u32 = &(unique.as_ptr()); + | ^^^^^^^^^^^^^^^^^ temporary value does not live long enough +LL | //~^ ERROR borrowed value does not live long enough +LL | } + | - temporary value only lives until here + | + = note: borrowed value must be valid for the static lifetime... + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. From 009c91a294e56878e3359d58903d5dc8d3d0f35b Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 30 Jan 2019 14:04:56 -0600 Subject: [PATCH 17/54] add option to calculate documentation coverage --- src/doc/rustdoc/src/unstable-features.md | 21 ++++ src/librustdoc/config.rs | 12 +++ src/librustdoc/core.rs | 11 +- src/librustdoc/lib.rs | 12 +++ .../passes/calculate_doc_coverage.rs | 101 ++++++++++++++++++ src/librustdoc/passes/mod.rs | 14 +++ 6 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 src/librustdoc/passes/calculate_doc_coverage.rs diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index 3463cdb126cc6..cb741d1bf99ff 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -428,3 +428,24 @@ $ rustdoc src/lib.rs --test -Z unstable-options --persist-doctests target/rustdo This flag allows you to keep doctest executables around after they're compiled or run. Usually, rustdoc will immediately discard a compiled doctest after it's been tested, but with this option, you can keep those binaries around for farther testing. + +### `--show-coverage`: calculate the percentage of items with documentation + +Using this flag looks like this: + +```bash +$ rustdoc src/lib.rs -Z unstable-options --show-coverage +``` + +If you want to determine how many items in your crate are documented, pass this flag to rustdoc. +When it receives this flag, it will count the public items in your crate that have documentation, +and print out the counts and a percentage instead of generating docs. + +Some methodology notes about what rustdoc counts in this metric: + +* Rustdoc will only count items from your crate (i.e. items re-exported from other crates don't + count). +* Since trait implementations can inherit documentation from their trait, it will count trait impl + blocks separately, and show totals both with and without trait impls included. +* Inherent impl blocks are not counted, even though their doc comments are displayed, because the + common pattern in Rust code is to write all inherent methods into the same impl block. diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index b1c53ea92b300..d7c6b197164dc 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -85,6 +85,9 @@ pub struct Options { /// Whether to display warnings during doc generation or while gathering doctests. By default, /// all non-rustdoc-specific lints are allowed when generating docs. pub display_warnings: bool, + /// Whether to run the `calculate-doc-coverage` pass, which counts the number of public items + /// with and without documentation. + pub show_coverage: bool, // Options that alter generated documentation pages @@ -128,6 +131,7 @@ impl fmt::Debug for Options { .field("default_passes", &self.default_passes) .field("manual_passes", &self.manual_passes) .field("display_warnings", &self.display_warnings) + .field("show_coverage", &self.show_coverage) .field("crate_version", &self.crate_version) .field("render_options", &self.render_options) .finish() @@ -224,6 +228,10 @@ impl Options { for &name in passes::DEFAULT_PRIVATE_PASSES { println!("{:>20}", name); } + println!("\nPasses run with `--show-coverage`:"); + for &name in passes::DEFAULT_COVERAGE_PASSES { + println!("{:>20}", name); + } return Err(0); } @@ -415,12 +423,15 @@ impl Options { let default_passes = if matches.opt_present("no-defaults") { passes::DefaultPassOption::None + } else if matches.opt_present("show-coverage") { + passes::DefaultPassOption::Coverage } else if matches.opt_present("document-private-items") { passes::DefaultPassOption::Private } else { passes::DefaultPassOption::Default }; let manual_passes = matches.opt_strs("passes"); + let show_coverage = matches.opt_present("show-coverage"); let crate_name = matches.opt_str("crate-name"); let playground_url = matches.opt_str("playground-url"); @@ -463,6 +474,7 @@ impl Options { default_passes, manual_passes, display_warnings, + show_coverage, crate_version, persist_doctests, render_options: RenderOptions { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 4dce4e86cc449..76c3bca7c14c8 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -605,10 +605,13 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt info!("Executing passes"); - for pass in &passes { - match passes::find_pass(pass).map(|p| p.pass) { - Some(pass) => krate = pass(krate, &ctxt), - None => error!("unknown pass {}, skipping", *pass), + for pass_name in &passes { + match passes::find_pass(pass_name).map(|p| p.pass) { + Some(pass) => { + debug!("running pass {}", pass_name); + krate = pass(krate, &ctxt); + } + None => error!("unknown pass {}, skipping", *pass_name), } } diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 39e504951d1c6..625e3d05c2997 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -347,6 +347,11 @@ fn opts() -> Vec { "generate-redirect-pages", "Generate extra pages to support legacy URLs and tool links") }), + unstable("show-coverage", |o| { + o.optflag("", + "show-coverage", + "calculate percentage of public items with documentation") + }), ] } @@ -391,7 +396,14 @@ fn main_args(args: &[String]) -> isize { let diag_opts = (options.error_format, options.debugging_options.treat_err_as_bug, options.debugging_options.ui_testing); + let show_coverage = options.show_coverage; rust_input(options, move |out| { + if show_coverage { + // if we ran coverage, bail early, we don't need to also generate docs at this point + // (also we didn't load in any of the useful passes) + return rustc_driver::EXIT_SUCCESS; + } + let Output { krate, passes, renderinfo, renderopts } = out; info!("going to format"); let (error_format, treat_err_as_bug, ui_testing) = diag_opts; diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs new file mode 100644 index 0000000000000..b812415d6775f --- /dev/null +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -0,0 +1,101 @@ +use crate::clean; +use crate::core::DocContext; +use crate::fold::{self, DocFolder}; +use crate::passes::Pass; + +use syntax::attr; + +pub const CALCULATE_DOC_COVERAGE: Pass = Pass { + name: "calculate-doc-coverage", + pass: calculate_doc_coverage, + description: "counts the number of items with and without documentation", +}; + +fn calculate_doc_coverage(krate: clean::Crate, _: &DocContext<'_, '_, '_>) -> clean::Crate { + let mut calc = CoverageCalculator::default(); + let krate = calc.fold_crate(krate); + + let total_minus_traits = calc.total - calc.total_trait_impls; + let docs_minus_traits = calc.with_docs - calc.trait_impls_with_docs; + + print!("Rustdoc found {}/{} items with documentation", calc.with_docs, calc.total); + println!(" ({}/{} not counting trait impls)", docs_minus_traits, total_minus_traits); + + if calc.total > 0 { + let percentage = (calc.with_docs as f64 * 100.0) / calc.total as f64; + let percentage_minus_traits = + (docs_minus_traits as f64 * 100.0) / total_minus_traits as f64; + println!(" Score: {:.1}% ({:.1}% not counting trait impls)", + percentage, percentage_minus_traits); + } + + krate +} + +#[derive(Default)] +struct CoverageCalculator { + total: usize, + with_docs: usize, + total_trait_impls: usize, + trait_impls_with_docs: usize, +} + +impl fold::DocFolder for CoverageCalculator { + fn fold_item(&mut self, i: clean::Item) -> Option { + match i.inner { + clean::StrippedItem(..) => {} + clean::ImplItem(ref impl_) + if attr::contains_name(&i.attrs.other_attrs, "automatically_derived") + || impl_.synthetic || impl_.blanket_impl.is_some() => + { + // skip counting anything inside these impl blocks + // FIXME(misdreavus): need to also find items that came out of a derive macro + return Some(i); + } + // non-local items are skipped because they can be out of the users control, especially + // in the case of trait impls, which rustdoc eagerly inlines + _ => if i.def_id.is_local() { + let has_docs = !i.attrs.doc_strings.is_empty(); + + if let clean::ImplItem(ref i) = i.inner { + if let Some(ref tr) = i.trait_ { + debug!("counting impl {:#} for {:#}", tr, i.for_); + + self.total += 1; + if has_docs { + self.with_docs += 1; + } + + // trait impls inherit their docs from the trait definition, so documenting + // them can be considered optional + + self.total_trait_impls += 1; + if has_docs { + self.trait_impls_with_docs += 1; + } + + for it in &i.items { + self.total_trait_impls += 1; + if !it.attrs.doc_strings.is_empty() { + self.trait_impls_with_docs += 1; + } + } + } else { + // inherent impls *can* be documented, and those docs show up, but in most + // cases it doesn't make sense, as all methods on a type are in one single + // impl block + debug!("not counting impl {:#}", i.for_); + } + } else { + debug!("counting {} {:?}", i.type_(), i.name); + self.total += 1; + if has_docs { + self.with_docs += 1; + } + } + } + } + + self.fold_item_recur(i) + } +} diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 3a9d8ef20ce84..bda63ae18fd7b 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -45,6 +45,9 @@ pub use self::collect_trait_impls::COLLECT_TRAIT_IMPLS; mod check_code_block_syntax; pub use self::check_code_block_syntax::CHECK_CODE_BLOCK_SYNTAX; +mod calculate_doc_coverage; +pub use self::calculate_doc_coverage::CALCULATE_DOC_COVERAGE; + /// A single pass over the cleaned documentation. /// /// Runs in the compiler context, so it has access to types and traits and the like. @@ -67,6 +70,7 @@ pub const PASSES: &'static [Pass] = &[ COLLECT_INTRA_DOC_LINKS, CHECK_CODE_BLOCK_SYNTAX, COLLECT_TRAIT_IMPLS, + CALCULATE_DOC_COVERAGE, ]; /// The list of passes run by default. @@ -94,12 +98,21 @@ pub const DEFAULT_PRIVATE_PASSES: &[&str] = &[ "propagate-doc-cfg", ]; +/// The list of default passes run when `--doc-coverage` is passed to rustdoc. +pub const DEFAULT_COVERAGE_PASSES: &'static [&'static str] = &[ + "collect-trait-impls", + "strip-hidden", + "strip-private", + "calculate-doc-coverage", +]; + /// A shorthand way to refer to which set of passes to use, based on the presence of /// `--no-defaults` or `--document-private-items`. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum DefaultPassOption { Default, Private, + Coverage, None, } @@ -108,6 +121,7 @@ pub fn defaults(default_set: DefaultPassOption) -> &'static [&'static str] { match default_set { DefaultPassOption::Default => DEFAULT_PASSES, DefaultPassOption::Private => DEFAULT_PRIVATE_PASSES, + DefaultPassOption::Coverage => DEFAULT_COVERAGE_PASSES, DefaultPassOption::None => &[], } } From 9e98a25b9520861a6b443a1d28c04a9b1854e24e Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 30 Jan 2019 14:15:09 -0600 Subject: [PATCH 18/54] tabs -> spaces --- src/doc/rustdoc/src/unstable-features.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index cb741d1bf99ff..d838f9d95132f 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -53,7 +53,7 @@ For example, in the following code: ```rust /// Does the thing. pub fn do_the_thing(_: SomeType) { - println!("Let's do the thing!"); + println!("Let's do the thing!"); } /// Token you use to [`do_the_thing`]. @@ -66,15 +66,15 @@ target out also works: ```rust pub mod some_module { - /// Token you use to do the thing. - pub struct SomeStruct; + /// Token you use to do the thing. + pub struct SomeStruct; } /// Does the thing. Requires one [`SomeStruct`] for the thing to work. /// /// [`SomeStruct`]: some_module::SomeStruct pub fn do_the_thing(_: some_module::SomeStruct) { - println!("Let's do the thing!"); + println!("Let's do the thing!"); } ``` From fc9459351c0136715089f9e9d96f57fed2c80a52 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 20 Feb 2019 15:15:13 -0600 Subject: [PATCH 19/54] count fewer items in calculate-doc-coverage --- src/librustdoc/passes/calculate_doc_coverage.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index b812415d6775f..57ac75bf4d418 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -43,7 +43,11 @@ struct CoverageCalculator { impl fold::DocFolder for CoverageCalculator { fn fold_item(&mut self, i: clean::Item) -> Option { match i.inner { - clean::StrippedItem(..) => {} + clean::StrippedItem(..) => { + // don't count items in stripped modules + return Some(i); + } + clean::ImportItem(..) | clean::ExternCrateItem(..) => {} clean::ImplItem(ref impl_) if attr::contains_name(&i.attrs.other_attrs, "automatically_derived") || impl_.synthetic || impl_.blanket_impl.is_some() => From 95500c078bec0082fe0e4f3e20d962d436a0e099 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Wed, 20 Feb 2019 15:26:35 -0600 Subject: [PATCH 20/54] refactor: combine item count numbers into a new struct --- .../passes/calculate_doc_coverage.rs | 86 ++++++++++++------- 1 file changed, 57 insertions(+), 29 deletions(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 57ac75bf4d418..cb6d180fbd3b4 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -5,6 +5,9 @@ use crate::passes::Pass; use syntax::attr; +use std::ops::Sub; +use std::fmt; + pub const CALCULATE_DOC_COVERAGE: Pass = Pass { name: "calculate-doc-coverage", pass: calculate_doc_coverage, @@ -15,29 +18,66 @@ fn calculate_doc_coverage(krate: clean::Crate, _: &DocContext<'_, '_, '_>) -> cl let mut calc = CoverageCalculator::default(); let krate = calc.fold_crate(krate); - let total_minus_traits = calc.total - calc.total_trait_impls; - let docs_minus_traits = calc.with_docs - calc.trait_impls_with_docs; + let non_traits = calc.items - calc.trait_impl_items; - print!("Rustdoc found {}/{} items with documentation", calc.with_docs, calc.total); - println!(" ({}/{} not counting trait impls)", docs_minus_traits, total_minus_traits); + print!("Rustdoc found {} items with documentation", calc.items); + println!(" ({} not counting trait impls)", non_traits); - if calc.total > 0 { - let percentage = (calc.with_docs as f64 * 100.0) / calc.total as f64; - let percentage_minus_traits = - (docs_minus_traits as f64 * 100.0) / total_minus_traits as f64; + if let (Some(percentage), Some(percentage_non_traits)) = + (calc.items.percentage(), non_traits.percentage()) + { println!(" Score: {:.1}% ({:.1}% not counting trait impls)", - percentage, percentage_minus_traits); + percentage, percentage_non_traits); } krate } +#[derive(Default, Copy, Clone)] +struct ItemCount { + total: u64, + with_docs: u64, +} + +impl ItemCount { + fn count_item(&mut self, has_docs: bool) { + self.total += 1; + + if has_docs { + self.with_docs += 1; + } + } + + fn percentage(&self) -> Option { + if self.total > 0 { + Some((self.with_docs as f64 * 100.0) / self.total as f64) + } else { + None + } + } +} + +impl Sub for ItemCount { + type Output = Self; + + fn sub(self, rhs: Self) -> Self { + ItemCount { + total: self.total - rhs.total, + with_docs: self.with_docs - rhs.with_docs, + } + } +} + +impl fmt::Display for ItemCount { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}/{}", self.with_docs, self.total) + } +} + #[derive(Default)] struct CoverageCalculator { - total: usize, - with_docs: usize, - total_trait_impls: usize, - trait_impls_with_docs: usize, + items: ItemCount, + trait_impl_items: ItemCount, } impl fold::DocFolder for CoverageCalculator { @@ -65,24 +105,15 @@ impl fold::DocFolder for CoverageCalculator { if let Some(ref tr) = i.trait_ { debug!("counting impl {:#} for {:#}", tr, i.for_); - self.total += 1; - if has_docs { - self.with_docs += 1; - } + self.items.count_item(has_docs); // trait impls inherit their docs from the trait definition, so documenting // them can be considered optional - self.total_trait_impls += 1; - if has_docs { - self.trait_impls_with_docs += 1; - } + self.trait_impl_items.count_item(has_docs); for it in &i.items { - self.total_trait_impls += 1; - if !it.attrs.doc_strings.is_empty() { - self.trait_impls_with_docs += 1; - } + self.trait_impl_items.count_item(!it.attrs.doc_strings.is_empty()); } } else { // inherent impls *can* be documented, and those docs show up, but in most @@ -92,10 +123,7 @@ impl fold::DocFolder for CoverageCalculator { } } else { debug!("counting {} {:?}", i.type_(), i.name); - self.total += 1; - if has_docs { - self.with_docs += 1; - } + self.items.count_item(has_docs); } } } From 5eb1ab5265402cf0a40b71a5236cfe6289e7647d Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 10:58:40 -0600 Subject: [PATCH 21/54] print doc coverage as a table of individual item types --- src/librustdoc/html/item_type.rs | 2 +- .../passes/calculate_doc_coverage.rs | 206 ++++++++++++++---- 2 files changed, 166 insertions(+), 42 deletions(-) diff --git a/src/librustdoc/html/item_type.rs b/src/librustdoc/html/item_type.rs index 353fa4ae8c999..366e60b3ad920 100644 --- a/src/librustdoc/html/item_type.rs +++ b/src/librustdoc/html/item_type.rs @@ -15,7 +15,7 @@ use crate::clean; /// module headings. If you are adding to this enum and want to ensure that the sidebar also prints /// a heading, edit the listing in `html/render.rs`, function `sidebar_module`. This uses an /// ordering based on a helper function inside `item_module`, in the same file. -#[derive(Copy, PartialEq, Clone, Debug)] +#[derive(Copy, PartialEq, Eq, Clone, Debug, PartialOrd, Ord)] pub enum ItemType { Module = 0, ExternCrate = 1, diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index cb6d180fbd3b4..06f9a604ec83f 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -1,12 +1,14 @@ use crate::clean; use crate::core::DocContext; +use crate::html::item_type::ItemType; use crate::fold::{self, DocFolder}; use crate::passes::Pass; use syntax::attr; -use std::ops::Sub; +use std::collections::BTreeMap; use std::fmt; +use std::ops; pub const CALCULATE_DOC_COVERAGE: Pass = Pass { name: "calculate-doc-coverage", @@ -18,17 +20,7 @@ fn calculate_doc_coverage(krate: clean::Crate, _: &DocContext<'_, '_, '_>) -> cl let mut calc = CoverageCalculator::default(); let krate = calc.fold_crate(krate); - let non_traits = calc.items - calc.trait_impl_items; - - print!("Rustdoc found {} items with documentation", calc.items); - println!(" ({} not counting trait impls)", non_traits); - - if let (Some(percentage), Some(percentage_non_traits)) = - (calc.items.percentage(), non_traits.percentage()) - { - println!(" Score: {:.1}% ({:.1}% not counting trait impls)", - percentage, percentage_non_traits); - } + calc.print_results(); krate } @@ -57,7 +49,7 @@ impl ItemCount { } } -impl Sub for ItemCount { +impl ops::Sub for ItemCount { type Output = Self; fn sub(self, rhs: Self) -> Self { @@ -68,6 +60,13 @@ impl Sub for ItemCount { } } +impl ops::AddAssign for ItemCount { + fn add_assign(&mut self, rhs: Self) { + self.total += rhs.total; + self.with_docs += rhs.with_docs; + } +} + impl fmt::Display for ItemCount { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}/{}", self.with_docs, self.total) @@ -76,58 +75,183 @@ impl fmt::Display for ItemCount { #[derive(Default)] struct CoverageCalculator { - items: ItemCount, - trait_impl_items: ItemCount, + items: BTreeMap, +} + +impl CoverageCalculator { + fn print_results(&self) { + use crate::html::item_type::ItemType::*; + + let mut total = ItemCount::default(); + + let main_types = [ + Module, Function, + Struct, StructField, + Enum, Variant, + Union, + Method, + Trait, TyMethod, + AssociatedType, AssociatedConst, + Macro, + Static, Constant, + ForeignType, Existential, + Typedef, TraitAlias, + Primitive, Keyword, + ]; + + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + println!("| {:<25} | {:>10} | {:>10} | {:>10} |", + "Item Type", "Documented", "Total", "Percentage"); + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + + for item_type in &main_types { + let count = self.items.get(item_type).cloned().unwrap_or_default(); + + if let Some(percentage) = count.percentage() { + println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", + table_name(item_type), count.with_docs, count.total, percentage); + + total += count; + } + } + + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + + if let Some(count) = self.items.get(&Impl) { + if let Some(percentage) = count.percentage() { + if let Some(percentage) = total.percentage() { + println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", + "Total (non trait impls)", total.with_docs, total.total, percentage); + } + + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + + println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", + table_name(&Impl), count.with_docs, count.total, percentage); + + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + + total += *count; + } + } + + println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", + "Total", total.with_docs, total.total, total.percentage().unwrap_or(0.0)); + println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + } } impl fold::DocFolder for CoverageCalculator { - fn fold_item(&mut self, i: clean::Item) -> Option { + fn fold_item(&mut self, mut i: clean::Item) -> Option { + let has_docs = !i.attrs.doc_strings.is_empty(); + match i.inner { + _ if !i.def_id.is_local() => { + // non-local items are skipped because they can be out of the users control, + // especially in the case of trait impls, which rustdoc eagerly inlines + return Some(i); + } clean::StrippedItem(..) => { // don't count items in stripped modules return Some(i); } - clean::ImportItem(..) | clean::ExternCrateItem(..) => {} + clean::ImportItem(..) | clean::ExternCrateItem(..) => { + // docs on `use` and `extern crate` statements are not displayed, so they're not + // worth counting + return Some(i); + } clean::ImplItem(ref impl_) if attr::contains_name(&i.attrs.other_attrs, "automatically_derived") || impl_.synthetic || impl_.blanket_impl.is_some() => { - // skip counting anything inside these impl blocks + // built-in derives get the `#[automatically_derived]` attribute, and + // synthetic/blanket impls are made up by rustdoc and can't be documented // FIXME(misdreavus): need to also find items that came out of a derive macro return Some(i); } - // non-local items are skipped because they can be out of the users control, especially - // in the case of trait impls, which rustdoc eagerly inlines - _ => if i.def_id.is_local() { - let has_docs = !i.attrs.doc_strings.is_empty(); - - if let clean::ImplItem(ref i) = i.inner { - if let Some(ref tr) = i.trait_ { - debug!("counting impl {:#} for {:#}", tr, i.for_); + clean::ImplItem(ref impl_) => { + if let Some(ref tr) = impl_.trait_ { + debug!("counting impl {:#} for {:#}", tr, impl_.for_); - self.items.count_item(has_docs); + // trait impls inherit their docs from the trait definition, so documenting + // them can be considered optional + self.items.entry(ItemType::Impl).or_default().count_item(has_docs); - // trait impls inherit their docs from the trait definition, so documenting - // them can be considered optional + for it in &impl_.items { + let has_docs = !it.attrs.doc_strings.is_empty(); + self.items.entry(ItemType::Impl).or_default().count_item(has_docs); + } - self.trait_impl_items.count_item(has_docs); + // now skip recursing, so that we don't double-count this impl's items + return Some(i); + } else { + // inherent impls *can* be documented, and those docs show up, but in most + // cases it doesn't make sense, as all methods on a type are in one single + // impl block + debug!("not counting impl {:#}", impl_.for_); + } + } + clean::MacroItem(..) | clean::ProcMacroItem(..) => { + // combine `macro_rules!` macros and proc-macros in the same count + debug!("counting macro {:?}", i.name); + self.items.entry(ItemType::Macro).or_default().count_item(has_docs); + } + clean::TraitItem(ref mut trait_) => { + // because both trait methods with a default impl and struct methods are + // ItemType::Method, we need to properly tag trait methods as TyMethod instead + debug!("counting trait {:?}", i.name); + self.items.entry(ItemType::Trait).or_default().count_item(has_docs); - for it in &i.items { - self.trait_impl_items.count_item(!it.attrs.doc_strings.is_empty()); - } + // since we're not going on to document the crate, it doesn't matter if we discard + // the item after counting it + trait_.items.retain(|it| { + if it.type_() == ItemType::Method { + let has_docs = !it.attrs.doc_strings.is_empty(); + self.items.entry(ItemType::TyMethod).or_default().count_item(has_docs); + false } else { - // inherent impls *can* be documented, and those docs show up, but in most - // cases it doesn't make sense, as all methods on a type are in one single - // impl block - debug!("not counting impl {:#}", i.for_); + true } - } else { - debug!("counting {} {:?}", i.type_(), i.name); - self.items.count_item(has_docs); - } + }); + } + _ => { + debug!("counting {} {:?}", i.type_(), i.name); + self.items.entry(i.type_()).or_default().count_item(has_docs); } } self.fold_item_recur(i) } } + +fn table_name(type_: &ItemType) -> &'static str { + match *type_ { + ItemType::Module => "Modules", + ItemType::Struct => "Structs", + ItemType::Union => "Unions", + ItemType::Enum => "Enums", + ItemType::Function => "Functions", + ItemType::Typedef => "Type Aliases", + ItemType::Static => "Statics", + ItemType::Trait => "Traits", + // inherent impls aren't counted, and trait impls get all their items thrown into this + // counter + ItemType::Impl => "Trait Impl Items", + // even though trait methods with a default impl get cleaned as Method, we convert them + // to TyMethod when counting + ItemType::TyMethod => "Trait Methods", + ItemType::Method => "Methods", + ItemType::StructField => "Struct Fields", + ItemType::Variant => "Enum Variants", + ItemType::Macro => "Macros", + ItemType::Primitive => "Primitives", + ItemType::AssociatedType => "Associated Types", + ItemType::Constant => "Constants", + ItemType::AssociatedConst => "Associated Constants", + ItemType::ForeignType => "Foreign Types", + ItemType::Keyword => "Keywords", + ItemType::Existential => "Existential Types", + ItemType::TraitAlias => "Trait Aliases", + _ => panic!("unanticipated ItemType: {}", type_), + } +} From a3a255990e714827601623634bbc5fd59f689f71 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 11:10:12 -0600 Subject: [PATCH 22/54] add a coverage mode for private items --- src/librustdoc/config.rs | 14 +++++++++++--- src/librustdoc/passes/mod.rs | 9 +++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index d7c6b197164dc..5cbcc2433ba57 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -232,6 +232,10 @@ impl Options { for &name in passes::DEFAULT_COVERAGE_PASSES { println!("{:>20}", name); } + println!("\nPasses run with `--show-coverage --document-private-items`:"); + for &name in passes::PRIVATE_COVERAGE_PASSES { + println!("{:>20}", name); + } return Err(0); } @@ -421,17 +425,21 @@ impl Options { } }); + let show_coverage = matches.opt_present("show-coverage"); + let document_private = matches.opt_present("document-private-items"); + let default_passes = if matches.opt_present("no-defaults") { passes::DefaultPassOption::None - } else if matches.opt_present("show-coverage") { + } else if show_coverage && document_private { + passes::DefaultPassOption::PrivateCoverage + } else if show_coverage { passes::DefaultPassOption::Coverage - } else if matches.opt_present("document-private-items") { + } else if document_private { passes::DefaultPassOption::Private } else { passes::DefaultPassOption::Default }; let manual_passes = matches.opt_strs("passes"); - let show_coverage = matches.opt_present("show-coverage"); let crate_name = matches.opt_str("crate-name"); let playground_url = matches.opt_str("playground-url"); diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index bda63ae18fd7b..e36a029f97523 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -106,6 +106,13 @@ pub const DEFAULT_COVERAGE_PASSES: &'static [&'static str] = &[ "calculate-doc-coverage", ]; +/// The list of default passes run when `--doc-coverage --document-private-items` is passed to +/// rustdoc. +pub const PRIVATE_COVERAGE_PASSES: &'static [&'static str] = &[ + "collect-trait-impls", + "calculate-doc-coverage", +]; + /// A shorthand way to refer to which set of passes to use, based on the presence of /// `--no-defaults` or `--document-private-items`. #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -113,6 +120,7 @@ pub enum DefaultPassOption { Default, Private, Coverage, + PrivateCoverage, None, } @@ -122,6 +130,7 @@ pub fn defaults(default_set: DefaultPassOption) -> &'static [&'static str] { DefaultPassOption::Default => DEFAULT_PASSES, DefaultPassOption::Private => DEFAULT_PRIVATE_PASSES, DefaultPassOption::Coverage => DEFAULT_COVERAGE_PASSES, + DefaultPassOption::PrivateCoverage => PRIVATE_COVERAGE_PASSES, DefaultPassOption::None => &[], } } From 3ce19b4a2c062a7a269bd2783a4a441d515b64c8 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 16:02:21 -0600 Subject: [PATCH 23/54] tweak wording of extern types --- src/librustdoc/passes/calculate_doc_coverage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 06f9a604ec83f..f70de1ce9510e 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -248,7 +248,7 @@ fn table_name(type_: &ItemType) -> &'static str { ItemType::AssociatedType => "Associated Types", ItemType::Constant => "Constants", ItemType::AssociatedConst => "Associated Constants", - ItemType::ForeignType => "Foreign Types", + ItemType::ForeignType => "Extern Types", ItemType::Keyword => "Keywords", ItemType::Existential => "Existential Types", ItemType::TraitAlias => "Trait Aliases", From 63bdd29ef4e744d6cacc2373b3b317eeebdf2a07 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 16:02:56 -0600 Subject: [PATCH 24/54] add tests for doc coverage --- src/test/rustdoc-ui/coverage/basic.rs | 50 +++++++++++++++++++ src/test/rustdoc-ui/coverage/basic.stdout | 15 ++++++ src/test/rustdoc-ui/coverage/empty.rs | 4 ++ src/test/rustdoc-ui/coverage/empty.stdout | 7 +++ src/test/rustdoc-ui/coverage/enums.rs | 22 ++++++++ src/test/rustdoc-ui/coverage/enums.stdout | 10 ++++ src/test/rustdoc-ui/coverage/exotic.rs | 15 ++++++ src/test/rustdoc-ui/coverage/exotic.stdout | 9 ++++ src/test/rustdoc-ui/coverage/private.rs | 21 ++++++++ src/test/rustdoc-ui/coverage/private.stdout | 10 ++++ .../rustdoc-ui/coverage/statics-consts.rs | 23 +++++++++ .../rustdoc-ui/coverage/statics-consts.stdout | 12 +++++ src/test/rustdoc-ui/coverage/traits.rs | 37 ++++++++++++++ src/test/rustdoc-ui/coverage/traits.stdout | 16 ++++++ 14 files changed, 251 insertions(+) create mode 100644 src/test/rustdoc-ui/coverage/basic.rs create mode 100644 src/test/rustdoc-ui/coverage/basic.stdout create mode 100644 src/test/rustdoc-ui/coverage/empty.rs create mode 100644 src/test/rustdoc-ui/coverage/empty.stdout create mode 100644 src/test/rustdoc-ui/coverage/enums.rs create mode 100644 src/test/rustdoc-ui/coverage/enums.stdout create mode 100644 src/test/rustdoc-ui/coverage/exotic.rs create mode 100644 src/test/rustdoc-ui/coverage/exotic.stdout create mode 100644 src/test/rustdoc-ui/coverage/private.rs create mode 100644 src/test/rustdoc-ui/coverage/private.stdout create mode 100644 src/test/rustdoc-ui/coverage/statics-consts.rs create mode 100644 src/test/rustdoc-ui/coverage/statics-consts.stdout create mode 100644 src/test/rustdoc-ui/coverage/traits.rs create mode 100644 src/test/rustdoc-ui/coverage/traits.stdout diff --git a/src/test/rustdoc-ui/coverage/basic.rs b/src/test/rustdoc-ui/coverage/basic.rs new file mode 100644 index 0000000000000..4247fdf989556 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/basic.rs @@ -0,0 +1,50 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +#![feature(extern_types)] + +//! Make sure to have some docs on your crate root + +/// This struct is documented, but its fields are not. +/// +/// However, one field is private, so it shouldn't show in the total. +pub struct SomeStruct { + pub some_field: usize, + other_field: usize, +} + +impl SomeStruct { + /// Method with docs + pub fn this_fn(&self) {} + + // Method without docs + pub fn other_method(&self) {} +} + +// struct without docs +pub struct OtherStruct; + +// function with no docs +pub fn some_fn() {} + +/// Function with docs +pub fn other_fn() {} + +pub enum SomeEnum { + /// Some of these variants are documented... + VarOne, + /// ...but some of them are not. + VarTwo, + // (like this one) + VarThree, +} + +/// There's a macro here, too +#[macro_export] +macro_rules! some_macro { + () => {}; +} + +extern { + pub type ExternType; +} diff --git a/src/test/rustdoc-ui/coverage/basic.stdout b/src/test/rustdoc-ui/coverage/basic.stdout new file mode 100644 index 0000000000000..089ab6017d194 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/basic.stdout @@ -0,0 +1,15 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 1 | 100.0% | +| Functions | 1 | 2 | 50.0% | +| Structs | 1 | 2 | 50.0% | +| Struct Fields | 0 | 1 | 0.0% | +| Enums | 0 | 1 | 0.0% | +| Enum Variants | 2 | 3 | 66.7% | +| Methods | 1 | 2 | 50.0% | +| Macros | 1 | 1 | 100.0% | +| Extern Types | 0 | 1 | 0.0% | ++---------------------------+------------+------------+------------+ +| Total | 7 | 14 | 50.0% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/empty.rs b/src/test/rustdoc-ui/coverage/empty.rs new file mode 100644 index 0000000000000..463617a1143df --- /dev/null +++ b/src/test/rustdoc-ui/coverage/empty.rs @@ -0,0 +1,4 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +// an empty crate still has one item to document: the crate root diff --git a/src/test/rustdoc-ui/coverage/empty.stdout b/src/test/rustdoc-ui/coverage/empty.stdout new file mode 100644 index 0000000000000..df68205bbaa3f --- /dev/null +++ b/src/test/rustdoc-ui/coverage/empty.stdout @@ -0,0 +1,7 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 0 | 1 | 0.0% | ++---------------------------+------------+------------+------------+ +| Total | 0 | 1 | 0.0% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/enums.rs b/src/test/rustdoc-ui/coverage/enums.rs new file mode 100644 index 0000000000000..5cd7f490d1a9a --- /dev/null +++ b/src/test/rustdoc-ui/coverage/enums.rs @@ -0,0 +1,22 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +//! (remember the crate root is still a module) + +/// so check out this enum here +pub enum ThisEnum { + /// this variant has some weird stuff going on + VarOne { + /// like, it has some named fields inside + field_one: usize, + // (these show up as struct fields) + field_two: usize, + }, + /// here's another variant for you + VarTwo(String), + // but not all of them need to be documented as thoroughly + VarThree, +} + +/// uninhabited enums? sure, let's throw one of those around +pub enum OtherEnum {} diff --git a/src/test/rustdoc-ui/coverage/enums.stdout b/src/test/rustdoc-ui/coverage/enums.stdout new file mode 100644 index 0000000000000..651ea0953102f --- /dev/null +++ b/src/test/rustdoc-ui/coverage/enums.stdout @@ -0,0 +1,10 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 1 | 100.0% | +| Struct Fields | 1 | 2 | 50.0% | +| Enums | 2 | 2 | 100.0% | +| Enum Variants | 2 | 3 | 66.7% | ++---------------------------+------------+------------+------------+ +| Total | 6 | 8 | 75.0% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/exotic.rs b/src/test/rustdoc-ui/coverage/exotic.rs new file mode 100644 index 0000000000000..b4adf45b90b8a --- /dev/null +++ b/src/test/rustdoc-ui/coverage/exotic.rs @@ -0,0 +1,15 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +#![feature(doc_keyword)] + +//! the features only used in std also have entries in the table, so make sure those get pulled out +//! properly as well + +/// woo, check it out, we can write our own primitive docs lol +#[doc(primitive="unit")] +mod prim_unit {} + +/// keywords? sure, pile them on +#[doc(keyword="where")] +mod where_keyword {} diff --git a/src/test/rustdoc-ui/coverage/exotic.stdout b/src/test/rustdoc-ui/coverage/exotic.stdout new file mode 100644 index 0000000000000..97eab50a55a48 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/exotic.stdout @@ -0,0 +1,9 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 1 | 100.0% | +| Primitives | 1 | 1 | 100.0% | +| Keywords | 1 | 1 | 100.0% | ++---------------------------+------------+------------+------------+ +| Total | 3 | 3 | 100.0% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/private.rs b/src/test/rustdoc-ui/coverage/private.rs new file mode 100644 index 0000000000000..9024185856daa --- /dev/null +++ b/src/test/rustdoc-ui/coverage/private.rs @@ -0,0 +1,21 @@ +// compile-flags:-Z unstable-options --show-coverage --document-private-items +// compile-pass + +#![allow(unused)] + +//! when `--document-private-items` is passed, nothing is safe. everything must have docs or your +//! score will suffer the consequences + +mod this_mod { + fn private_fn() {} +} + +/// See, our public items have docs! +pub struct SomeStruct { + /// Look, all perfectly documented! + pub field: usize, + other: usize, +} + +/// Nothing shady going on here. Just a bunch of well-documented code. (cough) +pub fn public_fn() {} diff --git a/src/test/rustdoc-ui/coverage/private.stdout b/src/test/rustdoc-ui/coverage/private.stdout new file mode 100644 index 0000000000000..f1a5461b836cf --- /dev/null +++ b/src/test/rustdoc-ui/coverage/private.stdout @@ -0,0 +1,10 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 2 | 50.0% | +| Functions | 1 | 2 | 50.0% | +| Structs | 1 | 1 | 100.0% | +| Struct Fields | 1 | 2 | 50.0% | ++---------------------------+------------+------------+------------+ +| Total | 4 | 7 | 57.1% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/statics-consts.rs b/src/test/rustdoc-ui/coverage/statics-consts.rs new file mode 100644 index 0000000000000..3c1dd35dfe1ab --- /dev/null +++ b/src/test/rustdoc-ui/coverage/statics-consts.rs @@ -0,0 +1,23 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +//! gotta make sure we can count statics and consts correctly, too + +/// static like electricity, right? +pub static THIS_STATIC: usize = 0; + +/// (it's not electricity, is it) +pub const THIS_CONST: usize = 1; + +/// associated consts show up separately, but let's throw them in as well +pub trait SomeTrait { + /// just like that, yeah + const ASSOC_CONST: usize; +} + +pub struct SomeStruct; + +impl SomeStruct { + /// wait, structs can have them too, can't forget those + pub const ASSOC_CONST: usize = 100; +} diff --git a/src/test/rustdoc-ui/coverage/statics-consts.stdout b/src/test/rustdoc-ui/coverage/statics-consts.stdout new file mode 100644 index 0000000000000..54516fe613fb4 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/statics-consts.stdout @@ -0,0 +1,12 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 1 | 1 | 100.0% | +| Structs | 0 | 1 | 0.0% | +| Traits | 1 | 1 | 100.0% | +| Associated Constants | 2 | 2 | 100.0% | +| Statics | 1 | 1 | 100.0% | +| Constants | 1 | 1 | 100.0% | ++---------------------------+------------+------------+------------+ +| Total | 6 | 7 | 85.7% | ++---------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/traits.rs b/src/test/rustdoc-ui/coverage/traits.rs new file mode 100644 index 0000000000000..0a6defa17f85b --- /dev/null +++ b/src/test/rustdoc-ui/coverage/traits.rs @@ -0,0 +1,37 @@ +// compile-flags:-Z unstable-options --show-coverage +// compile-pass + +#![feature(trait_alias)] + +/// look at this trait right here +pub trait ThisTrait { + /// that's a trait all right + fn right_here(&self); + + /// even the provided functions show up as trait methods + fn aww_yeah(&self) {} + + /// gotta check those associated types, they're slippery + type SomeType; +} + +/// so what happens if we take some struct... +pub struct SomeStruct; + +/// ...and slap this trait on it? +impl ThisTrait for SomeStruct { + /// what we get is a perfect combo! + fn right_here(&self) {} + + type SomeType = String; +} + +/// but what about those aliases? i hear they're pretty exotic +pub trait MyAlias = ThisTrait + Send + Sync; + +// FIXME(58624): once rustdoc can process existential types, we need to make sure they're counted +// /// woah, getting all existential in here +// pub existential type ThisExists: ThisTrait; +// +// /// why don't we get a little more concrete +// pub fn defines() -> ThisExists { SomeStruct {} } diff --git a/src/test/rustdoc-ui/coverage/traits.stdout b/src/test/rustdoc-ui/coverage/traits.stdout new file mode 100644 index 0000000000000..6f5db8729efb3 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/traits.stdout @@ -0,0 +1,16 @@ ++---------------------------+------------+------------+------------+ +| Item Type | Documented | Total | Percentage | ++---------------------------+------------+------------+------------+ +| Modules | 0 | 1 | 0.0% | +| Structs | 1 | 1 | 100.0% | +| Traits | 1 | 1 | 100.0% | +| Trait Methods | 2 | 2 | 100.0% | +| Associated Types | 1 | 1 | 100.0% | +| Trait Aliases | 1 | 1 | 100.0% | ++---------------------------+------------+------------+------------+ +| Total (non trait impls) | 6 | 7 | 85.7% | ++---------------------------+------------+------------+------------+ +| Trait Impl Items | 2 | 3 | 66.7% | ++---------------------------+------------+------------+------------+ +| Total | 8 | 10 | 80.0% | ++---------------------------+------------+------------+------------+ From 80b49191bbf8cf7b418fc828f944bf4580121db3 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Feb 2019 16:04:56 -0600 Subject: [PATCH 25/54] update docs for doc coverage --- src/doc/rustdoc/src/unstable-features.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index d838f9d95132f..22bfa0bd553b3 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -445,7 +445,9 @@ Some methodology notes about what rustdoc counts in this metric: * Rustdoc will only count items from your crate (i.e. items re-exported from other crates don't count). -* Since trait implementations can inherit documentation from their trait, it will count trait impl - blocks separately, and show totals both with and without trait impls included. +* Since trait implementations can inherit documentation from their trait, separate totals are given + both with and without trait implementations. * Inherent impl blocks are not counted, even though their doc comments are displayed, because the common pattern in Rust code is to write all inherent methods into the same impl block. +* By default, only public items are counted. To count private items as well, pass + `--document-private-items` at the same time. From 1b63543dc62d2df0143b8d003017e29eca097063 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Feb 2019 15:27:42 -0600 Subject: [PATCH 26/54] track items per-file instead of per-type --- .../passes/calculate_doc_coverage.rs | 150 ++++-------------- 1 file changed, 33 insertions(+), 117 deletions(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index f70de1ce9510e..6e0238b7a4d5e 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -1,10 +1,10 @@ use crate::clean; use crate::core::DocContext; -use crate::html::item_type::ItemType; use crate::fold::{self, DocFolder}; use crate::passes::Pass; use syntax::attr; +use syntax_pos::FileName; use std::collections::BTreeMap; use std::fmt; @@ -75,74 +75,51 @@ impl fmt::Display for ItemCount { #[derive(Default)] struct CoverageCalculator { - items: BTreeMap, + items: BTreeMap, } impl CoverageCalculator { fn print_results(&self) { - use crate::html::item_type::ItemType::*; - let mut total = ItemCount::default(); - let main_types = [ - Module, Function, - Struct, StructField, - Enum, Variant, - Union, - Method, - Trait, TyMethod, - AssociatedType, AssociatedConst, - Macro, - Static, Constant, - ForeignType, Existential, - Typedef, TraitAlias, - Primitive, Keyword, - ]; - - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); - println!("| {:<25} | {:>10} | {:>10} | {:>10} |", - "Item Type", "Documented", "Total", "Percentage"); - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); - - for item_type in &main_types { - let count = self.items.get(item_type).cloned().unwrap_or_default(); - - if let Some(percentage) = count.percentage() { - println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", - table_name(item_type), count.with_docs, count.total, percentage); + fn print_table_line() { + println!("+-{0:->35}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + } - total += count; - } + fn print_table_record(name: &str, count: ItemCount, percentage: f64) { + println!("| {:<35} | {:>10} | {:>10} | {:>9.1}% |", + name, count.with_docs, count.total, percentage); } - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + print_table_line(); + println!("| {:<35} | {:>10} | {:>10} | {:>10} |", + "File", "Documented", "Total", "Percentage"); + print_table_line(); - if let Some(count) = self.items.get(&Impl) { + for (file, &count) in &self.items { if let Some(percentage) = count.percentage() { - if let Some(percentage) = total.percentage() { - println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", - "Total (non trait impls)", total.with_docs, total.total, percentage); + let mut name = file.to_string(); + // if a filename is too long, shorten it so we don't blow out the table + // FIXME(misdreavus): this needs to count graphemes, and probably also track + // double-wide characters... + if name.len() > 35 { + name = "...".to_string() + &name[name.len()-32..]; } - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); - - println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", - table_name(&Impl), count.with_docs, count.total, percentage); - - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + print_table_record(&name, count, percentage); - total += *count; + total += count; } } - println!("| {:<25} | {:>10} | {:>10} | {:>9.1}% |", - "Total", total.with_docs, total.total, total.percentage().unwrap_or(0.0)); - println!("+-{0:->25}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + print_table_line(); + print_table_record("Total", total, total.percentage().unwrap_or(0.0)); + print_table_line(); } } impl fold::DocFolder for CoverageCalculator { - fn fold_item(&mut self, mut i: clean::Item) -> Option { + fn fold_item(&mut self, i: clean::Item) -> Option { let has_docs = !i.attrs.doc_strings.is_empty(); match i.inner { @@ -171,87 +148,26 @@ impl fold::DocFolder for CoverageCalculator { } clean::ImplItem(ref impl_) => { if let Some(ref tr) = impl_.trait_ { - debug!("counting impl {:#} for {:#}", tr, impl_.for_); - - // trait impls inherit their docs from the trait definition, so documenting - // them can be considered optional - self.items.entry(ItemType::Impl).or_default().count_item(has_docs); - - for it in &impl_.items { - let has_docs = !it.attrs.doc_strings.is_empty(); - self.items.entry(ItemType::Impl).or_default().count_item(has_docs); - } + debug!("impl {:#} for {:#} in {}", tr, impl_.for_, i.source.filename); - // now skip recursing, so that we don't double-count this impl's items + // don't count trait impls, the missing-docs lint doesn't so we shouldn't + // either return Some(i); } else { // inherent impls *can* be documented, and those docs show up, but in most // cases it doesn't make sense, as all methods on a type are in one single // impl block - debug!("not counting impl {:#}", impl_.for_); + debug!("impl {:#} in {}", impl_.for_, i.source.filename); } } - clean::MacroItem(..) | clean::ProcMacroItem(..) => { - // combine `macro_rules!` macros and proc-macros in the same count - debug!("counting macro {:?}", i.name); - self.items.entry(ItemType::Macro).or_default().count_item(has_docs); - } - clean::TraitItem(ref mut trait_) => { - // because both trait methods with a default impl and struct methods are - // ItemType::Method, we need to properly tag trait methods as TyMethod instead - debug!("counting trait {:?}", i.name); - self.items.entry(ItemType::Trait).or_default().count_item(has_docs); - - // since we're not going on to document the crate, it doesn't matter if we discard - // the item after counting it - trait_.items.retain(|it| { - if it.type_() == ItemType::Method { - let has_docs = !it.attrs.doc_strings.is_empty(); - self.items.entry(ItemType::TyMethod).or_default().count_item(has_docs); - false - } else { - true - } - }); - } _ => { - debug!("counting {} {:?}", i.type_(), i.name); - self.items.entry(i.type_()).or_default().count_item(has_docs); + debug!("counting {} {:?} in {}", i.type_(), i.name, i.source.filename); + self.items.entry(i.source.filename.clone()) + .or_default() + .count_item(has_docs); } } self.fold_item_recur(i) } } - -fn table_name(type_: &ItemType) -> &'static str { - match *type_ { - ItemType::Module => "Modules", - ItemType::Struct => "Structs", - ItemType::Union => "Unions", - ItemType::Enum => "Enums", - ItemType::Function => "Functions", - ItemType::Typedef => "Type Aliases", - ItemType::Static => "Statics", - ItemType::Trait => "Traits", - // inherent impls aren't counted, and trait impls get all their items thrown into this - // counter - ItemType::Impl => "Trait Impl Items", - // even though trait methods with a default impl get cleaned as Method, we convert them - // to TyMethod when counting - ItemType::TyMethod => "Trait Methods", - ItemType::Method => "Methods", - ItemType::StructField => "Struct Fields", - ItemType::Variant => "Enum Variants", - ItemType::Macro => "Macros", - ItemType::Primitive => "Primitives", - ItemType::AssociatedType => "Associated Types", - ItemType::Constant => "Constants", - ItemType::AssociatedConst => "Associated Constants", - ItemType::ForeignType => "Extern Types", - ItemType::Keyword => "Keywords", - ItemType::Existential => "Existential Types", - ItemType::TraitAlias => "Trait Aliases", - _ => panic!("unanticipated ItemType: {}", type_), - } -} From 74cf1adfd640ac81f01238d6d5b2a5befb707e6f Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Feb 2019 15:31:39 -0600 Subject: [PATCH 27/54] tweak docs for rustdoc's `--show-coverage` --- src/doc/rustdoc/src/unstable-features.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index 22bfa0bd553b3..3938df1a68267 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -445,9 +445,13 @@ Some methodology notes about what rustdoc counts in this metric: * Rustdoc will only count items from your crate (i.e. items re-exported from other crates don't count). -* Since trait implementations can inherit documentation from their trait, separate totals are given - both with and without trait implementations. -* Inherent impl blocks are not counted, even though their doc comments are displayed, because the - common pattern in Rust code is to write all inherent methods into the same impl block. +* Docs written directly onto inherent impl blocks are not counted, even though their doc comments + are displayed, because the common pattern in Rust code is to write all inherent methods into the + same impl block. +* Items in a trait implementation are not counted, as those impls will inherit any docs from the + trait itself. * By default, only public items are counted. To count private items as well, pass `--document-private-items` at the same time. + +Public items that are not documented can be seen with the built-in `missing_docs` lint. Private +items that are not documented can be seen with Clippy's `missing_docs_in_private_items` lint. From 515dbe73abb3ee20582011f9bcc2cb20ced098d3 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Feb 2019 16:24:38 -0600 Subject: [PATCH 28/54] update rustdoc coverage tests with new table layout --- src/test/rustdoc-ui/coverage/basic.stdout | 22 ++++++------------ src/test/rustdoc-ui/coverage/empty.stdout | 14 +++++------ src/test/rustdoc-ui/coverage/enums.stdout | 17 ++++++-------- src/test/rustdoc-ui/coverage/exotic.stdout | 17 +++++++------- src/test/rustdoc-ui/coverage/private.stdout | 17 ++++++-------- .../rustdoc-ui/coverage/statics-consts.stdout | 19 ++++++--------- src/test/rustdoc-ui/coverage/traits.rs | 2 +- src/test/rustdoc-ui/coverage/traits.stdout | 23 ++++++------------- 8 files changed, 51 insertions(+), 80 deletions(-) diff --git a/src/test/rustdoc-ui/coverage/basic.stdout b/src/test/rustdoc-ui/coverage/basic.stdout index 089ab6017d194..3e91660631626 100644 --- a/src/test/rustdoc-ui/coverage/basic.stdout +++ b/src/test/rustdoc-ui/coverage/basic.stdout @@ -1,15 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 1 | 100.0% | -| Functions | 1 | 2 | 50.0% | -| Structs | 1 | 2 | 50.0% | -| Struct Fields | 0 | 1 | 0.0% | -| Enums | 0 | 1 | 0.0% | -| Enum Variants | 2 | 3 | 66.7% | -| Methods | 1 | 2 | 50.0% | -| Macros | 1 | 1 | 100.0% | -| Extern Types | 0 | 1 | 0.0% | -+---------------------------+------------+------------+------------+ -| Total | 7 | 14 | 50.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/basic.rs | 7 | 14 | 50.0% | ++-------------------------------------+------------+------------+------------+ +| Total | 7 | 14 | 50.0% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/empty.stdout b/src/test/rustdoc-ui/coverage/empty.stdout index df68205bbaa3f..11b514fbfeaef 100644 --- a/src/test/rustdoc-ui/coverage/empty.stdout +++ b/src/test/rustdoc-ui/coverage/empty.stdout @@ -1,7 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 0 | 1 | 0.0% | -+---------------------------+------------+------------+------------+ -| Total | 0 | 1 | 0.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/empty.rs | 0 | 1 | 0.0% | ++-------------------------------------+------------+------------+------------+ +| Total | 0 | 1 | 0.0% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/enums.stdout b/src/test/rustdoc-ui/coverage/enums.stdout index 651ea0953102f..87e2ad9f20df6 100644 --- a/src/test/rustdoc-ui/coverage/enums.stdout +++ b/src/test/rustdoc-ui/coverage/enums.stdout @@ -1,10 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 1 | 100.0% | -| Struct Fields | 1 | 2 | 50.0% | -| Enums | 2 | 2 | 100.0% | -| Enum Variants | 2 | 3 | 66.7% | -+---------------------------+------------+------------+------------+ -| Total | 6 | 8 | 75.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/enums.rs | 6 | 8 | 75.0% | ++-------------------------------------+------------+------------+------------+ +| Total | 6 | 8 | 75.0% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/exotic.stdout b/src/test/rustdoc-ui/coverage/exotic.stdout index 97eab50a55a48..2bacfcfcecabe 100644 --- a/src/test/rustdoc-ui/coverage/exotic.stdout +++ b/src/test/rustdoc-ui/coverage/exotic.stdout @@ -1,9 +1,8 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 1 | 100.0% | -| Primitives | 1 | 1 | 100.0% | -| Keywords | 1 | 1 | 100.0% | -+---------------------------+------------+------------+------------+ -| Total | 3 | 3 | 100.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...st/rustdoc-ui/coverage/exotic.rs | 1 | 1 | 100.0% | +| | 2 | 2 | 100.0% | ++-------------------------------------+------------+------------+------------+ +| Total | 3 | 3 | 100.0% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/private.stdout b/src/test/rustdoc-ui/coverage/private.stdout index f1a5461b836cf..0d4c7c68fd05e 100644 --- a/src/test/rustdoc-ui/coverage/private.stdout +++ b/src/test/rustdoc-ui/coverage/private.stdout @@ -1,10 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 2 | 50.0% | -| Functions | 1 | 2 | 50.0% | -| Structs | 1 | 1 | 100.0% | -| Struct Fields | 1 | 2 | 50.0% | -+---------------------------+------------+------------+------------+ -| Total | 4 | 7 | 57.1% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...t/rustdoc-ui/coverage/private.rs | 4 | 7 | 57.1% | ++-------------------------------------+------------+------------+------------+ +| Total | 4 | 7 | 57.1% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/statics-consts.stdout b/src/test/rustdoc-ui/coverage/statics-consts.stdout index 54516fe613fb4..8459f90ae7b31 100644 --- a/src/test/rustdoc-ui/coverage/statics-consts.stdout +++ b/src/test/rustdoc-ui/coverage/statics-consts.stdout @@ -1,12 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 1 | 1 | 100.0% | -| Structs | 0 | 1 | 0.0% | -| Traits | 1 | 1 | 100.0% | -| Associated Constants | 2 | 2 | 100.0% | -| Statics | 1 | 1 | 100.0% | -| Constants | 1 | 1 | 100.0% | -+---------------------------+------------+------------+------------+ -| Total | 6 | 7 | 85.7% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...oc-ui/coverage/statics-consts.rs | 6 | 7 | 85.7% | ++-------------------------------------+------------+------------+------------+ +| Total | 6 | 7 | 85.7% | ++-------------------------------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/traits.rs b/src/test/rustdoc-ui/coverage/traits.rs index 0a6defa17f85b..5f32d5b0cccc7 100644 --- a/src/test/rustdoc-ui/coverage/traits.rs +++ b/src/test/rustdoc-ui/coverage/traits.rs @@ -20,7 +20,7 @@ pub struct SomeStruct; /// ...and slap this trait on it? impl ThisTrait for SomeStruct { - /// what we get is a perfect combo! + /// nothing! trait impls are totally ignored in this calculation, sorry. fn right_here(&self) {} type SomeType = String; diff --git a/src/test/rustdoc-ui/coverage/traits.stdout b/src/test/rustdoc-ui/coverage/traits.stdout index 6f5db8729efb3..e347a4da0b978 100644 --- a/src/test/rustdoc-ui/coverage/traits.stdout +++ b/src/test/rustdoc-ui/coverage/traits.stdout @@ -1,16 +1,7 @@ -+---------------------------+------------+------------+------------+ -| Item Type | Documented | Total | Percentage | -+---------------------------+------------+------------+------------+ -| Modules | 0 | 1 | 0.0% | -| Structs | 1 | 1 | 100.0% | -| Traits | 1 | 1 | 100.0% | -| Trait Methods | 2 | 2 | 100.0% | -| Associated Types | 1 | 1 | 100.0% | -| Trait Aliases | 1 | 1 | 100.0% | -+---------------------------+------------+------------+------------+ -| Total (non trait impls) | 6 | 7 | 85.7% | -+---------------------------+------------+------------+------------+ -| Trait Impl Items | 2 | 3 | 66.7% | -+---------------------------+------------+------------+------------+ -| Total | 8 | 10 | 80.0% | -+---------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+ +| File | Documented | Total | Percentage | ++-------------------------------------+------------+------------+------------+ +| ...st/rustdoc-ui/coverage/traits.rs | 6 | 7 | 85.7% | ++-------------------------------------+------------+------------+------------+ +| Total | 6 | 7 | 85.7% | ++-------------------------------------+------------+------------+------------+ From 5360ded0e50b332b8f7dfc725e3a59d34d4265a8 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Fri, 1 Mar 2019 21:26:10 -0500 Subject: [PATCH 29/54] fix an issue with path probing on Windows The old logic would incorrectly look for "python2.exe" when searching for "python2.7.exe". --- src/bootstrap/sanity.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/sanity.rs b/src/bootstrap/sanity.rs index ff4fb85bbfad3..1a26309b47146 100644 --- a/src/bootstrap/sanity.rs +++ b/src/bootstrap/sanity.rs @@ -34,15 +34,17 @@ impl Finder { fn maybe_have>(&mut self, cmd: S) -> Option { let cmd: OsString = cmd.as_ref().into(); - let path = self.path.clone(); + let path = &self.path; self.cache.entry(cmd.clone()).or_insert_with(|| { - for path in env::split_paths(&path) { + for path in env::split_paths(path) { let target = path.join(&cmd); - let mut cmd_alt = cmd.clone(); - cmd_alt.push(".exe"); - if target.is_file() || // some/path/git - target.with_extension("exe").exists() || // some/path/git.exe - target.join(&cmd_alt).exists() { // some/path/git/git.exe + let mut cmd_exe = cmd.clone(); + cmd_exe.push(".exe"); + + if target.is_file() // some/path/git + || path.join(&cmd_exe).exists() // some/path/git.exe + || target.join(&cmd_exe).exists() // some/path/git/git.exe + { return Some(target); } } From 12d8a7d64e0c70a33dea591f7158aad38b1563c5 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Fri, 22 Feb 2019 10:24:29 -0500 Subject: [PATCH 30/54] look for python2 symlinks before bootstrap python Before this commit, if you're running x.py directly on a system where `python` is symlinked to Python 3, then the `python` config option will default to a Python 3 interpreter. This causes debuginfo tests to fail with an opaque error message, since they have a hard requirement on Python 2. This commit modifies the Python probe behavior to look for python2.7 and python2 *before* using the interpreter used to execute `x.py`. --- config.toml.example | 3 +++ src/bootstrap/sanity.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/config.toml.example b/config.toml.example index f45db37c33b86..b4ad15a823578 100644 --- a/config.toml.example +++ b/config.toml.example @@ -162,6 +162,9 @@ # Python interpreter to use for various tasks throughout the build, notably # rustdoc tests, the lldb python interpreter, and some dist bits and pieces. # Note that Python 2 is currently required. +# +# Defaults to python2.7, then python2. If neither executable can be found, then +# it defaults to the Python interpreter used to execute x.py. #python = "python2.7" # Force Cargo to check that Cargo.lock describes the precise dependency diff --git a/src/bootstrap/sanity.rs b/src/bootstrap/sanity.rs index 1a26309b47146..b9f456e910038 100644 --- a/src/bootstrap/sanity.rs +++ b/src/bootstrap/sanity.rs @@ -109,9 +109,9 @@ pub fn check(build: &mut Build) { } build.config.python = build.config.python.take().map(|p| cmd_finder.must_have(p)) - .or_else(|| env::var_os("BOOTSTRAP_PYTHON").map(PathBuf::from)) // set by bootstrap.py .or_else(|| cmd_finder.maybe_have("python2.7")) .or_else(|| cmd_finder.maybe_have("python2")) + .or_else(|| env::var_os("BOOTSTRAP_PYTHON").map(PathBuf::from)) // set by bootstrap.py .or_else(|| Some(cmd_finder.must_have("python"))); build.config.nodejs = build.config.nodejs.take().map(|p| cmd_finder.must_have(p)) From 60a649ef6ecf905253507997211ebd081f298f24 Mon Sep 17 00:00:00 2001 From: Tim Date: Sun, 3 Mar 2019 23:43:46 +0100 Subject: [PATCH 31/54] Add .nll.stderr output --- .../ui/consts/const-ptr-nonnull.nll.stderr | 25 +++++++++++++++++++ .../ui/consts/const-ptr-unique.nll.stderr | 14 +++++++++++ 2 files changed, 39 insertions(+) create mode 100644 src/test/ui/consts/const-ptr-nonnull.nll.stderr create mode 100644 src/test/ui/consts/const-ptr-unique.nll.stderr diff --git a/src/test/ui/consts/const-ptr-nonnull.nll.stderr b/src/test/ui/consts/const-ptr-nonnull.nll.stderr new file mode 100644 index 0000000000000..6977e7fdc1183 --- /dev/null +++ b/src/test/ui/consts/const-ptr-nonnull.nll.stderr @@ -0,0 +1,25 @@ +error[E0716]: temporary value dropped while borrowed + --> $DIR/const-ptr-nonnull.rs:4:37 + | +LL | let x: &'static NonNull = &(NonNull::dangling()); + | --------------------- ^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use + | | + | type annotation requires that borrow lasts for `'static` +... +LL | } + | - temporary value is freed at the end of this statement + +error[E0716]: temporary value dropped while borrowed + --> $DIR/const-ptr-nonnull.rs:9:37 + | +LL | let x: &'static NonNull = &(non_null.cast()); + | --------------------- ^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use + | | + | type annotation requires that borrow lasts for `'static` +LL | //~^ ERROR borrowed value does not live long enough +LL | } + | - temporary value is freed at the end of this statement + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0716`. diff --git a/src/test/ui/consts/const-ptr-unique.nll.stderr b/src/test/ui/consts/const-ptr-unique.nll.stderr new file mode 100644 index 0000000000000..b201994c894e4 --- /dev/null +++ b/src/test/ui/consts/const-ptr-unique.nll.stderr @@ -0,0 +1,14 @@ +error[E0716]: temporary value dropped while borrowed + --> $DIR/const-ptr-unique.rs:8:33 + | +LL | let x: &'static *mut u32 = &(unique.as_ptr()); + | ----------------- ^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use + | | + | type annotation requires that borrow lasts for `'static` +LL | //~^ ERROR borrowed value does not live long enough +LL | } + | - temporary value is freed at the end of this statement + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0716`. From 5c0615b89c399ce6a431ba660f9457c24bc69964 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 20:51:43 +0100 Subject: [PATCH 32/54] Use early unwraps instead of bubbling up errors just to unwrap in the end --- src/librustc_codegen_ssa/mir/constant.rs | 14 ++++---- src/librustc_mir/const_eval.rs | 43 +++++++++++------------- src/librustc_mir/hair/pattern/_match.rs | 8 +---- src/librustc_mir/hair/pattern/mod.rs | 9 ++--- 4 files changed, 30 insertions(+), 44 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/constant.rs b/src/librustc_codegen_ssa/mir/constant.rs index 6bc69efa4a7d5..349c9132842b8 100644 --- a/src/librustc_codegen_ssa/mir/constant.rs +++ b/src/librustc_codegen_ssa/mir/constant.rs @@ -49,36 +49,36 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { constant: Result, ErrorHandled>, ) -> (Bx::Value, Ty<'tcx>) { constant - .and_then(|c| { + .map(|c| { let field_ty = c.ty.builtin_index().unwrap(); let fields = match c.ty.sty { ty::Array(_, n) => n.unwrap_usize(bx.tcx()), ref other => bug!("invalid simd shuffle type: {}", other), }; - let values: Result, ErrorHandled> = (0..fields).map(|field| { + let values: Vec<_> = (0..fields).map(|field| { let field = const_field( bx.tcx(), ty::ParamEnv::reveal_all(), None, mir::Field::new(field as usize), c, - )?; + ); if let Some(prim) = field.val.try_to_scalar() { let layout = bx.layout_of(field_ty); let scalar = match layout.abi { layout::Abi::Scalar(ref x) => x, _ => bug!("from_const: invalid ByVal layout: {:#?}", layout) }; - Ok(bx.scalar_to_backend( + bx.scalar_to_backend( prim, scalar, bx.immediate_backend_type(layout), - )) + ) } else { bug!("simd shuffle field {:?}", field) } }).collect(); - let llval = bx.const_struct(&values?, false); - Ok((llval, c.ty)) + let llval = bx.const_struct(&values, false); + (llval, c.ty) }) .unwrap_or_else(|_| { bx.tcx().sess.span_err( diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 35ca13891876c..ee522570b3634 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -466,45 +466,42 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx> } /// Projects to a field of a (variant of a) const. +// this function uses `unwrap` copiously, because an already validated constant must have valid +// fields and can thus never fail outside of compiler bugs pub fn const_field<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, param_env: ty::ParamEnv<'tcx>, variant: Option, field: mir::Field, value: ty::Const<'tcx>, -) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> { +) -> ty::Const<'tcx> { trace!("const_field: {:?}, {:?}", field, value); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - let result = (|| { - // get the operand again - let op = ecx.const_to_op(value, None)?; - // downcast - let down = match variant { - None => op, - Some(variant) => ecx.operand_downcast(op, variant)? - }; - // then project - let field = ecx.operand_field(down, field.index() as u64)?; - // and finally move back to the const world, always normalizing because - // this is not called for statics. - op_to_const(&ecx, field) - })(); - result.map_err(|error| { - let err = error_to_const_error(&ecx, error); - err.report_as_error(ecx.tcx, "could not access field of constant"); - ErrorHandled::Reported - }) + // get the operand again + let op = ecx.const_to_op(value, None).unwrap(); + // downcast + let down = match variant { + None => op, + Some(variant) => ecx.operand_downcast(op, variant).unwrap(), + }; + // then project + let field = ecx.operand_field(down, field.index() as u64).unwrap(); + // and finally move back to the const world, always normalizing because + // this is not called for statics. + op_to_const(&ecx, field).unwrap() } +// this function uses `unwrap` copiously, because an already validated constant must have valid +// fields and can thus never fail outside of compiler bugs pub fn const_variant_index<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, param_env: ty::ParamEnv<'tcx>, val: ty::Const<'tcx>, -) -> EvalResult<'tcx, VariantIdx> { +) -> VariantIdx { trace!("const_variant_index: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - let op = ecx.const_to_op(val, None)?; - Ok(ecx.read_discriminant(op)?.1) + let op = ecx.const_to_op(val, None).unwrap(); + ecx.read_discriminant(op).unwrap().1 } pub fn error_to_const_error<'a, 'mir, 'tcx>( diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 60eb30e075339..586a3fdb907ee 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -440,13 +440,7 @@ impl<'tcx> Constructor<'tcx> { assert!(!adt.is_enum()); VariantIdx::new(0) } - &ConstantValue(c) => { - crate::const_eval::const_variant_index( - cx.tcx, - cx.param_env, - c, - ).unwrap() - }, + &ConstantValue(c) => crate::const_eval::const_variant_index(cx.tcx, cx.param_env, c), _ => bug!("bad constructor {:?} for adt {:?}", self, adt) } } diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index d5f2e7a7275e8..ab54d4c50b5ee 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -937,10 +937,7 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { debug!("const_to_pat: cv={:#?} id={:?}", cv, id); let adt_subpattern = |i, variant_opt| { let field = Field::new(i); - let val = const_field( - self.tcx, self.param_env, - variant_opt, field, cv, - ).expect("field access failed"); + let val = const_field(self.tcx, self.param_env, variant_opt, field, cv); self.const_to_pat(instance, val, id, span) }; let adt_subpatterns = |n, variant_opt| { @@ -979,9 +976,7 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { PatternKind::Wild } ty::Adt(adt_def, substs) if adt_def.is_enum() => { - let variant_index = const_variant_index( - self.tcx, self.param_env, cv - ).expect("const_variant_index failed"); + let variant_index = const_variant_index(self.tcx, self.param_env, cv); let subpatterns = adt_subpatterns( adt_def.variants[variant_index].fields.len(), Some(variant_index), From 538a0963ff4a55da47fcf2a1e14c62edab5af48d Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Mon, 4 Mar 2019 15:12:45 -0800 Subject: [PATCH 33/54] Add as_slice() to slice::IterMut and vec::Drain In bluss/indexmap#88, we found that there was no easy way to implement `Debug` for our `IterMut` and `Drain` iterators. Those are built on `slice::IterMut` and `vec::Drain`, which implement `Debug` themselves, but have no other way to access their data. With a new `as_slice()` method, we can read the data and customize its presentation. --- src/liballoc/vec.rs | 19 +++++++++++++++++++ src/libcore/slice/mod.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 947ce354ae711..7c3cab77bfbbf 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -2468,6 +2468,25 @@ impl fmt::Debug for Drain<'_, T> { } } +impl<'a, T> Drain<'a, T> { + /// Returns the remaining items of this iterator as a slice. + /// + /// # Examples + /// + /// ``` + /// # #![feature(vec_drain_as_slice)] + /// let mut vec = vec!['a', 'b', 'c']; + /// let mut drain = vec.drain(..); + /// assert_eq!(drain.as_slice(), &['a', 'b', 'c']); + /// let _ = drain.next().unwrap(); + /// assert_eq!(drain.as_slice(), &['b', 'c']); + /// ``` + #[unstable(feature = "vec_drain_as_slice", reason = "recently added", issue = "0")] + pub fn as_slice(&self) -> &[T] { + self.iter.as_slice() + } +} + #[stable(feature = "drain", since = "1.6.0")] unsafe impl Sync for Drain<'_, T> {} #[stable(feature = "drain", since = "1.6.0")] diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 2063f8ffaf65a..b48101c23dad7 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -3288,6 +3288,38 @@ impl<'a, T> IterMut<'a, T> { pub fn into_slice(self) -> &'a mut [T] { unsafe { from_raw_parts_mut(self.ptr, len!(self)) } } + + /// Views the underlying data as a subslice of the original data. + /// + /// To avoid creating `&mut` references that alias, this has a + /// borrowed lifetime from the iterator. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ``` + /// # #![feature(slice_iter_mut_as_slice)] + /// // First, we declare a type which has `iter_mut` method to get the `IterMut` + /// // struct (&[usize here]): + /// let mut slice = &mut [1, 2, 3]; + /// + /// // Then, we get the iterator: + /// let mut iter = slice.iter_mut(); + /// // So if we print what `as_slice` method returns here, we have "[1, 2, 3]": + /// println!("{:?}", iter.as_slice()); + /// assert_eq!(iter.as_slice(), &[1, 2, 3]); + /// + /// // Next, we move to the second element of the slice: + /// iter.next(); + /// // Now `as_slice` returns "[2, 3]": + /// println!("{:?}", iter.as_slice()); + /// assert_eq!(iter.as_slice(), &[2, 3]); + /// ``` + #[unstable(feature = "slice_iter_mut_as_slice", reason = "recently added", issue = "0")] + pub fn as_slice(&self) -> &[T] { + self.make_slice() + } } iterator!{struct IterMut -> *mut T, &'a mut T, mut, {mut}, {}} From 9902f8c3c2e89c87ab25a543e12c851bef955608 Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Sat, 23 Feb 2019 16:11:34 +0530 Subject: [PATCH 34/54] fixes rust-lang#52482 --- src/librustc/ty/context.rs | 34 ++++++++++++------------- src/librustc_mir/hair/cx/expr.rs | 7 +---- src/librustc_passes/rvalue_promotion.rs | 17 +++++-------- src/librustc_typeck/check/cast.rs | 5 ++-- src/librustc_typeck/check/writeback.rs | 16 ++++-------- 5 files changed, 32 insertions(+), 47 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index b37b632f4beec..621a596118371 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -40,7 +40,7 @@ use crate::ty::steal::Steal; use crate::ty::subst::{UserSubsts, UnpackedKind}; use crate::ty::{BoundVar, BindingMode}; use crate::ty::CanonicalPolyFnSig; -use crate::util::nodemap::{DefIdMap, DefIdSet, ItemLocalMap}; +use crate::util::nodemap::{DefIdMap, DefIdSet, ItemLocalMap, ItemLocalSet}; use crate::util::nodemap::{FxHashMap, FxHashSet}; use errors::DiagnosticBuilder; use rustc_data_structures::interner::HashInterner; @@ -409,9 +409,9 @@ pub struct TypeckTables<'tcx> { /// MIR construction and hence is not serialized to metadata. fru_field_types: ItemLocalMap>>, - /// Maps a cast expression to its kind. This is keyed on the - /// *from* expression of the cast, not the cast itself. - cast_kinds: ItemLocalMap, + /// For every coercion cast we add the HIR node ID of the cast + /// expression to this set. + coercion_casts: ItemLocalSet, /// Set of trait imports actually used in the method resolution. /// This is used for warning unused imports. During type @@ -456,7 +456,7 @@ impl<'tcx> TypeckTables<'tcx> { closure_kind_origins: Default::default(), liberated_fn_sigs: Default::default(), fru_field_types: Default::default(), - cast_kinds: Default::default(), + coercion_casts: Default::default(), used_trait_imports: Lrc::new(Default::default()), tainted_by_errors: false, free_region_map: Default::default(), @@ -718,19 +718,19 @@ impl<'tcx> TypeckTables<'tcx> { } } - pub fn cast_kinds(&self) -> LocalTableInContext<'_, ty::cast::CastKind> { - LocalTableInContext { - local_id_root: self.local_id_root, - data: &self.cast_kinds - } + pub fn is_coercion_cast(&self, hir_id: hir::HirId) -> bool { + validate_hir_id_for_typeck_tables(self.local_id_root, hir_id, true); + self.coercion_casts.contains(&hir_id.local_id) } - pub fn cast_kinds_mut(&mut self) -> LocalTableInContextMut<'_, ty::cast::CastKind> { - LocalTableInContextMut { - local_id_root: self.local_id_root, - data: &mut self.cast_kinds - } + pub fn set_coercion_cast(&mut self, id: ItemLocalId) { + self.coercion_casts.insert(id); + } + + pub fn coercion_casts(&self) -> &ItemLocalSet { + &self.coercion_casts } + } impl<'a, 'gcx> HashStable> for TypeckTables<'gcx> { @@ -753,7 +753,7 @@ impl<'a, 'gcx> HashStable> for TypeckTables<'gcx> { ref liberated_fn_sigs, ref fru_field_types, - ref cast_kinds, + ref coercion_casts, ref used_trait_imports, tainted_by_errors, @@ -798,7 +798,7 @@ impl<'a, 'gcx> HashStable> for TypeckTables<'gcx> { closure_kind_origins.hash_stable(hcx, hasher); liberated_fn_sigs.hash_stable(hcx, hasher); fru_field_types.hash_stable(hcx, hasher); - cast_kinds.hash_stable(hcx, hasher); + coercion_casts.hash_stable(hcx, hasher); used_trait_imports.hash_stable(hcx, hasher); tainted_by_errors.hash_stable(hcx, hasher); free_region_map.hash_stable(hcx, hasher); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 5548366db66ec..ff5f3018acc08 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -8,7 +8,6 @@ use rustc::hir::def::{Def, CtorKind}; use rustc::mir::interpret::{GlobalId, ErrorHandled}; use rustc::ty::{self, AdtKind, Ty}; use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability}; -use rustc::ty::cast::CastKind as TyCastKind; use rustc::ty::subst::{InternalSubsts, SubstsRef}; use rustc::hir; use rustc::hir::def_id::LocalDefId; @@ -656,11 +655,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, // Check to see if this cast is a "coercion cast", where the cast is actually done // using a coercion (or is a no-op). - let cast = if let Some(&TyCastKind::CoercionCast) = - cx.tables() - .cast_kinds() - .get(source.hir_id) - { + let cast = if cx.tables().is_coercion_cast(source.hir_id) { // Convert the lexpr to a vexpr. ExprKind::Use { source: source.to_ref() } } else { diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs index 6b8e37b3b3133..b0dd72030cc51 100644 --- a/src/librustc_passes/rvalue_promotion.rs +++ b/src/librustc_passes/rvalue_promotion.rs @@ -14,7 +14,7 @@ // - It's not possible to take the address of a static item with unsafe interior. This is enforced // by borrowck::gather_loans -use rustc::ty::cast::CastKind; +use rustc::ty::cast::CastTy; use rustc::hir::def::{Def, CtorKind}; use rustc::hir::def_id::DefId; use rustc::middle::expr_use_visitor as euv; @@ -319,15 +319,12 @@ fn check_expr_kind<'a, 'tcx>( hir::ExprKind::Cast(ref from, _) => { let expr_promotability = v.check_expr(from); debug!("Checking const cast(id={})", from.hir_id); - match v.tables.cast_kinds().get(from.hir_id) { - None => { - v.tcx.sess.delay_span_bug(e.span, "no kind for cast"); - NotPromotable - }, - Some(&CastKind::PtrAddrCast) | Some(&CastKind::FnPtrAddrCast) => { - NotPromotable - } - _ => expr_promotability + let cast_in = CastTy::from_ty(v.tables.expr_ty(from)); + let cast_out = CastTy::from_ty(v.tables.expr_ty(e)); + match (cast_in, cast_out) { + (Some(CastTy::FnPtr), Some(CastTy::Int(_))) | + (Some(CastTy::Ptr(_)), Some(CastTy::Int(_))) => NotPromotable, + (_, _) => expr_promotability } } hir::ExprKind::Path(ref qpath) => { diff --git a/src/librustc_typeck/check/cast.rs b/src/librustc_typeck/check/cast.rs index 87276b8c66ca4..cad9e73bd2ac9 100644 --- a/src/librustc_typeck/check/cast.rs +++ b/src/librustc_typeck/check/cast.rs @@ -428,13 +428,12 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> { } else if self.try_coercion_cast(fcx) { self.trivial_cast_lint(fcx); debug!(" -> CoercionCast"); - fcx.tables.borrow_mut().cast_kinds_mut().insert(self.expr.hir_id, - CastKind::CoercionCast); + fcx.tables.borrow_mut().set_coercion_cast(self.expr.hir_id.local_id); + } else { match self.do_check(fcx) { Ok(k) => { debug!(" -> {:?}", k); - fcx.tables.borrow_mut().cast_kinds_mut().insert(self.expr.hir_id, k); } Err(e) => self.report_cast_error(fcx, e), }; diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 7c1283a6d2108..73eadf6f275dd 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -50,7 +50,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { wbcx.visit_liberated_fn_sigs(); wbcx.visit_fru_field_types(); wbcx.visit_opaque_types(body.value.span); - wbcx.visit_cast_types(); + wbcx.visit_coercion_casts(); wbcx.visit_free_region_map(); wbcx.visit_user_provided_tys(); wbcx.visit_user_provided_sigs(); @@ -355,19 +355,13 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { } } - fn visit_cast_types(&mut self) { + fn visit_coercion_casts(&mut self) { let fcx_tables = self.fcx.tables.borrow(); - let fcx_cast_kinds = fcx_tables.cast_kinds(); + let fcx_coercion_casts = fcx_tables.coercion_casts(); debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root); - let mut self_cast_kinds = self.tables.cast_kinds_mut(); - let common_local_id_root = fcx_tables.local_id_root.unwrap(); - for (&local_id, &cast_kind) in fcx_cast_kinds.iter() { - let hir_id = hir::HirId { - owner: common_local_id_root.index, - local_id, - }; - self_cast_kinds.insert(hir_id, cast_kind); + for local_id in fcx_coercion_casts { + self.tables.set_coercion_cast(*local_id); } } From e28cf7416207111308bdd0a91196d99b009f031c Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 5 Mar 2019 14:18:26 -0600 Subject: [PATCH 35/54] remove unused Display impl --- src/librustdoc/passes/calculate_doc_coverage.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 6e0238b7a4d5e..04f403888c1fb 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -7,7 +7,6 @@ use syntax::attr; use syntax_pos::FileName; use std::collections::BTreeMap; -use std::fmt; use std::ops; pub const CALCULATE_DOC_COVERAGE: Pass = Pass { @@ -67,12 +66,6 @@ impl ops::AddAssign for ItemCount { } } -impl fmt::Display for ItemCount { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}/{}", self.with_docs, self.total) - } -} - #[derive(Default)] struct CoverageCalculator { items: BTreeMap, From 3df0b895c1ba705bd4ecf110e3f6bdac7fe20953 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 5 Mar 2019 14:23:37 -0600 Subject: [PATCH 36/54] only print coverage pass lists if running on nightly --- src/librustdoc/config.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 5cbcc2433ba57..aeff78350d37c 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -228,14 +228,18 @@ impl Options { for &name in passes::DEFAULT_PRIVATE_PASSES { println!("{:>20}", name); } - println!("\nPasses run with `--show-coverage`:"); - for &name in passes::DEFAULT_COVERAGE_PASSES { - println!("{:>20}", name); - } - println!("\nPasses run with `--show-coverage --document-private-items`:"); - for &name in passes::PRIVATE_COVERAGE_PASSES { - println!("{:>20}", name); + + if nightly_options::is_nightly_build() { + println!("\nPasses run with `--show-coverage`:"); + for &name in passes::DEFAULT_COVERAGE_PASSES { + println!("{:>20}", name); + } + println!("\nPasses run with `--show-coverage --document-private-items`:"); + for &name in passes::PRIVATE_COVERAGE_PASSES { + println!("{:>20}", name); + } } + return Err(0); } From 5384a11fcaa71d74d93f7e4ea4a2662ae28698fe Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 5 Mar 2019 16:17:50 -0800 Subject: [PATCH 37/54] Apply suggestions from code review Co-Authored-By: cuviper --- src/libcore/slice/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index b48101c23dad7..4bccd44f5036a 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -3291,8 +3291,8 @@ impl<'a, T> IterMut<'a, T> { /// Views the underlying data as a subslice of the original data. /// - /// To avoid creating `&mut` references that alias, this has a - /// borrowed lifetime from the iterator. + /// To avoid creating `&mut [T]` references that alias, the returned slice + /// borrows its lifetime from the iterator the method is applied on. /// /// # Examples /// @@ -3302,11 +3302,11 @@ impl<'a, T> IterMut<'a, T> { /// # #![feature(slice_iter_mut_as_slice)] /// // First, we declare a type which has `iter_mut` method to get the `IterMut` /// // struct (&[usize here]): - /// let mut slice = &mut [1, 2, 3]; + /// let mut slice: &mut [usize] = &mut [1, 2, 3]; /// /// // Then, we get the iterator: /// let mut iter = slice.iter_mut(); - /// // So if we print what `as_slice` method returns here, we have "[1, 2, 3]": + /// // So if we print what the `as_slice` method returns here, we have "[1, 2, 3]": /// println!("{:?}", iter.as_slice()); /// assert_eq!(iter.as_slice(), &[1, 2, 3]); /// From 51e0d1c2990619ed55e0a387ae4fab71992069df Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 5 Mar 2019 16:20:50 -0800 Subject: [PATCH 38/54] Clean up the example on slice::IterMut::as_slice() --- src/libcore/slice/mod.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 4bccd44f5036a..5af6b9e8fe632 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -3300,20 +3300,16 @@ impl<'a, T> IterMut<'a, T> { /// /// ``` /// # #![feature(slice_iter_mut_as_slice)] - /// // First, we declare a type which has `iter_mut` method to get the `IterMut` - /// // struct (&[usize here]): /// let mut slice: &mut [usize] = &mut [1, 2, 3]; /// - /// // Then, we get the iterator: + /// // First, we get the iterator: /// let mut iter = slice.iter_mut(); - /// // So if we print what the `as_slice` method returns here, we have "[1, 2, 3]": - /// println!("{:?}", iter.as_slice()); + /// // So if we check what the `as_slice` method returns here, we have "[1, 2, 3]": /// assert_eq!(iter.as_slice(), &[1, 2, 3]); /// /// // Next, we move to the second element of the slice: /// iter.next(); /// // Now `as_slice` returns "[2, 3]": - /// println!("{:?}", iter.as_slice()); /// assert_eq!(iter.as_slice(), &[2, 3]); /// ``` #[unstable(feature = "slice_iter_mut_as_slice", reason = "recently added", issue = "0")] From e478cadbbe51e335b7248018141877b88770fe68 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 5 Mar 2019 16:28:32 -0800 Subject: [PATCH 39/54] Add a tracking issue for new as_slice methods --- src/liballoc/vec.rs | 2 +- src/libcore/slice/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 7c3cab77bfbbf..adcd3d84f4832 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -2481,7 +2481,7 @@ impl<'a, T> Drain<'a, T> { /// let _ = drain.next().unwrap(); /// assert_eq!(drain.as_slice(), &['b', 'c']); /// ``` - #[unstable(feature = "vec_drain_as_slice", reason = "recently added", issue = "0")] + #[unstable(feature = "vec_drain_as_slice", reason = "recently added", issue = "58957")] pub fn as_slice(&self) -> &[T] { self.iter.as_slice() } diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 5af6b9e8fe632..b3594f8a3858a 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -3312,7 +3312,7 @@ impl<'a, T> IterMut<'a, T> { /// // Now `as_slice` returns "[2, 3]": /// assert_eq!(iter.as_slice(), &[2, 3]); /// ``` - #[unstable(feature = "slice_iter_mut_as_slice", reason = "recently added", issue = "0")] + #[unstable(feature = "slice_iter_mut_as_slice", reason = "recently added", issue = "58957")] pub fn as_slice(&self) -> &[T] { self.make_slice() } From d5bb71c9f1042c0c931c7dddecc418233de6e30f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 23 Feb 2019 16:40:15 +0100 Subject: [PATCH 40/54] Split up privacy checking so privacy_access_levels only does computations required for AccessLevels --- src/librustc/dep_graph/dep_node.rs | 1 + src/librustc/ty/query/config.rs | 6 ++ src/librustc/ty/query/mod.rs | 3 +- src/librustc/ty/query/plumbing.rs | 1 + src/librustc_interface/passes.rs | 29 ++++--- src/librustc_privacy/lib.rs | 78 +++++++++---------- .../ui/privacy/private-inferred-type.stderr | 36 ++++----- 7 files changed, 84 insertions(+), 70 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index c607eb5906e60..a46ffffe94cb8 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -456,6 +456,7 @@ define_dep_nodes!( <'tcx> [eval_always] CoherenceInherentImplOverlapCheck, [] CoherenceCheckTrait(DefId), [eval_always] PrivacyAccessLevels(CrateNum), + [eval_always] CheckPrivacy(CrateNum), [eval_always] Analysis(CrateNum), // Represents the MIR for a fn; also used as the task node for diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index feca0f7170ef3..0fef90c945e94 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -369,6 +369,12 @@ impl<'tcx> QueryDescription<'tcx> for queries::privacy_access_levels<'tcx> { } } +impl<'tcx> QueryDescription<'tcx> for queries::check_privacy<'tcx> { + fn describe(_: TyCtxt<'_, '_, '_>, _: CrateNum) -> Cow<'static, str> { + "privacy checking".into() + } +} + impl<'tcx> QueryDescription<'tcx> for queries::typeck_item_bodies<'tcx> { fn describe(_: TyCtxt<'_, '_, '_>, _: CrateNum) -> Cow<'static, str> { "type-checking all item bodies".into() diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 197b9a71b0ac0..cb72ad6fe8bd9 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -350,8 +350,9 @@ define_queries! { <'tcx> [] fn check_match: CheckMatch(DefId) -> Result<(), ErrorReported>, - /// Performs the privacy check and computes "access levels". + /// Performs part of the privacy check and computes "access levels". [] fn privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Lrc, + [] fn check_privacy: CheckPrivacy(CrateNum) -> (), }, Other { diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index ebaa31d703f8e..ff2639da13650 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1251,6 +1251,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, force!(crate_inherent_impls_overlap_check, LOCAL_CRATE) }, DepKind::PrivacyAccessLevels => { force!(privacy_access_levels, LOCAL_CRATE); } + DepKind::CheckPrivacy => { force!(check_privacy, LOCAL_CRATE); } DepKind::MirBuilt => { force!(mir_built, def_id!()); } DepKind::MirConstQualif => { force!(mir_const_qualif, def_id!()); } DepKind::MirConst => { force!(mir_const, def_id!()); } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 16ced6956380b..591583a1526d0 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -21,7 +21,7 @@ use rustc_borrowck as borrowck; use rustc_codegen_utils::codegen_backend::CodegenBackend; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::stable_hasher::StableHasher; -use rustc_data_structures::sync::Lrc; +use rustc_data_structures::sync::{Lrc, ParallelIterator, par_iter}; use rustc_incremental; use rustc_metadata::creader::CrateLoader; use rustc_metadata::cstore::{self, CStore}; @@ -278,17 +278,28 @@ fn analysis<'tcx>( time(sess, "misc checking", || { parallel!({ - time(sess, "privacy checking", || { - rustc_privacy::check_crate(tcx) + time(sess, "privacy access levels", || { + tcx.ensure().privacy_access_levels(LOCAL_CRATE); }); - }, { - time(sess, "death checking", || middle::dead::check_crate(tcx)); - }, { - time(sess, "unused lib feature checking", || { - stability::check_unused_or_stable_features(tcx) + parallel!({ + time(sess, "privacy checking", || { + tcx.ensure().check_privacy(LOCAL_CRATE); + }); + }, { + time(sess, "death checking", || middle::dead::check_crate(tcx)); + }, { + time(sess, "unused lib feature checking", || { + stability::check_unused_or_stable_features(tcx) + }); + }, { + time(sess, "lint checking", || lint::check_crate(tcx)); }); }, { - time(sess, "lint checking", || lint::check_crate(tcx)); + time(sess, "privacy checking modules", || { + par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { + tcx.ensure().check_mod_privacy(tcx.hir().local_def_id(module)); + }); + }); }); }); diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 550b333700b04..372a923069424 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1760,19 +1760,15 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> pub fn provide(providers: &mut Providers<'_>) { *providers = Providers { privacy_access_levels, + check_privacy, check_mod_privacy, ..*providers }; } -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Lrc { - tcx.privacy_access_levels(LOCAL_CRATE) -} - fn check_mod_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { let empty_tables = ty::TypeckTables::empty(None); - // Check privacy of names not checked in previous compilation stages. let mut visitor = NamePrivacyVisitor { tcx, @@ -1803,18 +1799,6 @@ fn privacy_access_levels<'tcx>( ) -> Lrc { assert_eq!(krate, LOCAL_CRATE); - let krate = tcx.hir().krate(); - - for &module in krate.modules.keys() { - tcx.ensure().check_mod_privacy(tcx.hir().local_def_id(module)); - } - - let private_crates: FxHashSet = tcx.sess.opts.extern_private.iter() - .flat_map(|c| { - tcx.crates().iter().find(|&&krate| &tcx.crate_name(krate) == c).cloned() - }).collect(); - - // Build up a set of all exported items in the AST. This is a set of all // items which are reachable from external crates based on visibility. let mut visitor = EmbargoVisitor { @@ -1824,7 +1808,7 @@ fn privacy_access_levels<'tcx>( changed: false, }; loop { - intravisit::walk_crate(&mut visitor, krate); + intravisit::walk_crate(&mut visitor, tcx.hir().krate()); if visitor.changed { visitor.changed = false; } else { @@ -1833,36 +1817,46 @@ fn privacy_access_levels<'tcx>( } visitor.update(hir::CRATE_HIR_ID, Some(AccessLevel::Public)); - { - let mut visitor = ObsoleteVisiblePrivateTypesVisitor { - tcx, - access_levels: &visitor.access_levels, - in_variant: false, - old_error_set: Default::default(), - }; - intravisit::walk_crate(&mut visitor, krate); + Lrc::new(visitor.access_levels) +} +fn check_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, krate: CrateNum) { + assert_eq!(krate, LOCAL_CRATE); - let has_pub_restricted = { - let mut pub_restricted_visitor = PubRestrictedVisitor { - tcx, - has_pub_restricted: false - }; - intravisit::walk_crate(&mut pub_restricted_visitor, krate); - pub_restricted_visitor.has_pub_restricted - }; + let access_levels = tcx.privacy_access_levels(LOCAL_CRATE); - // Check for private types and traits in public interfaces. - let mut visitor = PrivateItemsInPublicInterfacesVisitor { + let krate = tcx.hir().krate(); + + let mut visitor = ObsoleteVisiblePrivateTypesVisitor { + tcx, + access_levels: &access_levels, + in_variant: false, + old_error_set: Default::default(), + }; + intravisit::walk_crate(&mut visitor, krate); + + let has_pub_restricted = { + let mut pub_restricted_visitor = PubRestrictedVisitor { tcx, - has_pub_restricted, - old_error_set: &visitor.old_error_set, - private_crates + has_pub_restricted: false }; - krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); - } + intravisit::walk_crate(&mut pub_restricted_visitor, krate); + pub_restricted_visitor.has_pub_restricted + }; - Lrc::new(visitor.access_levels) + let private_crates: FxHashSet = tcx.sess.opts.extern_private.iter() + .flat_map(|c| { + tcx.crates().iter().find(|&&krate| &tcx.crate_name(krate) == c).cloned() + }).collect(); + + // Check for private types and traits in public interfaces. + let mut visitor = PrivateItemsInPublicInterfacesVisitor { + tcx, + has_pub_restricted, + old_error_set: &visitor.old_error_set, + private_crates + }; + krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); } __build_diagnostic_array! { librustc_privacy, DIAGNOSTICS } diff --git a/src/test/ui/privacy/private-inferred-type.stderr b/src/test/ui/privacy/private-inferred-type.stderr index 80a475f7dceea..568d4dadc8cc4 100644 --- a/src/test/ui/privacy/private-inferred-type.stderr +++ b/src/test/ui/privacy/private-inferred-type.stderr @@ -1,3 +1,21 @@ +error[E0446]: private type `m::Priv` in public interface + --> $DIR/private-inferred-type.rs:61:36 + | +LL | struct Priv; + | - `m::Priv` declared as private +... +LL | impl TraitWithAssocTy for u8 { type AssocTy = Priv; } + | ^^^^^^^^^^^^^^^^^^^^ can't leak private type + +error[E0446]: private type `adjust::S2` in public interface + --> $DIR/private-inferred-type.rs:83:9 + | +LL | struct S2; + | - `adjust::S2` declared as private +... +LL | type Target = S2Alias; //~ ERROR private type `adjust::S2` in public interface + | ^^^^^^^^^^^^^^^^^^^^^^ can't leak private type + error: type `m::Priv` is private --> $DIR/private-inferred-type.rs:97:9 | @@ -202,24 +220,6 @@ error: type `m::Priv` is private LL | match a { //~ ERROR type `m::Priv` is private | ^ -error[E0446]: private type `m::Priv` in public interface - --> $DIR/private-inferred-type.rs:61:36 - | -LL | struct Priv; - | - `m::Priv` declared as private -... -LL | impl TraitWithAssocTy for u8 { type AssocTy = Priv; } - | ^^^^^^^^^^^^^^^^^^^^ can't leak private type - -error[E0446]: private type `adjust::S2` in public interface - --> $DIR/private-inferred-type.rs:83:9 - | -LL | struct S2; - | - `adjust::S2` declared as private -... -LL | type Target = S2Alias; //~ ERROR private type `adjust::S2` in public interface - | ^^^^^^^^^^^^^^^^^^^^^^ can't leak private type - error: aborting due to 33 previous errors For more information about this error, try `rustc --explain E0446`. From d2923e5a77b318300c9d35d63d594125b8b9a43f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 23 Feb 2019 16:47:13 +0100 Subject: [PATCH 41/54] Run the first block in a parallel! macro directly in the scope which guarantees that it will run immediately --- src/librustc_data_structures/sync.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustc_data_structures/sync.rs b/src/librustc_data_structures/sync.rs index ba1f6eb56fe88..14e625af29923 100644 --- a/src/librustc_data_structures/sync.rs +++ b/src/librustc_data_structures/sync.rs @@ -280,21 +280,22 @@ cfg_if! { #[macro_export] macro_rules! parallel { - (impl [$($c:tt,)*] [$block:tt $(, $rest:tt)*]) => { - parallel!(impl [$block, $($c,)*] [$($rest),*]) + (impl $fblock:tt [$($c:tt,)*] [$block:tt $(, $rest:tt)*]) => { + parallel!(impl $fblock [$block, $($c,)*] [$($rest),*]) }; - (impl [$($blocks:tt,)*] []) => { + (impl $fblock:tt [$($blocks:tt,)*] []) => { ::rustc_data_structures::sync::scope(|s| { $( s.spawn(|_| $blocks); )* + $fblock; }) }; - ($($blocks:tt),*) => { + ($fblock:tt, $($blocks:tt),*) => { // Reverse the order of the blocks since Rayon executes them in reverse order // when using a single thread. This ensures the execution order matches that // of a single threaded rustc - parallel!(impl [] [$($blocks),*]); + parallel!(impl $fblock [] [$($blocks),*]); }; } From 1745957d637d86f111f4aa96e47f68a7433f0e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 23 Feb 2019 18:12:38 +0100 Subject: [PATCH 42/54] Make misc checking 2 more parallel --- src/librustc/middle/intrinsicck.rs | 6 ---- src/librustc/middle/liveness.rs | 6 ---- src/librustc_interface/passes.rs | 33 ++++++++++---------- src/librustc_mir/hair/pattern/check_match.rs | 7 ----- src/librustc_mir/hair/pattern/mod.rs | 1 - src/librustc_mir/lib.rs | 1 - src/librustc_passes/rvalue_promotion.rs | 7 ----- 7 files changed, 17 insertions(+), 44 deletions(-) diff --git a/src/librustc/middle/intrinsicck.rs b/src/librustc/middle/intrinsicck.rs index ce20ca39533b1..c4071e9f354b1 100644 --- a/src/librustc/middle/intrinsicck.rs +++ b/src/librustc/middle/intrinsicck.rs @@ -10,12 +10,6 @@ use syntax_pos::Span; use crate::hir::intravisit::{self, Visitor, NestedVisitorMap}; use crate::hir; -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - for &module in tcx.hir().krate().modules.keys() { - tcx.ensure().check_mod_intrinsics(tcx.hir().local_def_id(module)); - } -} - fn check_mod_intrinsics<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { tcx.hir().visit_item_likes_in_module( module_def_id, diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 76933a6e3484b..031d6dec090af 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -185,12 +185,6 @@ fn check_mod_liveness<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { tcx.hir().visit_item_likes_in_module(module_def_id, &mut IrMaps::new(tcx).as_deep_visitor()); } -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - for &module in tcx.hir().krate().modules.keys() { - tcx.ensure().check_mod_liveness(tcx.hir().local_def_id(module)); - } -} - pub fn provide(providers: &mut Providers<'_>) { *providers = Providers { check_mod_liveness, diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 591583a1526d0..cf4a3ecf555b8 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -218,24 +218,25 @@ fn analysis<'tcx>( // passes are timed inside typeck typeck::check_crate(tcx)?; - time(sess, "misc checking", || { + time(sess, "misc checking 2", || { parallel!({ - time(sess, "rvalue promotion", || { - rvalue_promotion::check_crate(tcx) - }); - }, { - time(sess, "intrinsic checking", || { - middle::intrinsicck::check_crate(tcx) + time(sess, "rvalue promotion + match checking", || { + tcx.par_body_owners(|def_id| { + tcx.ensure().const_is_rvalue_promotable_to_static(def_id); + tcx.ensure().check_match(def_id); + }); }); }, { - time(sess, "match checking", || mir::matchck_crate(tcx)); - }, { - // this must run before MIR dump, because - // "not all control paths return a value" is reported here. - // - // maybe move the check to a MIR pass? - time(sess, "liveness checking", || { - middle::liveness::check_crate(tcx) + time(sess, "liveness checking + intrinsic checking", || { + par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { + // this must run before MIR dump, because + // "not all control paths return a value" is reported here. + // + // maybe move the check to a MIR pass? + tcx.ensure().check_mod_liveness(tcx.hir().local_def_id(module)); + + tcx.ensure().check_mod_intrinsics(tcx.hir().local_def_id(module)); + }); }); }); }); @@ -276,7 +277,7 @@ fn analysis<'tcx>( return Err(ErrorReported); } - time(sess, "misc checking", || { + time(sess, "misc checking 3", || { parallel!({ time(sess, "privacy access levels", || { tcx.ensure().privacy_access_levels(LOCAL_CRATE); diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 9af513a90905e..787dba15f4da1 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -27,13 +27,6 @@ use std::slice; use syntax::ptr::P; use syntax_pos::{Span, DUMMY_SP, MultiSpan}; -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - for def_id in tcx.body_owners() { - tcx.ensure().check_match(def_id); - } - tcx.sess.abort_if_errors(); -} - pub(crate) fn check_match<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId, diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index d5f2e7a7275e8..4788454b86ab8 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -3,7 +3,6 @@ mod _match; mod check_match; -pub use self::check_match::check_crate; pub(crate) use self::check_match::check_match; use crate::const_eval::{const_field, const_variant_index}; diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index 06e79dc4e7097..0b735b4b39cf5 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -54,7 +54,6 @@ pub mod interpret; pub mod monomorphize; pub mod const_eval; -pub use hair::pattern::check_crate as matchck_crate; use rustc::ty::query::Providers; pub fn provide(providers: &mut Providers<'_>) { diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs index edd658254467f..a059ab40697bf 100644 --- a/src/librustc_passes/rvalue_promotion.rs +++ b/src/librustc_passes/rvalue_promotion.rs @@ -39,13 +39,6 @@ pub fn provide(providers: &mut Providers<'_>) { }; } -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - for &body_id in &tcx.hir().krate().body_ids { - let def_id = tcx.hir().body_owner_def_id(body_id); - tcx.const_is_rvalue_promotable_to_static(def_id); - } -} - fn const_is_rvalue_promotable_to_static<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> bool From 140a837fb647c5554102ec28e8713a913224cdbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 23 Feb 2019 18:32:45 +0100 Subject: [PATCH 43/54] Make misc checking 1 more parallel --- src/librustc/hir/check_attr.rs | 6 ------ src/librustc/middle/stability.rs | 6 ------ src/librustc_interface/passes.rs | 36 +++++++++++++++----------------- src/librustc_passes/loops.rs | 6 ------ 4 files changed, 17 insertions(+), 37 deletions(-) diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index 8b304007a3572..86f7e14996488 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -344,12 +344,6 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckAttrVisitor<'a, 'tcx> { } } -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - for &module in tcx.hir().krate().modules.keys() { - tcx.ensure().check_mod_attrs(tcx.hir().local_def_id(module)); - } -} - fn is_c_like_enum(item: &hir::Item) -> bool { if let hir::ItemKind::Enum(ref def, _) = item.node { for variant in &def.variants { diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index aa23924486165..1677384059e09 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -456,12 +456,6 @@ impl<'a, 'tcx> Index<'tcx> { } } -pub fn check_unstable_api_usage<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - for &module in tcx.hir().krate().modules.keys() { - tcx.ensure().check_mod_unstable_api_usage(tcx.hir().local_def_id(module)); - } -} - /// Cross-references the feature names of unstable APIs with enabled /// features and possibly prints errors. fn check_mod_unstable_api_usage<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index cf4a3ecf555b8..2066747a6ffb6 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -191,27 +191,25 @@ fn analysis<'tcx>( let sess = tcx.sess; - parallel!({ - time(sess, "looking for entry point", || { - middle::entry::find_entry_point(tcx) - }); + time(sess, "misc checking 1", || { + parallel!({ + time(sess, "looking for entry point", || { + middle::entry::find_entry_point(tcx) + }); - time(sess, "looking for plugin registrar", || { - plugin::build::find_plugin_registrar(tcx) - }); + time(sess, "looking for plugin registrar", || { + plugin::build::find_plugin_registrar(tcx) + }); - time(sess, "looking for derive registrar", || { - proc_macro_decls::find(tcx) - }); - }, { - time(sess, "loop checking", || loops::check_crate(tcx)); - }, { - time(sess, "attribute checking", || { - hir::check_attr::check_crate(tcx) - }); - }, { - time(sess, "stability checking", || { - stability::check_unstable_api_usage(tcx) + time(sess, "looking for derive registrar", || { + proc_macro_decls::find(tcx) + }); + }, { + par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { + tcx.ensure().check_mod_loops(tcx.hir().local_def_id(module)); + tcx.ensure().check_mod_attrs(tcx.hir().local_def_id(module)); + tcx.ensure().check_mod_unstable_api_usage(tcx.hir().local_def_id(module)); + }); }); }); diff --git a/src/librustc_passes/loops.rs b/src/librustc_passes/loops.rs index 533e043efa9d2..fa7cb69fcf7d9 100644 --- a/src/librustc_passes/loops.rs +++ b/src/librustc_passes/loops.rs @@ -46,12 +46,6 @@ struct CheckLoopVisitor<'a, 'hir: 'a> { cx: Context, } -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - for &module in tcx.hir().krate().modules.keys() { - tcx.ensure().check_mod_loops(tcx.hir().local_def_id(module)); - } -} - fn check_mod_loops<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { tcx.hir().visit_item_likes_in_module(module_def_id, &mut CheckLoopVisitor { sess: &tcx.sess, From 350f72fc671c197573585cc6919fc99b182a1f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 23 Feb 2019 19:18:14 +0100 Subject: [PATCH 44/54] Make wf checking parallel --- src/librustc_typeck/check/mod.rs | 14 +------------- src/librustc_typeck/check/wfcheck.rs | 10 +++++----- src/librustc_typeck/coherence/mod.rs | 4 +--- src/librustc_typeck/collect.rs | 6 ------ src/librustc_typeck/lib.rs | 17 +++++++++++++---- 5 files changed, 20 insertions(+), 31 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 183667e224462..3d95e29082ebe 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -695,15 +695,7 @@ impl<'a, 'tcx> ItemLikeVisitor<'tcx> for CheckItemTypesVisitor<'a, 'tcx> { pub fn check_wf_new<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Result<(), ErrorReported> { tcx.sess.track_errors(|| { let mut visit = wfcheck::CheckTypeWellFormedVisitor::new(tcx); - tcx.hir().krate().visit_all_item_likes(&mut visit); - }) -} - -pub fn check_item_types<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Result<(), ErrorReported> { - tcx.sess.track_errors(|| { - for &module in tcx.hir().krate().modules.keys() { - tcx.ensure().check_mod_item_types(tcx.hir().local_def_id(module)); - } + tcx.hir().krate().par_visit_all_item_likes(&mut visit); }) } @@ -711,10 +703,6 @@ fn check_mod_item_types<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) tcx.hir().visit_item_likes_in_module(module_def_id, &mut CheckItemTypesVisitor { tcx }); } -pub fn check_item_bodies<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Result<(), ErrorReported> { - tcx.typeck_item_bodies(LOCAL_CRATE) -} - fn typeck_item_bodies<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum) -> Result<(), ErrorReported> { diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index 860fa526a1b91..4103b73151be5 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -14,7 +14,7 @@ use syntax::feature_gate::{self, GateIssue}; use syntax_pos::Span; use errors::{DiagnosticBuilder, DiagnosticId}; -use rustc::hir::itemlikevisit::ItemLikeVisitor; +use rustc::hir::itemlikevisit::ParItemLikeVisitor; use rustc::hir; /// Helper type of a temporary returned by `.for_item(...)`. @@ -1015,20 +1015,20 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> { } } -impl<'a, 'tcx> ItemLikeVisitor<'tcx> for CheckTypeWellFormedVisitor<'a, 'tcx> { - fn visit_item(&mut self, i: &'tcx hir::Item) { +impl<'a, 'tcx> ParItemLikeVisitor<'tcx> for CheckTypeWellFormedVisitor<'a, 'tcx> { + fn visit_item(&self, i: &'tcx hir::Item) { debug!("visit_item: {:?}", i); let def_id = self.tcx.hir().local_def_id_from_hir_id(i.hir_id); self.tcx.ensure().check_item_well_formed(def_id); } - fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem) { + fn visit_trait_item(&self, trait_item: &'tcx hir::TraitItem) { debug!("visit_trait_item: {:?}", trait_item); let def_id = self.tcx.hir().local_def_id_from_hir_id(trait_item.hir_id); self.tcx.ensure().check_trait_item_well_formed(def_id); } - fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem) { + fn visit_impl_item(&self, impl_item: &'tcx hir::ImplItem) { debug!("visit_impl_item: {:?}", impl_item); let def_id = self.tcx.hir().local_def_id_from_hir_id(impl_item.hir_id); self.tcx.ensure().check_impl_item_well_formed(def_id); diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 01aba658850bf..39a2f5d37bd7a 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -141,9 +141,7 @@ fn coherent_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) { for &impl_id in impls { check_impl_overlap(tcx, impl_id); } - use rustc::util::common::time; - time(tcx.sess, "builtin::check_trait checking", || - builtin::check_trait(tcx, def_id)); + builtin::check_trait(tcx, def_id); } pub fn check_coherence<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 594e29ab9ddea..30035094ccd29 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -56,12 +56,6 @@ struct OnlySelfBounds(bool); /////////////////////////////////////////////////////////////////////////// // Main entry point -pub fn collect_item_types<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - for &module in tcx.hir().krate().modules.keys() { - tcx.ensure().collect_mod_item_types(tcx.hir().local_def_id(module)); - } -} - fn collect_mod_item_types<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { tcx.hir().visit_item_likes_in_module( module_def_id, diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index a918113b1fc0b..ebb617c23c6ca 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -322,8 +322,11 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) // this ensures that later parts of type checking can assume that items // have valid types and not error tcx.sess.track_errors(|| { - time(tcx.sess, "type collecting", || - collect::collect_item_types(tcx)); + time(tcx.sess, "type collecting", || { + for &module in tcx.hir().krate().modules.keys() { + tcx.ensure().collect_mod_item_types(tcx.hir().local_def_id(module)); + } + }); })?; if tcx.features().rustc_attrs { @@ -352,9 +355,15 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) time(tcx.sess, "wf checking", || check::check_wf_new(tcx))?; - time(tcx.sess, "item-types checking", || check::check_item_types(tcx))?; + time(tcx.sess, "item-types checking", || { + tcx.sess.track_errors(|| { + for &module in tcx.hir().krate().modules.keys() { + tcx.ensure().check_mod_item_types(tcx.hir().local_def_id(module)); + } + }) + })?; - time(tcx.sess, "item-bodies checking", || check::check_item_bodies(tcx))?; + time(tcx.sess, "item-bodies checking", || tcx.typeck_item_bodies(LOCAL_CRATE))?; check_unused::check_crate(tcx); check_for_entry_fn(tcx); From 01f7450ae40694a22ad5ba5df4a2d770ae9f6702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 23 Feb 2019 23:25:30 +0100 Subject: [PATCH 45/54] Update tests --- src/test/ui/issues/issue-20413.rs | 1 + src/test/ui/issues/issue-20413.stderr | 82 ++++++++++++++++++++++++- src/test/ui/issues/issue-21946.rs | 1 + src/test/ui/issues/issue-21946.stderr | 8 ++- src/test/ui/issues/issue-23122-1.rs | 1 + src/test/ui/issues/issue-23122-1.stderr | 8 ++- src/test/ui/issues/issue-23122-2.rs | 1 + src/test/ui/issues/issue-23122-2.stderr | 11 +++- 8 files changed, 109 insertions(+), 4 deletions(-) diff --git a/src/test/ui/issues/issue-20413.rs b/src/test/ui/issues/issue-20413.rs index 34094fe6a44e2..7eb6d5c0ecbaa 100644 --- a/src/test/ui/issues/issue-20413.rs +++ b/src/test/ui/issues/issue-20413.rs @@ -8,6 +8,7 @@ struct NoData; impl Foo for T where NoData: Foo { //~^ ERROR: overflow evaluating the requirement fn answer(self) { + //~^ ERROR: overflow evaluating the requirement let val: NoData = NoData; } } diff --git a/src/test/ui/issues/issue-20413.stderr b/src/test/ui/issues/issue-20413.stderr index 1c353fec8aabf..db746bebbe273 100644 --- a/src/test/ui/issues/issue-20413.stderr +++ b/src/test/ui/issues/issue-20413.stderr @@ -12,6 +12,7 @@ error[E0275]: overflow evaluating the requirement `NoData Foo for T where NoData: Foo { LL | | //~^ ERROR: overflow evaluating the requirement LL | | fn answer(self) { +LL | | //~^ ERROR: overflow evaluating the requirement LL | | let val: NoData = NoData; LL | | } LL | | } @@ -87,7 +88,86 @@ note: required by `Foo` LL | trait Foo { | ^^^^^^^^^ -error: aborting due to 2 previous errors +error[E0275]: overflow evaluating the requirement `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>: Foo` + --> $DIR/issue-20413.rs:10:3 + | +LL | / fn answer(self) { +LL | | //~^ ERROR: overflow evaluating the requirement +LL | | let val: NoData = NoData; +LL | | } + | |___^ + | + = help: consider adding a `#![recursion_limit="128"]` attribute to your crate + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>>` + = note: required because of the requirements on the impl of `Foo` for `NoData>` + = note: required because of the requirements on the impl of `Foo` for `NoData` +note: required by `Foo` + --> $DIR/issue-20413.rs:1:1 + | +LL | trait Foo { + | ^^^^^^^^^ + +error: aborting due to 3 previous errors Some errors occurred: E0275, E0392. For more information about an error, try `rustc --explain E0275`. diff --git a/src/test/ui/issues/issue-21946.rs b/src/test/ui/issues/issue-21946.rs index d7a6c656df98f..2d99769cfa31c 100644 --- a/src/test/ui/issues/issue-21946.rs +++ b/src/test/ui/issues/issue-21946.rs @@ -7,6 +7,7 @@ struct FooStruct; impl Foo for FooStruct { //~^ ERROR overflow evaluating the requirement `::A` type A = ::A; + //~^ ERROR overflow evaluating the requirement `::A` } fn main() {} diff --git a/src/test/ui/issues/issue-21946.stderr b/src/test/ui/issues/issue-21946.stderr index 7a178bee6ae03..5ac49f61543e4 100644 --- a/src/test/ui/issues/issue-21946.stderr +++ b/src/test/ui/issues/issue-21946.stderr @@ -4,6 +4,12 @@ error[E0275]: overflow evaluating the requirement `::A` LL | impl Foo for FooStruct { | ^^^ -error: aborting due to previous error +error[E0275]: overflow evaluating the requirement `::A` + --> $DIR/issue-21946.rs:9:5 + | +LL | type A = ::A; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0275`. diff --git a/src/test/ui/issues/issue-23122-1.rs b/src/test/ui/issues/issue-23122-1.rs index a882aa36af758..d6f64650f36bb 100644 --- a/src/test/ui/issues/issue-23122-1.rs +++ b/src/test/ui/issues/issue-23122-1.rs @@ -7,6 +7,7 @@ struct GetNext { t: T } impl Next for GetNext { //~^ ERROR overflow evaluating the requirement type Next = as Next>::Next; + //~^ ERROR overflow evaluating the requirement } fn main() {} diff --git a/src/test/ui/issues/issue-23122-1.stderr b/src/test/ui/issues/issue-23122-1.stderr index 39dd424a86ca0..1b752b7afe2e6 100644 --- a/src/test/ui/issues/issue-23122-1.stderr +++ b/src/test/ui/issues/issue-23122-1.stderr @@ -4,6 +4,12 @@ error[E0275]: overflow evaluating the requirement ` as Next>::Next` LL | impl Next for GetNext { | ^^^^ -error: aborting due to previous error +error[E0275]: overflow evaluating the requirement ` as Next>::Next` + --> $DIR/issue-23122-1.rs:9:5 + | +LL | type Next = as Next>::Next; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0275`. diff --git a/src/test/ui/issues/issue-23122-2.rs b/src/test/ui/issues/issue-23122-2.rs index b841120034301..695712d2cc929 100644 --- a/src/test/ui/issues/issue-23122-2.rs +++ b/src/test/ui/issues/issue-23122-2.rs @@ -7,6 +7,7 @@ struct GetNext { t: T } impl Next for GetNext { //~^ ERROR overflow evaluating the requirement type Next = as Next>::Next; + //~^ ERROR overflow evaluating the requirement } fn main() {} diff --git a/src/test/ui/issues/issue-23122-2.stderr b/src/test/ui/issues/issue-23122-2.stderr index 8592877581103..b122dd42373c8 100644 --- a/src/test/ui/issues/issue-23122-2.stderr +++ b/src/test/ui/issues/issue-23122-2.stderr @@ -7,6 +7,15 @@ LL | impl Next for GetNext { = help: consider adding a `#![recursion_limit="128"]` attribute to your crate = note: required because of the requirements on the impl of `Next` for `GetNext<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next>` -error: aborting due to previous error +error[E0275]: overflow evaluating the requirement `<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next: std::marker::Sized` + --> $DIR/issue-23122-2.rs:9:5 + | +LL | type Next = as Next>::Next; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a `#![recursion_limit="128"]` attribute to your crate + = note: required because of the requirements on the impl of `Next` for `GetNext<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next>` + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0275`. From 7cc7b8f190565af501b0b4eda7be18f029a5d676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sun, 24 Feb 2019 22:37:55 +0100 Subject: [PATCH 46/54] Execute all parallel blocks even if they panic in a single-threaded compiler --- src/librustc/hir/mod.rs | 8 +++--- src/librustc_data_structures/sync.rs | 42 +++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 2d0296aa38c70..80d9c1e299847 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -30,7 +30,7 @@ use syntax::util::parser::ExprPrecedence; use crate::ty::AdtKind; use crate::ty::query::Providers; -use rustc_data_structures::sync::{ParallelIterator, par_iter, Send, Sync}; +use rustc_data_structures::sync::{par_for_each_in, Send, Sync}; use rustc_data_structures::thin_vec::ThinVec; use serialize::{self, Encoder, Encodable, Decoder, Decodable}; @@ -782,15 +782,15 @@ impl Crate { where V: itemlikevisit::ParItemLikeVisitor<'hir> + Sync + Send { parallel!({ - par_iter(&self.items).for_each(|(_, item)| { + par_for_each_in(&self.items, |(_, item)| { visitor.visit_item(item); }); }, { - par_iter(&self.trait_items).for_each(|(_, trait_item)| { + par_for_each_in(&self.trait_items, |(_, trait_item)| { visitor.visit_trait_item(trait_item); }); }, { - par_iter(&self.impl_items).for_each(|(_, impl_item)| { + par_for_each_in(&self.impl_items, |(_, impl_item)| { visitor.visit_impl_item(impl_item); }); }); diff --git a/src/librustc_data_structures/sync.rs b/src/librustc_data_structures/sync.rs index 14e625af29923..f006a95f3fa3c 100644 --- a/src/librustc_data_structures/sync.rs +++ b/src/librustc_data_structures/sync.rs @@ -65,6 +65,7 @@ cfg_if! { } use std::ops::Add; + use std::panic::{resume_unwind, catch_unwind, AssertUnwindSafe}; #[derive(Debug)] pub struct Atomic(Cell); @@ -130,7 +131,19 @@ cfg_if! { #[macro_export] macro_rules! parallel { ($($blocks:tt),*) => { - $($blocks)*; + let mut panic = None; + $( + if let Err(p) = ::std::panic::catch_unwind( + ::std::panic::AssertUnwindSafe(|| $blocks) + ) { + if panic.is_none() { + panic = Some(p); + } + } + )* + if let Some(panic) = panic { + ::std::panic::resume_unwind(panic); + } } } @@ -140,6 +153,24 @@ cfg_if! { t.into_iter() } + pub fn par_for_each_in( + t: T, + for_each: + impl Fn(<::IntoIter as Iterator>::Item) + Sync + Send + ) { + let mut panic = None; + t.into_iter().for_each(|i| { + if let Err(p) = catch_unwind(AssertUnwindSafe(|| for_each(i))) { + if panic.is_none() { + panic = Some(p); + } + } + }); + if let Some(panic) = panic { + resume_unwind(panic); + } + } + pub type MetadataRef = OwningRef, [u8]>; pub use std::rc::Rc as Lrc; @@ -308,6 +339,15 @@ cfg_if! { t.into_par_iter() } + pub fn par_for_each_in( + t: T, + for_each: impl Fn( + <::Iter as ParallelIterator>::Item + ) + Sync + Send + ) { + t.into_par_iter().for_each(for_each) + } + pub type MetadataRef = OwningRef, [u8]>; /// This makes locks panic if they are already held. From db9a1c1aaf261c8505d09ac6bd3364ef0d19ee71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 6 Mar 2019 04:46:46 +0100 Subject: [PATCH 47/54] Add some comments --- src/librustc_data_structures/sync.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/librustc_data_structures/sync.rs b/src/librustc_data_structures/sync.rs index f006a95f3fa3c..73247c1469efd 100644 --- a/src/librustc_data_structures/sync.rs +++ b/src/librustc_data_structures/sync.rs @@ -131,6 +131,8 @@ cfg_if! { #[macro_export] macro_rules! parallel { ($($blocks:tt),*) => { + // We catch panics here ensuring that all the blocks execute. + // This makes behavior consistent with the parallel compiler. let mut panic = None; $( if let Err(p) = ::std::panic::catch_unwind( @@ -158,6 +160,8 @@ cfg_if! { for_each: impl Fn(<::IntoIter as Iterator>::Item) + Sync + Send ) { + // We catch panics here ensuring that all the loop iterations execute. + // This makes behavior consistent with the parallel compiler. let mut panic = None; t.into_iter().for_each(|i| { if let Err(p) = catch_unwind(AssertUnwindSafe(|| for_each(i))) { @@ -309,6 +313,8 @@ cfg_if! { use std::thread; pub use rayon::{join, scope}; + /// Runs a list of blocks in parallel. The first block is executed immediately on + /// the current thread. Use that for the longest running block. #[macro_export] macro_rules! parallel { (impl $fblock:tt [$($c:tt,)*] [$block:tt $(, $rest:tt)*]) => { @@ -323,7 +329,7 @@ cfg_if! { }) }; ($fblock:tt, $($blocks:tt),*) => { - // Reverse the order of the blocks since Rayon executes them in reverse order + // Reverse the order of the later blocks since Rayon executes them in reverse order // when using a single thread. This ensures the execution order matches that // of a single threaded rustc parallel!(impl $fblock [] [$($blocks),*]); From 7985c6f8ecf680dcc960bb2ccc0c787274a449de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 6 Mar 2019 04:50:50 +0100 Subject: [PATCH 48/54] Rename check_privacy to check_private_in_public --- src/librustc/dep_graph/dep_node.rs | 2 +- src/librustc/ty/query/config.rs | 4 ++-- src/librustc/ty/query/mod.rs | 2 +- src/librustc/ty/query/plumbing.rs | 2 +- src/librustc_interface/passes.rs | 4 ++-- src/librustc_privacy/lib.rs | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index a46ffffe94cb8..41a4a8031006f 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -456,7 +456,7 @@ define_dep_nodes!( <'tcx> [eval_always] CoherenceInherentImplOverlapCheck, [] CoherenceCheckTrait(DefId), [eval_always] PrivacyAccessLevels(CrateNum), - [eval_always] CheckPrivacy(CrateNum), + [eval_always] CheckPrivateInPublic(CrateNum), [eval_always] Analysis(CrateNum), // Represents the MIR for a fn; also used as the task node for diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index 0fef90c945e94..835a8314959ab 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -369,9 +369,9 @@ impl<'tcx> QueryDescription<'tcx> for queries::privacy_access_levels<'tcx> { } } -impl<'tcx> QueryDescription<'tcx> for queries::check_privacy<'tcx> { +impl<'tcx> QueryDescription<'tcx> for queries::check_private_in_public<'tcx> { fn describe(_: TyCtxt<'_, '_, '_>, _: CrateNum) -> Cow<'static, str> { - "privacy checking".into() + "checking for private elements in public interfaces".into() } } diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index cb72ad6fe8bd9..8804ed22264ce 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -352,7 +352,7 @@ define_queries! { <'tcx> /// Performs part of the privacy check and computes "access levels". [] fn privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Lrc, - [] fn check_privacy: CheckPrivacy(CrateNum) -> (), + [] fn check_private_in_public: CheckPrivateInPublic(CrateNum) -> (), }, Other { diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index ff2639da13650..e3276ba0bea7b 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1251,7 +1251,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, force!(crate_inherent_impls_overlap_check, LOCAL_CRATE) }, DepKind::PrivacyAccessLevels => { force!(privacy_access_levels, LOCAL_CRATE); } - DepKind::CheckPrivacy => { force!(check_privacy, LOCAL_CRATE); } + DepKind::CheckPrivateInPublic => { force!(check_private_in_public, LOCAL_CRATE); } DepKind::MirBuilt => { force!(mir_built, def_id!()); } DepKind::MirConstQualif => { force!(mir_const_qualif, def_id!()); } DepKind::MirConst => { force!(mir_const, def_id!()); } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 2066747a6ffb6..8277615b46502 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -281,8 +281,8 @@ fn analysis<'tcx>( tcx.ensure().privacy_access_levels(LOCAL_CRATE); }); parallel!({ - time(sess, "privacy checking", || { - tcx.ensure().check_privacy(LOCAL_CRATE); + time(sess, "private in public", || { + tcx.ensure().check_private_in_public(LOCAL_CRATE); }); }, { time(sess, "death checking", || middle::dead::check_crate(tcx)); diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 372a923069424..5443f08debf74 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1760,7 +1760,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> pub fn provide(providers: &mut Providers<'_>) { *providers = Providers { privacy_access_levels, - check_privacy, + check_private_in_public, check_mod_privacy, ..*providers }; @@ -1820,7 +1820,7 @@ fn privacy_access_levels<'tcx>( Lrc::new(visitor.access_levels) } -fn check_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, krate: CrateNum) { +fn check_private_in_public<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, krate: CrateNum) { assert_eq!(krate, LOCAL_CRATE); let access_levels = tcx.privacy_access_levels(LOCAL_CRATE); From 9e5def9616aba274a6c3138d9f66e778d50c35e0 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 21 Feb 2019 20:21:50 -0500 Subject: [PATCH 49/54] rust-lldb: fix crash when printing empty string --- src/etc/lldb_rust_formatters.py | 2 ++ src/test/debuginfo/empty-string.rs | 35 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 src/test/debuginfo/empty-string.rs diff --git a/src/etc/lldb_rust_formatters.py b/src/etc/lldb_rust_formatters.py index 2c651c90f82eb..fdc1c4fa0cc38 100644 --- a/src/etc/lldb_rust_formatters.py +++ b/src/etc/lldb_rust_formatters.py @@ -290,6 +290,8 @@ def render_element(i): def read_utf8_string(ptr_val, byte_count): + if byte_count == 0: + return '""' error = lldb.SBError() process = ptr_val.get_wrapped_value().GetProcess() data = process.ReadMemory(ptr_val.as_integer(), byte_count, error) diff --git a/src/test/debuginfo/empty-string.rs b/src/test/debuginfo/empty-string.rs new file mode 100644 index 0000000000000..8c5f67a66043e --- /dev/null +++ b/src/test/debuginfo/empty-string.rs @@ -0,0 +1,35 @@ +// ignore-windows failing on win32 bot +// ignore-android: FIXME(#10381) +// compile-flags:-g +// min-gdb-version: 7.7 +// min-lldb-version: 310 + +// === GDB TESTS =================================================================================== + +// gdb-command: run + +// gdb-command: print empty_string +// gdb-check:$1 = "" + +// gdb-command: print empty_str +// gdb-check:$2 = "" + +// === LLDB TESTS ================================================================================== + +// lldb-command: run + +// lldb-command: fr v empty_string +// lldb-check:[...]empty_string = "" + +// lldb-command: fr v empty_str +// lldb-check:[...]empty_str = "" + +fn main() { + let empty_string = String::new(); + + let empty_str = ""; + + zzz(); // #break +} + +fn zzz() {} From 90bb07eca6c40ab98d31401597ba3f6c50a70c92 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 6 Mar 2019 19:57:00 +0100 Subject: [PATCH 50/54] Apply suggestions from code review Co-Authored-By: RalfJung --- src/libcore/mem.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 4b5056c5adffa..2e82c8c77816c 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1049,7 +1049,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let x: &i32 = unsafe { mem::zeroed() }; // undefined behavior! -/// // equivalent code with `MaybeUninit<&i32>` +/// // The equivalent code with `MaybeUninit<&i32>`: /// let x: &i32 = unsafe { MaybeUninit::zeroed().into_initialized() }; // undefined behavior! /// ``` /// @@ -1078,7 +1078,7 @@ impl DerefMut for ManuallyDrop { /// use std::mem::{self, MaybeUninit}; /// /// let x: i32 = unsafe { mem::uninitialized() }; // undefined behavior! -/// // equivalent code with `MaybeUninit` +/// // The equivalent code with `MaybeUninit`: /// let x: i32 = unsafe { MaybeUninit::uninitialized().into_initialized() }; // undefined behavior! /// ``` /// (Notice that the rules around uninitialized integers are not finalized yet, but @@ -1231,7 +1231,7 @@ impl MaybeUninit { /// let x_vec = unsafe { &*x.as_ptr() }; /// // We have created a reference to an uninitialized vector! This is undefined behavior. /// ``` - /// (Notice that the rules around referenced to uninitialized data are not finalized yet, but + /// (Notice that the rules around references to uninitialized data are not finalized yet, but /// until they are, it is advisable to avoid them.) #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] From cefe9b09c120cfd8684fc2310ed2b295aafca01c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Mar 2019 20:02:50 +0100 Subject: [PATCH 51/54] Apply suggestions from code review --- src/libcore/mem.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 2e82c8c77816c..2b6e7ab9b9950 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -1269,7 +1269,7 @@ impl MaybeUninit { /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; /// // We have created a reference to an uninitialized vector! This is undefined behavior. /// ``` - /// (Notice that the rules around referenced to uninitialized data are not finalized yet, but + /// (Notice that the rules around references to uninitialized data are not finalized yet, but /// until they are, it is advisable to avoid them.) #[unstable(feature = "maybe_uninit", issue = "53491")] #[inline(always)] From e5b3ed84a032b10db0c85fcfd8baf67424cac63e Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 7 Mar 2019 10:27:58 +0100 Subject: [PATCH 52/54] Actually publish miri in the manifest --- src/tools/build-manifest/src/main.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/build-manifest/src/main.rs b/src/tools/build-manifest/src/main.rs index 8d87c404d0b28..d44a51a9635e9 100644 --- a/src/tools/build-manifest/src/main.rs +++ b/src/tools/build-manifest/src/main.rs @@ -358,6 +358,7 @@ impl Builder { self.package("rust-src", &mut manifest.pkg, &["*"]); self.package("rls-preview", &mut manifest.pkg, HOSTS); self.package("clippy-preview", &mut manifest.pkg, HOSTS); + self.package("miri", &mut manifest.pkg, HOSTS); self.package("rustfmt-preview", &mut manifest.pkg, HOSTS); self.package("rust-analysis", &mut manifest.pkg, TARGETS); self.package("llvm-tools-preview", &mut manifest.pkg, TARGETS); @@ -375,7 +376,7 @@ impl Builder { &["rustc", "cargo", "rust-std", "rust-mingw", "rust-docs", "rustfmt-preview", "clippy-preview", "rls-preview", "rust-src", "llvm-tools-preview", - "lldb-preview", "rust-analysis"]); + "lldb-preview", "rust-analysis", "miri"]); manifest.renames.insert("rls".to_owned(), Rename { to: "rls-preview".to_owned() }); manifest.renames.insert("rustfmt".to_owned(), Rename { to: "rustfmt-preview".to_owned() }); @@ -422,6 +423,7 @@ impl Builder { // weren't built extensions.extend(vec![ Component { pkg: "clippy-preview".to_string(), target: host.to_string() }, + Component { pkg: "miri".to_string(), target: host.to_string() }, Component { pkg: "rls-preview".to_string(), target: host.to_string() }, Component { pkg: "rustfmt-preview".to_string(), target: host.to_string() }, Component { pkg: "llvm-tools-preview".to_string(), target: host.to_string() }, From 6465257e5442b489a1d4469d3f613aeb637e3665 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 8 Mar 2019 07:41:19 -0800 Subject: [PATCH 53/54] std: Delete a by-definition spuriously failing test This commit deletes the `connect_timeout_unbound` test from the standard library which, unfortunately, is by definition eventually going to be a spuriously failing test. There's no way to reserve a port as unbound so we can rely on ecosystem testing for this feature for now. Closes #52590 --- src/libstd/net/tcp.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/libstd/net/tcp.rs b/src/libstd/net/tcp.rs index 275557da96f67..ce0c5c0bb0dc7 100644 --- a/src/libstd/net/tcp.rs +++ b/src/libstd/net/tcp.rs @@ -1742,21 +1742,6 @@ mod tests { }) } - #[test] - fn connect_timeout_unbound() { - // bind and drop a socket to track down a "probably unassigned" port - let socket = TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = socket.local_addr().unwrap(); - drop(socket); - - let timeout = Duration::from_secs(1); - let e = TcpStream::connect_timeout(&addr, timeout).unwrap_err(); - assert!(e.kind() == io::ErrorKind::ConnectionRefused || - e.kind() == io::ErrorKind::TimedOut || - e.kind() == io::ErrorKind::Other, - "bad error: {} {:?}", e, e.kind()); - } - #[test] fn connect_timeout_valid() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); From 2309625941ac548c407def97eb87d03b237737cf Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sat, 9 Mar 2019 21:59:54 +0900 Subject: [PATCH 54/54] Expose new_sub_parser_from_file This function is useful when external tools like rustfmt want to parse internal files without parsing a whole crate. --- src/libsyntax/parse/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 98b5fe563b8a9..371e8fe5cf66f 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -185,7 +185,7 @@ pub fn maybe_new_parser_from_file<'a>(sess: &'a ParseSess, path: &Path) /// Given a session, a crate config, a path, and a span, add /// the file at the given path to the source_map, and return a parser. /// On an error, use the given span as the source of the problem. -crate fn new_sub_parser_from_file<'a>(sess: &'a ParseSess, +pub fn new_sub_parser_from_file<'a>(sess: &'a ParseSess, path: &Path, directory_ownership: DirectoryOwnership, module_name: Option,