From e720f4237a39dba0a4b741812de59d1654b3b253 Mon Sep 17 00:00:00 2001 From: Ryan Scheel Date: Sat, 1 Aug 2020 22:56:25 -0700 Subject: [PATCH 01/36] See also X-Link mem::{swap, take, replace} --- library/core/src/mem/mod.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index 4e58e118562ef..9107c570a8970 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -670,6 +670,9 @@ pub unsafe fn uninitialized() -> T { /// Swaps the values at two mutable locations, without deinitializing either one. /// +/// * If you want to swap with a default or dummy value, see [`take`]. +/// * If you want to swap with a passed value, returning the old value, see [`replace`]. +/// /// # Examples /// /// ``` @@ -683,6 +686,9 @@ pub unsafe fn uninitialized() -> T { /// assert_eq!(42, x); /// assert_eq!(5, y); /// ``` +/// +/// [`replace`]: fn.replace.html +/// [`take`]: fn.take.html #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn swap(x: &mut T, y: &mut T) { @@ -695,6 +701,9 @@ pub fn swap(x: &mut T, y: &mut T) { /// Replaces `dest` with the default value of `T`, returning the previous `dest` value. /// +/// * If you want to replace the values of two variables, see [`swap`]. +/// * If you want to replace with a passed value instead of the default value, see [`replace`]. +/// /// # Examples /// /// A simple example: @@ -747,6 +756,8 @@ pub fn swap(x: &mut T, y: &mut T) { /// ``` /// /// [`Clone`]: ../../std/clone/trait.Clone.html +/// [`replace`]: fn.replace.html +/// [`swap`]: fn.swap.html #[inline] #[stable(feature = "mem_take", since = "1.40.0")] pub fn take(dest: &mut T) -> T { @@ -757,6 +768,9 @@ pub fn take(dest: &mut T) -> T { /// /// Neither value is dropped. /// +/// * If you want to replace the values of two variables, see [`swap`]. +/// * If you want to replace with a default value, see [`take`]. +/// /// # Examples /// /// A simple example: @@ -810,6 +824,8 @@ pub fn take(dest: &mut T) -> T { /// ``` /// /// [`Clone`]: ../../std/clone/trait.Clone.html +/// [`swap`]: fn.swap.html +/// [`take`]: fn.take.html #[inline] #[stable(feature = "rust1", since = "1.0.0")] #[must_use = "if you don't need the old value, you can just assign the new value directly"] From 7835c8c06cc80b5a0d3d08c1ab1b91240a8aec52 Mon Sep 17 00:00:00 2001 From: Jan Riemer Date: Sun, 2 Aug 2020 14:57:19 +0200 Subject: [PATCH 02/36] docs(marker/copy): clarify that `&T` is also `Copy` In the current documentation about the `Copy` marker trait, there is a section about "additional implementors", which list additional implementors of the `Copy` trait. The fact that shared references are also `Copy` is mixed with another point, which makes it hard to recognize and make it seem not as important. This clarifies the fact that shared references are also `Copy`, by mentioning it as a separate item in the list of "additional implementors". --- library/core/src/marker.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 56dddee7b7799..255f4474ea07d 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -347,9 +347,8 @@ pub trait StructuralEq { /// * Tuple types, if each component also implements `Copy` (e.g., `()`, `(i32, bool)`) /// * Closure types, if they capture no value from the environment /// or if all such captured values implement `Copy` themselves. -/// Note that variables captured by shared reference always implement `Copy` -/// (even if the referent doesn't), -/// while variables captured by mutable reference never implement `Copy`. +/// * Variables captured by shared reference (e.g. `&T`) implement `Copy`, even if the referent (`T`) doesn't, +/// while variables captured by mutable reference (e.g. `&mut T`) never implement `Copy`. /// /// [`Vec`]: ../../std/vec/struct.Vec.html /// [`String`]: ../../std/string/struct.String.html From 9b0f3d1266b99ca49edcb8e73686c54849e79a24 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Thu, 13 Aug 2020 16:58:50 -0400 Subject: [PATCH 03/36] Fix documentation error --- library/core/src/str/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index eac4741cd260a..a062a68673f02 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -2030,7 +2030,7 @@ mod traits { /// # Panics /// /// Panics if `begin` does not point to the starting byte offset of - /// a character (as defined by `is_char_boundary`), or if `begin >= len`. + /// a character (as defined by `is_char_boundary`), or if `begin > len`. #[stable(feature = "str_checked_slicing", since = "1.20.0")] unsafe impl SliceIndex for ops::RangeFrom { type Output = str; From a876b3d8aaf21510e569ce62dfc6c50a3cf3efd3 Mon Sep 17 00:00:00 2001 From: Jan Riemer Date: Sun, 16 Aug 2020 20:03:34 +0200 Subject: [PATCH 04/36] docs(marker/copy): provide example for `&T` being `Copy` In the current documentation about the `Copy` marker trait, there is a section with examples of structs that can implement `Copy`. Currently there is no example for showing that shared references (`&T`) are also `Copy`. It is worth to have a dedicated example for `&T` being `Copy`, because shared references are an integral part of the language and it being `Copy` is not as intuitive as other types that share this behaviour like `i32` or `bool`. The example picks up on the previous non-`Copy` struct and shows that structs can be `Copy`, even when they hold a shared reference to a non-`Copy` type. --- library/core/src/marker.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 255f4474ea07d..1458d25d7f7d2 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -315,6 +315,18 @@ pub trait StructuralEq { /// the trait `Copy` may not be implemented for this type; field `points` does not implement `Copy` /// ``` /// +/// Shared references (`&T`) are also `Copy`, so a struct can be `Copy`, even when it holds +/// shared references of types `T` that are *not* `Copy`. Consider the following struct, +/// which can implement `Copy`, because it only holds a *shared reference* to our non-`Copy` +/// type `PointList` from above: +/// ``` +/// # #![allow(dead_code)] +/// # struct PointList; +/// struct PointListWrapper<'a> { +/// point_list_ref: &'a PointList, +/// } +/// ``` +/// /// ## When *can't* my type be `Copy`? /// /// Some types can't be copied safely. For example, copying `&mut T` would create an aliased @@ -347,8 +359,9 @@ pub trait StructuralEq { /// * Tuple types, if each component also implements `Copy` (e.g., `()`, `(i32, bool)`) /// * Closure types, if they capture no value from the environment /// or if all such captured values implement `Copy` themselves. -/// * Variables captured by shared reference (e.g. `&T`) implement `Copy`, even if the referent (`T`) doesn't, -/// while variables captured by mutable reference (e.g. `&mut T`) never implement `Copy`. +/// Note that variables captured by shared reference always implement `Copy` +/// (even if the referent doesn't), +/// while variables captured by mutable reference never implement `Copy`. /// /// [`Vec`]: ../../std/vec/struct.Vec.html /// [`String`]: ../../std/string/struct.String.html @@ -539,7 +552,7 @@ macro_rules! impls { /// For a more in-depth explanation of how to use `PhantomData`, please see /// [the Nomicon](../../nomicon/phantom-data.html). /// -/// # A ghastly note 👻👻👻 +/// # A ghastly note /// /// Though they both have scary names, `PhantomData` and 'phantom types' are /// related, but not identical. A phantom type parameter is simply a type From 43dec0e1715b5f20a36527f3e0ab3e09594e846d Mon Sep 17 00:00:00 2001 From: Jan Riemer Date: Sun, 16 Aug 2020 20:17:28 +0200 Subject: [PATCH 05/36] rephrase: struct -> type Co-authored-by: Joshua Nelson --- library/core/src/marker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 1458d25d7f7d2..b698cbd7802de 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -315,7 +315,7 @@ pub trait StructuralEq { /// the trait `Copy` may not be implemented for this type; field `points` does not implement `Copy` /// ``` /// -/// Shared references (`&T`) are also `Copy`, so a struct can be `Copy`, even when it holds +/// Shared references (`&T`) are also `Copy`, so a type can be `Copy`, even when it holds /// shared references of types `T` that are *not* `Copy`. Consider the following struct, /// which can implement `Copy`, because it only holds a *shared reference* to our non-`Copy` /// type `PointList` from above: From dce864454c3748dd4119eac40b0dd35772467fb2 Mon Sep 17 00:00:00 2001 From: Jan Riemer Date: Sun, 16 Aug 2020 20:19:21 +0200 Subject: [PATCH 06/36] add back emojis that have been removed accidentally Co-authored-by: Joshua Nelson --- library/core/src/marker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index b698cbd7802de..a9f8e332d2851 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -552,7 +552,7 @@ macro_rules! impls { /// For a more in-depth explanation of how to use `PhantomData`, please see /// [the Nomicon](../../nomicon/phantom-data.html). /// -/// # A ghastly note +/// # A ghastly note 👻👻👻 /// /// Though they both have scary names, `PhantomData` and 'phantom types' are /// related, but not identical. A phantom type parameter is simply a type From 9061da2e14afee47f039b74ecadae97b737c6c9b Mon Sep 17 00:00:00 2001 From: Jan Riemer Date: Sun, 16 Aug 2020 22:15:59 +0200 Subject: [PATCH 07/36] add empty line above code block Co-authored-by: Poliorcetics --- library/core/src/marker.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index a9f8e332d2851..85e2e3830a513 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -319,6 +319,7 @@ pub trait StructuralEq { /// shared references of types `T` that are *not* `Copy`. Consider the following struct, /// which can implement `Copy`, because it only holds a *shared reference* to our non-`Copy` /// type `PointList` from above: +/// /// ``` /// # #![allow(dead_code)] /// # struct PointList; From 56daf63d10f7ade09ab0523333c65370a88c79ae Mon Sep 17 00:00:00 2001 From: Jan Riemer Date: Sun, 16 Aug 2020 22:25:59 +0200 Subject: [PATCH 08/36] docs: add `derive` for struct Code blocks in doc comments are compiled and run, so we show `Copy` works in this example. Co-authored-by: Poliorcetics --- library/core/src/marker.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 85e2e3830a513..f113f9392301a 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -323,6 +323,7 @@ pub trait StructuralEq { /// ``` /// # #![allow(dead_code)] /// # struct PointList; +/// #[derive(Copy, Clone)] /// struct PointListWrapper<'a> { /// point_list_ref: &'a PointList, /// } From b0eb55a092506aa4cfe34969c56adfeca9616750 Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Sat, 15 Aug 2020 13:18:54 +1000 Subject: [PATCH 09/36] Add test demonstrating the issue. --- .../ui/lint/clashing-extern-fn-recursion.rs | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 src/test/ui/lint/clashing-extern-fn-recursion.rs diff --git a/src/test/ui/lint/clashing-extern-fn-recursion.rs b/src/test/ui/lint/clashing-extern-fn-recursion.rs new file mode 100644 index 0000000000000..51bfd2032838c --- /dev/null +++ b/src/test/ui/lint/clashing-extern-fn-recursion.rs @@ -0,0 +1,117 @@ +// check-pass +// +// This tests checks that clashing_extern_declarations handles types that are recursive through a +// pointer or ref argument. See #75512. + +#![crate_type = "lib"] + +mod raw_ptr_recursion { + mod a { + #[repr(C)] + struct Pointy { + pointy: *const Pointy, + } + + extern "C" { + fn run_pointy(pointy: Pointy); + } + } + mod b { + #[repr(C)] + struct Pointy { + pointy: *const Pointy, + } + + extern "C" { + fn run_pointy(pointy: Pointy); + } + } +} + +mod raw_ptr_recursion_once_removed { + mod a { + #[repr(C)] + struct Pointy1 { + pointy_two: *const Pointy2, + } + + #[repr(C)] + struct Pointy2 { + pointy_one: *const Pointy1, + } + + extern "C" { + fn run_pointy2(pointy: Pointy2); + } + } + + mod b { + #[repr(C)] + struct Pointy1 { + pointy_two: *const Pointy2, + } + + #[repr(C)] + struct Pointy2 { + pointy_one: *const Pointy1, + } + + extern "C" { + fn run_pointy2(pointy: Pointy2); + } + } +} + +mod ref_recursion { + mod a { + #[repr(C)] + struct Reffy<'a> { + reffy: &'a Reffy<'a>, + } + + extern "C" { + fn reffy_recursion(reffy: Reffy); + } + } + mod b { + #[repr(C)] + struct Reffy<'a> { + reffy: &'a Reffy<'a>, + } + + extern "C" { + fn reffy_recursion(reffy: Reffy); + } + } +} + +mod ref_recursion_once_removed { + mod a { + #[repr(C)] + struct Reffy1<'a> { + reffy: &'a Reffy1<'a>, + } + + struct Reffy2<'a> { + reffy: &'a Reffy2<'a>, + } + + extern "C" { + fn reffy_once_removed(reffy: Reffy1); + } + } + mod b { + #[repr(C)] + struct Reffy1<'a> { + reffy: &'a Reffy1<'a>, + } + + struct Reffy2<'a> { + reffy: &'a Reffy2<'a>, + } + + extern "C" { + fn reffy_once_removed(reffy: Reffy1); + } + } +} From db753137a14b04886567a1f20768b96feba05594 Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Sat, 15 Aug 2020 15:05:18 +1000 Subject: [PATCH 10/36] Fix stack overflow for recursive types. Adds a seen set to structurally_same_type to avoid recursing indefinitely when a reference or pointer member introduces a cycle in the visited types. --- src/librustc_lint/builtin.rs | 289 ++++++++++++++++++++++------------- 1 file changed, 187 insertions(+), 102 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 3859d0f163ad5..0e578ac5034db 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2153,44 +2153,99 @@ impl ClashingExternDeclarations { b: Ty<'tcx>, ckind: CItemKind, ) -> bool { - debug!("structurally_same_type(cx, a = {:?}, b = {:?})", a, b); - let tcx = cx.tcx; - if a == b || rustc_middle::ty::TyS::same_type(a, b) { - // All nominally-same types are structurally same, too. - true - } else { - // Do a full, depth-first comparison between the two. - use rustc_middle::ty::TyKind::*; - let a_kind = &a.kind; - let b_kind = &b.kind; - - let compare_layouts = |a, b| -> bool { - let a_layout = &cx.layout_of(a).unwrap().layout.abi; - let b_layout = &cx.layout_of(b).unwrap().layout.abi; - debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout); - a_layout == b_layout - }; + // In order to avoid endlessly recursing on recursive types, we maintain a "seen" set. + // We'll need to store every combination of types we encounter anyway, so we also memoize + // the result. + struct SeenSet<'tcx>(FxHashMap<(Ty<'tcx>, Ty<'tcx>), Option>); + + enum SeenSetResult { + /// We've never seen this combination of types. + Unseen, + /// We've seen this combination of types, but are still computing the result. + Computing, + /// We've seen this combination of types, and have already computed the result. + Computed(bool), + } + + impl<'tcx> SeenSet<'tcx> { + fn new() -> Self { + SeenSet(FxHashMap::default()) + } + /// Mark (a, b) as `Computing`. + fn mark_computing(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) { + self.0.insert((a, b), None); + } + /// Mark (a, b) as `Computed(result)`. + fn mark_computed(&mut self, a: Ty<'tcx>, b: Ty<'tcx>, result: bool) { + *self.0.get_mut(&(a, b)).expect("Missing prior call to mark_computing") = + Some(result); + } + fn get(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> SeenSetResult { + match self.0.get(&(a, b)) { + None => SeenSetResult::Unseen, + Some(None) => SeenSetResult::Computing, + Some(Some(b)) => SeenSetResult::Computed(*b), + } + } + } + fn structurally_same_type_impl<'tcx>( + seen_types: &mut SeenSet<'tcx>, + cx: &LateContext<'tcx>, + a: Ty<'tcx>, + b: Ty<'tcx>, + ckind: CItemKind, + ) -> bool { + debug!("structurally_same_type_impl(cx, a = {:?}, b = {:?})", a, b); + match seen_types.get(a, b) { + // If we've already computed the result, just return the memoized result. + SeenSetResult::Computed(result) => result, + // We are already in the process of computing structural sameness for this type, + // meaning we've found a cycle. The types are structurally same, then. + SeenSetResult::Computing => true, + // We haven't seen this combination of types at all -- compute their sameness. + SeenSetResult::Unseen => { + seen_types.mark_computing(a, b); + let tcx = cx.tcx; + let result = if a == b || rustc_middle::ty::TyS::same_type(a, b) { + // All nominally-same types are structurally same, too. + true + } else { + // Do a full, depth-first comparison between the two. + use rustc_middle::ty::TyKind::*; + let a_kind = &a.kind; + let b_kind = &b.kind; + + let compare_layouts = |a, b| -> bool { + let a_layout = &cx.layout_of(a).unwrap().layout.abi; + let b_layout = &cx.layout_of(b).unwrap().layout.abi; + debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout); + a_layout == b_layout + }; + + #[allow(rustc::usage_of_ty_tykind)] + let is_primitive_or_pointer = |kind: &ty::TyKind<'_>| { + kind.is_primitive() || matches!(kind, RawPtr(..)) + }; - #[allow(rustc::usage_of_ty_tykind)] - let is_primitive_or_pointer = - |kind: &ty::TyKind<'_>| kind.is_primitive() || matches!(kind, RawPtr(..)); - - match (a_kind, b_kind) { - (Adt(_, a_substs), Adt(_, b_substs)) => { - let a = a.subst(cx.tcx, a_substs); - let b = b.subst(cx.tcx, b_substs); - debug!("Comparing {:?} and {:?}", a, b); - - if let (Adt(a_def, ..), Adt(b_def, ..)) = (&a.kind, &b.kind) { - // Grab a flattened representation of all fields. - let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter()); - let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter()); - compare_layouts(a, b) + match (a_kind, b_kind) { + (Adt(_, a_substs), Adt(_, b_substs)) => { + let a = a.subst(cx.tcx, a_substs); + let b = b.subst(cx.tcx, b_substs); + debug!("Comparing {:?} and {:?}", a, b); + + if let (Adt(a_def, ..), Adt(b_def, ..)) = (&a.kind, &b.kind) { + // Grab a flattened representation of all fields. + let a_fields = + a_def.variants.iter().flat_map(|v| v.fields.iter()); + let b_fields = + b_def.variants.iter().flat_map(|v| v.fields.iter()); + compare_layouts(a, b) && a_fields.eq_by( b_fields, |&ty::FieldDef { did: a_did, .. }, &ty::FieldDef { did: b_did, .. }| { - Self::structurally_same_type( + structurally_same_type_impl( + seen_types, cx, tcx.type_of(a_did), tcx.type_of(b_did), @@ -2198,78 +2253,108 @@ impl ClashingExternDeclarations { ) }, ) - } else { - unreachable!() - } - } - (Array(a_ty, a_const), Array(b_ty, b_const)) => { - // For arrays, we also check the constness of the type. - a_const.val == b_const.val - && Self::structurally_same_type(cx, a_const.ty, b_const.ty, ckind) - && Self::structurally_same_type(cx, a_ty, b_ty, ckind) - } - (Slice(a_ty), Slice(b_ty)) => Self::structurally_same_type(cx, a_ty, b_ty, ckind), - (RawPtr(a_tymut), RawPtr(b_tymut)) => { - a_tymut.mutbl == b_tymut.mutbl - && Self::structurally_same_type(cx, &a_tymut.ty, &b_tymut.ty, ckind) - } - (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => { - // For structural sameness, we don't need the region to be same. - a_mut == b_mut && Self::structurally_same_type(cx, a_ty, b_ty, ckind) - } - (FnDef(..), FnDef(..)) => { - let a_poly_sig = a.fn_sig(tcx); - let b_poly_sig = b.fn_sig(tcx); - - // As we don't compare regions, skip_binder is fine. - let a_sig = a_poly_sig.skip_binder(); - let b_sig = b_poly_sig.skip_binder(); - - (a_sig.abi, a_sig.unsafety, a_sig.c_variadic) - == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic) - && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| { - Self::structurally_same_type(cx, a, b, ckind) - }) - && Self::structurally_same_type(cx, a_sig.output(), b_sig.output(), ckind) - } - (Tuple(a_substs), Tuple(b_substs)) => { - a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| { - Self::structurally_same_type(cx, a_ty, b_ty, ckind) - }) - } - // For these, it's not quite as easy to define structural-sameness quite so easily. - // For the purposes of this lint, take the conservative approach and mark them as - // not structurally same. - (Dynamic(..), Dynamic(..)) - | (Error(..), Error(..)) - | (Closure(..), Closure(..)) - | (Generator(..), Generator(..)) - | (GeneratorWitness(..), GeneratorWitness(..)) - | (Projection(..), Projection(..)) - | (Opaque(..), Opaque(..)) => false, - - // These definitely should have been caught above. - (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), - - // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a - // non-null field. - (Adt(..), other_kind) | (other_kind, Adt(..)) - if is_primitive_or_pointer(other_kind) => - { - let (primitive, adt) = - if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) }; - if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) { - ty == primitive - } else { - compare_layouts(a, b) - } + } else { + unreachable!() + } + } + (Array(a_ty, a_const), Array(b_ty, b_const)) => { + // For arrays, we also check the constness of the type. + a_const.val == b_const.val + && structurally_same_type_impl( + seen_types, cx, a_const.ty, b_const.ty, ckind, + ) + && structurally_same_type_impl( + seen_types, cx, a_ty, b_ty, ckind, + ) + } + (Slice(a_ty), Slice(b_ty)) => { + structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + } + (RawPtr(a_tymut), RawPtr(b_tymut)) => { + a_tymut.mutbl == b_tymut.mutbl + && structurally_same_type_impl( + seen_types, + cx, + &a_tymut.ty, + &b_tymut.ty, + ckind, + ) + } + (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => { + // For structural sameness, we don't need the region to be same. + a_mut == b_mut + && structurally_same_type_impl( + seen_types, cx, a_ty, b_ty, ckind, + ) + } + (FnDef(..), FnDef(..)) => { + let a_poly_sig = a.fn_sig(tcx); + let b_poly_sig = b.fn_sig(tcx); + + // As we don't compare regions, skip_binder is fine. + let a_sig = a_poly_sig.skip_binder(); + let b_sig = b_poly_sig.skip_binder(); + + (a_sig.abi, a_sig.unsafety, a_sig.c_variadic) + == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic) + && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| { + structurally_same_type_impl(seen_types, cx, a, b, ckind) + }) + && structurally_same_type_impl( + seen_types, + cx, + a_sig.output(), + b_sig.output(), + ckind, + ) + } + (Tuple(a_substs), Tuple(b_substs)) => { + a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| { + structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + }) + } + // For these, it's not quite as easy to define structural-sameness quite so easily. + // For the purposes of this lint, take the conservative approach and mark them as + // not structurally same. + (Dynamic(..), Dynamic(..)) + | (Error(..), Error(..)) + | (Closure(..), Closure(..)) + | (Generator(..), Generator(..)) + | (GeneratorWitness(..), GeneratorWitness(..)) + | (Projection(..), Projection(..)) + | (Opaque(..), Opaque(..)) => false, + + // These definitely should have been caught above. + (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => { + unreachable!() + } + + // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a + // non-null field. + (Adt(..), other_kind) | (other_kind, Adt(..)) + if is_primitive_or_pointer(other_kind) => + { + let (primitive, adt) = + if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) }; + if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) { + ty == primitive + } else { + compare_layouts(a, b) + } + } + // Otherwise, just compare the layouts. This may fail to lint for some + // incompatible types, but at the very least, will stop reads into + // uninitialised memory. + _ => compare_layouts(a, b), + } + }; + seen_types.mark_computed(a, b, result); + result } - // Otherwise, just compare the layouts. This may fail to lint for some - // incompatible types, but at the very least, will stop reads into - // uninitialised memory. - _ => compare_layouts(a, b), } } + let mut seen_types = SeenSet::new(); + structurally_same_type_impl(&mut seen_types, cx, a, b, ckind) } } From 154b74e8f959581d37d1a1ae860ee1bf7b682f34 Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Sun, 16 Aug 2020 10:26:45 +1000 Subject: [PATCH 11/36] Actually introduce a cycle in Reffy test. --- src/test/ui/lint/clashing-extern-fn-recursion.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/ui/lint/clashing-extern-fn-recursion.rs b/src/test/ui/lint/clashing-extern-fn-recursion.rs index 51bfd2032838c..ab0fd0a2e7085 100644 --- a/src/test/ui/lint/clashing-extern-fn-recursion.rs +++ b/src/test/ui/lint/clashing-extern-fn-recursion.rs @@ -89,28 +89,30 @@ mod ref_recursion_once_removed { mod a { #[repr(C)] struct Reffy1<'a> { - reffy: &'a Reffy1<'a>, + reffy: &'a Reffy2<'a>, } struct Reffy2<'a> { - reffy: &'a Reffy2<'a>, + reffy: &'a Reffy1<'a>, } extern "C" { + #[allow(improper_ctypes)] fn reffy_once_removed(reffy: Reffy1); } } mod b { #[repr(C)] struct Reffy1<'a> { - reffy: &'a Reffy1<'a>, + reffy: &'a Reffy2<'a>, } struct Reffy2<'a> { - reffy: &'a Reffy2<'a>, + reffy: &'a Reffy1<'a>, } extern "C" { + #[allow(improper_ctypes)] fn reffy_once_removed(reffy: Reffy1); } } From a1fa4e05acb2cbda9a02cc34f8000c965328dbde Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Sun, 16 Aug 2020 10:41:59 +1000 Subject: [PATCH 12/36] Remove unnecessary rebinding of def ids. --- src/librustc_lint/builtin.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 0e578ac5034db..47f680932f14e 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2228,18 +2228,15 @@ impl ClashingExternDeclarations { }; match (a_kind, b_kind) { - (Adt(_, a_substs), Adt(_, b_substs)) => { + (Adt(a_def, a_substs), Adt(b_def, b_substs)) => { let a = a.subst(cx.tcx, a_substs); let b = b.subst(cx.tcx, b_substs); debug!("Comparing {:?} and {:?}", a, b); - if let (Adt(a_def, ..), Adt(b_def, ..)) = (&a.kind, &b.kind) { - // Grab a flattened representation of all fields. - let a_fields = - a_def.variants.iter().flat_map(|v| v.fields.iter()); - let b_fields = - b_def.variants.iter().flat_map(|v| v.fields.iter()); - compare_layouts(a, b) + // Grab a flattened representation of all fields. + let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter()); + let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter()); + compare_layouts(a, b) && a_fields.eq_by( b_fields, |&ty::FieldDef { did: a_did, .. }, @@ -2253,9 +2250,6 @@ impl ClashingExternDeclarations { ) }, ) - } else { - unreachable!() - } } (Array(a_ty, a_const), Array(b_ty, b_const)) => { // For arrays, we also check the constness of the type. From 80c2c80d52e2384ee6ce33a7b41c0c47c4f0ffd1 Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Sun, 16 Aug 2020 10:47:35 +1000 Subject: [PATCH 13/36] Remove structural equiv check for Array const. --- src/librustc_lint/builtin.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 47f680932f14e..2cf0ed390f4fc 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2254,9 +2254,6 @@ impl ClashingExternDeclarations { (Array(a_ty, a_const), Array(b_ty, b_const)) => { // For arrays, we also check the constness of the type. a_const.val == b_const.val - && structurally_same_type_impl( - seen_types, cx, a_const.ty, b_const.ty, ckind, - ) && structurally_same_type_impl( seen_types, cx, a_ty, b_ty, ckind, ) From bca48ad7acbc7b7462bce58f0513c02c4e5444b5 Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Sun, 16 Aug 2020 10:50:20 +1000 Subject: [PATCH 14/36] Reduce indentation by replacing match arm w/ early return. --- src/librustc_lint/builtin.rs | 243 +++++++++++++++++------------------ 1 file changed, 118 insertions(+), 125 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 2cf0ed390f4fc..c23b443645de6 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2198,45 +2198,46 @@ impl ClashingExternDeclarations { debug!("structurally_same_type_impl(cx, a = {:?}, b = {:?})", a, b); match seen_types.get(a, b) { // If we've already computed the result, just return the memoized result. - SeenSetResult::Computed(result) => result, + SeenSetResult::Computed(result) => return result, // We are already in the process of computing structural sameness for this type, // meaning we've found a cycle. The types are structurally same, then. - SeenSetResult::Computing => true, - // We haven't seen this combination of types at all -- compute their sameness. - SeenSetResult::Unseen => { - seen_types.mark_computing(a, b); - let tcx = cx.tcx; - let result = if a == b || rustc_middle::ty::TyS::same_type(a, b) { - // All nominally-same types are structurally same, too. - true - } else { - // Do a full, depth-first comparison between the two. - use rustc_middle::ty::TyKind::*; - let a_kind = &a.kind; - let b_kind = &b.kind; - - let compare_layouts = |a, b| -> bool { - let a_layout = &cx.layout_of(a).unwrap().layout.abi; - let b_layout = &cx.layout_of(b).unwrap().layout.abi; - debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout); - a_layout == b_layout - }; + SeenSetResult::Computing => return true, + // We haven't seen this combination of types at all -- continue on to computing + // their sameness. + SeenSetResult::Unseen => (), + } + seen_types.mark_computing(a, b); + let tcx = cx.tcx; + let result = if a == b || rustc_middle::ty::TyS::same_type(a, b) { + // All nominally-same types are structurally same, too. + true + } else { + // Do a full, depth-first comparison between the two. + use rustc_middle::ty::TyKind::*; + let a_kind = &a.kind; + let b_kind = &b.kind; + + let compare_layouts = |a, b| -> bool { + let a_layout = &cx.layout_of(a).unwrap().layout.abi; + let b_layout = &cx.layout_of(b).unwrap().layout.abi; + debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout); + a_layout == b_layout + }; - #[allow(rustc::usage_of_ty_tykind)] - let is_primitive_or_pointer = |kind: &ty::TyKind<'_>| { - kind.is_primitive() || matches!(kind, RawPtr(..)) - }; + #[allow(rustc::usage_of_ty_tykind)] + let is_primitive_or_pointer = + |kind: &ty::TyKind<'_>| kind.is_primitive() || matches!(kind, RawPtr(..)); - match (a_kind, b_kind) { - (Adt(a_def, a_substs), Adt(b_def, b_substs)) => { - let a = a.subst(cx.tcx, a_substs); - let b = b.subst(cx.tcx, b_substs); - debug!("Comparing {:?} and {:?}", a, b); + match (a_kind, b_kind) { + (Adt(a_def, a_substs), Adt(b_def, b_substs)) => { + let a = a.subst(cx.tcx, a_substs); + let b = b.subst(cx.tcx, b_substs); + debug!("Comparing {:?} and {:?}", a, b); - // Grab a flattened representation of all fields. - let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter()); - let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter()); - compare_layouts(a, b) + // Grab a flattened representation of all fields. + let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter()); + let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter()); + compare_layouts(a, b) && a_fields.eq_by( b_fields, |&ty::FieldDef { did: a_did, .. }, @@ -2250,99 +2251,91 @@ impl ClashingExternDeclarations { ) }, ) - } - (Array(a_ty, a_const), Array(b_ty, b_const)) => { - // For arrays, we also check the constness of the type. - a_const.val == b_const.val - && structurally_same_type_impl( - seen_types, cx, a_ty, b_ty, ckind, - ) - } - (Slice(a_ty), Slice(b_ty)) => { - structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) - } - (RawPtr(a_tymut), RawPtr(b_tymut)) => { - a_tymut.mutbl == b_tymut.mutbl - && structurally_same_type_impl( - seen_types, - cx, - &a_tymut.ty, - &b_tymut.ty, - ckind, - ) - } - (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => { - // For structural sameness, we don't need the region to be same. - a_mut == b_mut - && structurally_same_type_impl( - seen_types, cx, a_ty, b_ty, ckind, - ) - } - (FnDef(..), FnDef(..)) => { - let a_poly_sig = a.fn_sig(tcx); - let b_poly_sig = b.fn_sig(tcx); - - // As we don't compare regions, skip_binder is fine. - let a_sig = a_poly_sig.skip_binder(); - let b_sig = b_poly_sig.skip_binder(); - - (a_sig.abi, a_sig.unsafety, a_sig.c_variadic) - == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic) - && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| { - structurally_same_type_impl(seen_types, cx, a, b, ckind) - }) - && structurally_same_type_impl( - seen_types, - cx, - a_sig.output(), - b_sig.output(), - ckind, - ) - } - (Tuple(a_substs), Tuple(b_substs)) => { - a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| { - structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) - }) - } - // For these, it's not quite as easy to define structural-sameness quite so easily. - // For the purposes of this lint, take the conservative approach and mark them as - // not structurally same. - (Dynamic(..), Dynamic(..)) - | (Error(..), Error(..)) - | (Closure(..), Closure(..)) - | (Generator(..), Generator(..)) - | (GeneratorWitness(..), GeneratorWitness(..)) - | (Projection(..), Projection(..)) - | (Opaque(..), Opaque(..)) => false, - - // These definitely should have been caught above. - (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => { - unreachable!() - } - - // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a - // non-null field. - (Adt(..), other_kind) | (other_kind, Adt(..)) - if is_primitive_or_pointer(other_kind) => - { - let (primitive, adt) = - if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) }; - if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) { - ty == primitive - } else { - compare_layouts(a, b) - } - } - // Otherwise, just compare the layouts. This may fail to lint for some - // incompatible types, but at the very least, will stop reads into - // uninitialised memory. - _ => compare_layouts(a, b), + } + (Array(a_ty, a_const), Array(b_ty, b_const)) => { + // For arrays, we also check the constness of the type. + a_const.val == b_const.val + && structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + } + (Slice(a_ty), Slice(b_ty)) => { + structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + } + (RawPtr(a_tymut), RawPtr(b_tymut)) => { + a_tymut.mutbl == b_tymut.mutbl + && structurally_same_type_impl( + seen_types, + cx, + &a_tymut.ty, + &b_tymut.ty, + ckind, + ) + } + (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => { + // For structural sameness, we don't need the region to be same. + a_mut == b_mut + && structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + } + (FnDef(..), FnDef(..)) => { + let a_poly_sig = a.fn_sig(tcx); + let b_poly_sig = b.fn_sig(tcx); + + // As we don't compare regions, skip_binder is fine. + let a_sig = a_poly_sig.skip_binder(); + let b_sig = b_poly_sig.skip_binder(); + + (a_sig.abi, a_sig.unsafety, a_sig.c_variadic) + == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic) + && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| { + structurally_same_type_impl(seen_types, cx, a, b, ckind) + }) + && structurally_same_type_impl( + seen_types, + cx, + a_sig.output(), + b_sig.output(), + ckind, + ) + } + (Tuple(a_substs), Tuple(b_substs)) => { + a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| { + structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + }) + } + // For these, it's not quite as easy to define structural-sameness quite so easily. + // For the purposes of this lint, take the conservative approach and mark them as + // not structurally same. + (Dynamic(..), Dynamic(..)) + | (Error(..), Error(..)) + | (Closure(..), Closure(..)) + | (Generator(..), Generator(..)) + | (GeneratorWitness(..), GeneratorWitness(..)) + | (Projection(..), Projection(..)) + | (Opaque(..), Opaque(..)) => false, + + // These definitely should have been caught above. + (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), + + // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a + // non-null field. + (Adt(..), other_kind) | (other_kind, Adt(..)) + if is_primitive_or_pointer(other_kind) => + { + let (primitive, adt) = + if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) }; + if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) { + ty == primitive + } else { + compare_layouts(a, b) } - }; - seen_types.mark_computed(a, b, result); - result + } + // Otherwise, just compare the layouts. This may fail to lint for some + // incompatible types, but at the very least, will stop reads into + // uninitialised memory. + _ => compare_layouts(a, b), } - } + }; + seen_types.mark_computed(a, b, result); + result } let mut seen_types = SeenSet::new(); structurally_same_type_impl(&mut seen_types, cx, a, b, ckind) From 6c57de1166d36725a689cef17e0dab8b9abcd00b Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Sun, 16 Aug 2020 11:01:14 +1000 Subject: [PATCH 15/36] Don't memoize seen types. That cache is unlikely to be particularly useful within a single invocation of structurally_same_type, especially compared to memoizing results across _all_ invocations of that function. --- src/librustc_lint/builtin.rs | 60 ++++++------------------------------ 1 file changed, 9 insertions(+), 51 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index c23b443645de6..781ebb6716758 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2153,62 +2153,22 @@ impl ClashingExternDeclarations { b: Ty<'tcx>, ckind: CItemKind, ) -> bool { - // In order to avoid endlessly recursing on recursive types, we maintain a "seen" set. - // We'll need to store every combination of types we encounter anyway, so we also memoize - // the result. - struct SeenSet<'tcx>(FxHashMap<(Ty<'tcx>, Ty<'tcx>), Option>); - - enum SeenSetResult { - /// We've never seen this combination of types. - Unseen, - /// We've seen this combination of types, but are still computing the result. - Computing, - /// We've seen this combination of types, and have already computed the result. - Computed(bool), - } - - impl<'tcx> SeenSet<'tcx> { - fn new() -> Self { - SeenSet(FxHashMap::default()) - } - /// Mark (a, b) as `Computing`. - fn mark_computing(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) { - self.0.insert((a, b), None); - } - /// Mark (a, b) as `Computed(result)`. - fn mark_computed(&mut self, a: Ty<'tcx>, b: Ty<'tcx>, result: bool) { - *self.0.get_mut(&(a, b)).expect("Missing prior call to mark_computing") = - Some(result); - } - fn get(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> SeenSetResult { - match self.0.get(&(a, b)) { - None => SeenSetResult::Unseen, - Some(None) => SeenSetResult::Computing, - Some(Some(b)) => SeenSetResult::Computed(*b), - } - } - } fn structurally_same_type_impl<'tcx>( - seen_types: &mut SeenSet<'tcx>, + seen_types: &mut FxHashSet<(Ty<'tcx>, Ty<'tcx>)>, cx: &LateContext<'tcx>, a: Ty<'tcx>, b: Ty<'tcx>, ckind: CItemKind, ) -> bool { debug!("structurally_same_type_impl(cx, a = {:?}, b = {:?})", a, b); - match seen_types.get(a, b) { - // If we've already computed the result, just return the memoized result. - SeenSetResult::Computed(result) => return result, - // We are already in the process of computing structural sameness for this type, - // meaning we've found a cycle. The types are structurally same, then. - SeenSetResult::Computing => return true, - // We haven't seen this combination of types at all -- continue on to computing - // their sameness. - SeenSetResult::Unseen => (), + if seen_types.contains(&(a, b)) { + // We've encountered a cycle. There's no point going any further -- the types are + // structurally the same. + return true; } - seen_types.mark_computing(a, b); + seen_types.insert((a, b)); let tcx = cx.tcx; - let result = if a == b || rustc_middle::ty::TyS::same_type(a, b) { + if a == b || rustc_middle::ty::TyS::same_type(a, b) { // All nominally-same types are structurally same, too. true } else { @@ -2333,11 +2293,9 @@ impl ClashingExternDeclarations { // uninitialised memory. _ => compare_layouts(a, b), } - }; - seen_types.mark_computed(a, b, result); - result + } } - let mut seen_types = SeenSet::new(); + let mut seen_types = FxHashSet::default(); structurally_same_type_impl(&mut seen_types, cx, a, b, ckind) } } From 7708abbbef679d208041bff57aa9ad50e9419895 Mon Sep 17 00:00:00 2001 From: jumbatm Date: Sun, 16 Aug 2020 17:14:09 +1000 Subject: [PATCH 16/36] Avoid double hashset lookup. Co-authored-by: Bastian Kauschke --- src/librustc_lint/builtin.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 781ebb6716758..8625dc096d500 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2161,12 +2161,11 @@ impl ClashingExternDeclarations { ckind: CItemKind, ) -> bool { debug!("structurally_same_type_impl(cx, a = {:?}, b = {:?})", a, b); - if seen_types.contains(&(a, b)) { + if !seen_types.insert((a, b)) { // We've encountered a cycle. There's no point going any further -- the types are // structurally the same. return true; } - seen_types.insert((a, b)); let tcx = cx.tcx; if a == b || rustc_middle::ty::TyS::same_type(a, b) { // All nominally-same types are structurally same, too. From 1321a2dce32893588a0ec4ba586c3a04fb5e47cf Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Sun, 16 Aug 2020 18:04:14 +1000 Subject: [PATCH 17/36] Also accept Refs for is_primitive_or_pointer --- src/librustc_lint/builtin.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 8625dc096d500..2439fc6660634 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2184,8 +2184,9 @@ impl ClashingExternDeclarations { }; #[allow(rustc::usage_of_ty_tykind)] - let is_primitive_or_pointer = - |kind: &ty::TyKind<'_>| kind.is_primitive() || matches!(kind, RawPtr(..)); + let is_primitive_or_pointer = |kind: &ty::TyKind<'_>| { + kind.is_primitive() || matches!(kind, RawPtr(..) | Ref(..)) + }; match (a_kind, b_kind) { (Adt(a_def, a_substs), Adt(b_def, b_substs)) => { @@ -2274,8 +2275,8 @@ impl ClashingExternDeclarations { // These definitely should have been caught above. (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), - // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a - // non-null field. + // An Adt and a primitive or pointer type. This can be FFI-safe if non-null + // enum layout optimisation is being applied. (Adt(..), other_kind) | (other_kind, Adt(..)) if is_primitive_or_pointer(other_kind) => { From bc15dd6dde58aaed724cf692c0635060a77fc99c Mon Sep 17 00:00:00 2001 From: jumbatm <30644300+jumbatm@users.noreply.github.com> Date: Tue, 18 Aug 2020 02:00:35 +1000 Subject: [PATCH 18/36] Wrap recursion in `ensure_sufficient_stack`. --- src/librustc_lint/builtin.rs | 179 ++++++++++++++++++----------------- 1 file changed, 91 insertions(+), 88 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 2439fc6660634..147eb8f06c81c 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -29,6 +29,7 @@ use rustc_ast::tokenstream::{TokenStream, TokenTree}; use rustc_ast::visit::{FnCtxt, FnKind}; use rustc_ast_pretty::pprust::{self, expr_to_string}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::{Applicability, DiagnosticBuilder, DiagnosticStyledString}; use rustc_feature::{deprecated_attributes, AttributeGate, AttributeTemplate, AttributeType}; use rustc_feature::{GateIssue, Stability}; @@ -2188,16 +2189,17 @@ impl ClashingExternDeclarations { kind.is_primitive() || matches!(kind, RawPtr(..) | Ref(..)) }; - match (a_kind, b_kind) { - (Adt(a_def, a_substs), Adt(b_def, b_substs)) => { - let a = a.subst(cx.tcx, a_substs); - let b = b.subst(cx.tcx, b_substs); - debug!("Comparing {:?} and {:?}", a, b); + ensure_sufficient_stack(|| { + match (a_kind, b_kind) { + (Adt(a_def, a_substs), Adt(b_def, b_substs)) => { + let a = a.subst(cx.tcx, a_substs); + let b = b.subst(cx.tcx, b_substs); + debug!("Comparing {:?} and {:?}", a, b); - // Grab a flattened representation of all fields. - let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter()); - let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter()); - compare_layouts(a, b) + // Grab a flattened representation of all fields. + let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter()); + let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter()); + compare_layouts(a, b) && a_fields.eq_by( b_fields, |&ty::FieldDef { did: a_did, .. }, @@ -2211,88 +2213,89 @@ impl ClashingExternDeclarations { ) }, ) - } - (Array(a_ty, a_const), Array(b_ty, b_const)) => { - // For arrays, we also check the constness of the type. - a_const.val == b_const.val - && structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) - } - (Slice(a_ty), Slice(b_ty)) => { - structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) - } - (RawPtr(a_tymut), RawPtr(b_tymut)) => { - a_tymut.mutbl == b_tymut.mutbl - && structurally_same_type_impl( - seen_types, - cx, - &a_tymut.ty, - &b_tymut.ty, - ckind, - ) - } - (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => { - // For structural sameness, we don't need the region to be same. - a_mut == b_mut - && structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) - } - (FnDef(..), FnDef(..)) => { - let a_poly_sig = a.fn_sig(tcx); - let b_poly_sig = b.fn_sig(tcx); - - // As we don't compare regions, skip_binder is fine. - let a_sig = a_poly_sig.skip_binder(); - let b_sig = b_poly_sig.skip_binder(); - - (a_sig.abi, a_sig.unsafety, a_sig.c_variadic) - == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic) - && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| { - structurally_same_type_impl(seen_types, cx, a, b, ckind) - }) - && structurally_same_type_impl( - seen_types, - cx, - a_sig.output(), - b_sig.output(), - ckind, - ) - } - (Tuple(a_substs), Tuple(b_substs)) => { - a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| { + } + (Array(a_ty, a_const), Array(b_ty, b_const)) => { + // For arrays, we also check the constness of the type. + a_const.val == b_const.val + && structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + } + (Slice(a_ty), Slice(b_ty)) => { structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) - }) - } - // For these, it's not quite as easy to define structural-sameness quite so easily. - // For the purposes of this lint, take the conservative approach and mark them as - // not structurally same. - (Dynamic(..), Dynamic(..)) - | (Error(..), Error(..)) - | (Closure(..), Closure(..)) - | (Generator(..), Generator(..)) - | (GeneratorWitness(..), GeneratorWitness(..)) - | (Projection(..), Projection(..)) - | (Opaque(..), Opaque(..)) => false, - - // These definitely should have been caught above. - (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), - - // An Adt and a primitive or pointer type. This can be FFI-safe if non-null - // enum layout optimisation is being applied. - (Adt(..), other_kind) | (other_kind, Adt(..)) - if is_primitive_or_pointer(other_kind) => - { - let (primitive, adt) = - if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) }; - if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) { - ty == primitive - } else { - compare_layouts(a, b) } + (RawPtr(a_tymut), RawPtr(b_tymut)) => { + a_tymut.mutbl == b_tymut.mutbl + && structurally_same_type_impl( + seen_types, + cx, + &a_tymut.ty, + &b_tymut.ty, + ckind, + ) + } + (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => { + // For structural sameness, we don't need the region to be same. + a_mut == b_mut + && structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + } + (FnDef(..), FnDef(..)) => { + let a_poly_sig = a.fn_sig(tcx); + let b_poly_sig = b.fn_sig(tcx); + + // As we don't compare regions, skip_binder is fine. + let a_sig = a_poly_sig.skip_binder(); + let b_sig = b_poly_sig.skip_binder(); + + (a_sig.abi, a_sig.unsafety, a_sig.c_variadic) + == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic) + && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| { + structurally_same_type_impl(seen_types, cx, a, b, ckind) + }) + && structurally_same_type_impl( + seen_types, + cx, + a_sig.output(), + b_sig.output(), + ckind, + ) + } + (Tuple(a_substs), Tuple(b_substs)) => { + a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| { + structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) + }) + } + // For these, it's not quite as easy to define structural-sameness quite so easily. + // For the purposes of this lint, take the conservative approach and mark them as + // not structurally same. + (Dynamic(..), Dynamic(..)) + | (Error(..), Error(..)) + | (Closure(..), Closure(..)) + | (Generator(..), Generator(..)) + | (GeneratorWitness(..), GeneratorWitness(..)) + | (Projection(..), Projection(..)) + | (Opaque(..), Opaque(..)) => false, + + // These definitely should have been caught above. + (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), + + // An Adt and a primitive or pointer type. This can be FFI-safe if non-null + // enum layout optimisation is being applied. + (Adt(..), other_kind) | (other_kind, Adt(..)) + if is_primitive_or_pointer(other_kind) => + { + let (primitive, adt) = + if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) }; + if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) { + ty == primitive + } else { + compare_layouts(a, b) + } + } + // Otherwise, just compare the layouts. This may fail to lint for some + // incompatible types, but at the very least, will stop reads into + // uninitialised memory. + _ => compare_layouts(a, b), } - // Otherwise, just compare the layouts. This may fail to lint for some - // incompatible types, but at the very least, will stop reads into - // uninitialised memory. - _ => compare_layouts(a, b), - } + }) } } let mut seen_types = FxHashSet::default(); From 431a465a8fea2d1939810d9a74f9d3055b85f63e Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Mon, 17 Aug 2020 22:17:14 +0200 Subject: [PATCH 19/36] Move to intra doc links for keyword documentation --- library/std/src/keyword_docs.rs | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/library/std/src/keyword_docs.rs b/library/std/src/keyword_docs.rs index c39989a60c92b..af25c39fccfd0 100644 --- a/library/std/src/keyword_docs.rs +++ b/library/std/src/keyword_docs.rs @@ -98,7 +98,6 @@ mod as_keyword {} /// [Reference on "break expression"]: ../reference/expressions/loop-expr.html#break-expressions /// [Reference on "break and loop values"]: /// ../reference/expressions/loop-expr.html#break-and-loop-values -/// mod break_keyword {} #[doc(keyword = "const")] @@ -336,7 +335,6 @@ mod else_keyword {} /// For more information, take a look at the [Rust Book] or the [Reference] /// /// [ADT]: https://en.wikipedia.org/wiki/Algebraic_data_type -/// [`Option`]: option/enum.Option.html /// [Rust Book]: ../book/ch06-01-defining-an-enum.html /// [Reference]: ../reference/items/enumerations.html mod enum_keyword {} @@ -534,7 +532,6 @@ mod fn_keyword {} /// [`in`]: keyword.in.html /// [`impl`]: keyword.impl.html /// [higher-ranked trait bounds]: ../reference/trait-bounds.html#higher-ranked-trait-bounds -/// [`IntoIterator`]: iter/trait.IntoIterator.html /// [Rust book]: /// ../book/ch03-05-control-flow.html#looping-through-a-collection-with-for /// [Reference]: ../reference/expressions/loop-expr.html#iterator-loops @@ -993,7 +990,6 @@ mod mod_keyword {} /// For more information on the `move` keyword, see the [closure]'s section /// of the Rust book or the [threads] section /// -/// [`Fn` trait]: ../std/ops/trait.Fn.html /// [closure]: ../book/ch13-01-closures.html /// [threads]: ../book/ch16-01-threads.html#using-move-closures-with-threads mod move_keyword {} @@ -1413,9 +1409,7 @@ mod self_upper_keyword {} /// [`extern`]: keyword.extern.html /// [`mut`]: keyword.mut.html /// [`unsafe`]: keyword.unsafe.html -/// [`drop`]: mem/fn.drop.html -/// [`Sync`]: marker/trait.Sync.html -/// [`RefCell`]: cell/struct.RefCell.html +/// [`RefCell`]: cell::RefCell /// [Reference]: ../reference/items/static-items.html mod static_keyword {} @@ -1522,7 +1516,7 @@ mod static_keyword {} /// For more information on structs, take a look at the [Rust Book][book] or the /// [Reference][reference]. /// -/// [`PhantomData`]: marker/struct.PhantomData.html +/// [`PhantomData`]: marker::PhantomData /// [book]: ../book/ch05-01-defining-structs.html /// [reference]: ../reference/items/structs.html mod struct_keyword {} @@ -1733,8 +1727,6 @@ mod super_keyword {} /// [`for`]: keyword.for.html /// [`impl`]: keyword.impl.html /// [`unsafe`]: keyword.unsafe.html -/// [`Send`]: marker/trait.Send.html -/// [`Sync`]: marker/trait.Sync.html /// [Ref-Traits]: ../reference/items/traits.html /// [Ref-Trait-Objects]: ../reference/types/trait-object.html mod trait_keyword {} @@ -1764,7 +1756,6 @@ mod trait_keyword {} /// [`while`]: keyword.while.html /// [`match`]: ../reference/expressions/match-expr.html#match-guards /// [`false`]: keyword.false.html -/// [`bool`]: primitive.bool.html mod true_keyword {} #[doc(keyword = "type")] @@ -1986,9 +1977,6 @@ mod type_keyword {} /// [`static`]: keyword.static.html /// [`union`]: keyword.union.html /// [`impl`]: keyword.impl.html -/// [Send]: marker/trait.Send.html -/// [Sync]: marker/trait.Sync.html -/// [`Vec::set_len`]: vec/struct.Vec.html#method.set_len /// [raw pointers]: ../reference/types/pointer.html /// [memory safety]: ../book/ch19-01-unsafe-rust.html /// [Rustnomicon]: ../nomicon/index.html @@ -2178,7 +2166,7 @@ mod where_keyword {} /// /// It is available for use in stable rust from version 1.39 onwards. /// -/// [`Future`]: ./future/trait.Future.html +/// [`Future`]: future::Future /// [async book]: https://rust-lang.github.io/async-book/ mod async_keyword {} @@ -2197,7 +2185,7 @@ mod async_keyword {} /// /// It is available for use in stable rust from version 1.39 onwards. /// -/// [`Future`]: ./future/trait.Future.html +/// [`Future`]: future::Future /// [async book]: https://rust-lang.github.io/async-book/ mod await_keyword {} From a4995fc8bd9febe40119b2294716ab87b1de62e5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 18:03:38 -0400 Subject: [PATCH 20/36] Refactor `impl_for_type` into a separate function --- src/librustdoc/clean/utils.rs | 63 +++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index a7d03fcabf546..0c6234f69ed6d 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -15,7 +15,7 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_middle::mir::interpret::{sign_extend, ConstValue, Scalar}; use rustc_middle::ty::subst::{GenericArgKind, SubstsRef}; -use rustc_middle::ty::{self, DefIdTree, Ty}; +use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt}; use rustc_span::symbol::{kw, sym, Symbol}; use std::mem; @@ -349,8 +349,39 @@ pub fn qpath_to_string(p: &hir::QPath<'_>) -> String { s } -pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut Vec) { +pub fn impl_for_type(tcx: TyCtxt<'_>, primitive: PrimitiveType) -> Option { use self::PrimitiveType::*; + + match primitive { + Isize => tcx.lang_items().isize_impl(), + I8 => tcx.lang_items().i8_impl(), + I16 => tcx.lang_items().i16_impl(), + I32 => tcx.lang_items().i32_impl(), + I64 => tcx.lang_items().i64_impl(), + I128 => tcx.lang_items().i128_impl(), + Usize => tcx.lang_items().usize_impl(), + U8 => tcx.lang_items().u8_impl(), + U16 => tcx.lang_items().u16_impl(), + U32 => tcx.lang_items().u32_impl(), + U64 => tcx.lang_items().u64_impl(), + U128 => tcx.lang_items().u128_impl(), + F32 => tcx.lang_items().f32_impl(), + F64 => tcx.lang_items().f64_impl(), + Char => tcx.lang_items().char_impl(), + Bool => tcx.lang_items().bool_impl(), + Str => tcx.lang_items().str_impl(), + Slice => tcx.lang_items().slice_impl(), + Array => tcx.lang_items().array_impl(), + Tuple => None, + Unit => None, + RawPointer => tcx.lang_items().const_ptr_impl(), + Reference => None, + Fn => None, + Never => None, + } +} + +pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut Vec) { let tcx = cx.tcx; for item in items { @@ -369,33 +400,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V None => continue, }, }; - let did = match primitive { - Isize => tcx.lang_items().isize_impl(), - I8 => tcx.lang_items().i8_impl(), - I16 => tcx.lang_items().i16_impl(), - I32 => tcx.lang_items().i32_impl(), - I64 => tcx.lang_items().i64_impl(), - I128 => tcx.lang_items().i128_impl(), - Usize => tcx.lang_items().usize_impl(), - U8 => tcx.lang_items().u8_impl(), - U16 => tcx.lang_items().u16_impl(), - U32 => tcx.lang_items().u32_impl(), - U64 => tcx.lang_items().u64_impl(), - U128 => tcx.lang_items().u128_impl(), - F32 => tcx.lang_items().f32_impl(), - F64 => tcx.lang_items().f64_impl(), - Char => tcx.lang_items().char_impl(), - Bool => tcx.lang_items().bool_impl(), - Str => tcx.lang_items().str_impl(), - Slice => tcx.lang_items().slice_impl(), - Array => tcx.lang_items().array_impl(), - Tuple => None, - Unit => None, - RawPointer => tcx.lang_items().const_ptr_impl(), - Reference => None, - Fn => None, - Never => None, - }; + let did = impl_for_type(tcx, primitive); if let Some(did) = did { if !did.is_local() { inline::build_impl(cx, did, None, ret); From 833bbb197f49838d0afe34b9faf59392b18531ca Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 18:06:46 -0400 Subject: [PATCH 21/36] Say `tcx.lang_items()` less --- src/librustdoc/clean/utils.rs | 41 ++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 0c6234f69ed6d..e911a406d6cae 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -352,29 +352,30 @@ pub fn qpath_to_string(p: &hir::QPath<'_>) -> String { pub fn impl_for_type(tcx: TyCtxt<'_>, primitive: PrimitiveType) -> Option { use self::PrimitiveType::*; + let lang_items = tcx.lang_items(); match primitive { - Isize => tcx.lang_items().isize_impl(), - I8 => tcx.lang_items().i8_impl(), - I16 => tcx.lang_items().i16_impl(), - I32 => tcx.lang_items().i32_impl(), - I64 => tcx.lang_items().i64_impl(), - I128 => tcx.lang_items().i128_impl(), - Usize => tcx.lang_items().usize_impl(), - U8 => tcx.lang_items().u8_impl(), - U16 => tcx.lang_items().u16_impl(), - U32 => tcx.lang_items().u32_impl(), - U64 => tcx.lang_items().u64_impl(), - U128 => tcx.lang_items().u128_impl(), - F32 => tcx.lang_items().f32_impl(), - F64 => tcx.lang_items().f64_impl(), - Char => tcx.lang_items().char_impl(), - Bool => tcx.lang_items().bool_impl(), - Str => tcx.lang_items().str_impl(), - Slice => tcx.lang_items().slice_impl(), - Array => tcx.lang_items().array_impl(), + Isize => lang_items.isize_impl(), + I8 => lang_items.i8_impl(), + I16 => lang_items.i16_impl(), + I32 => lang_items.i32_impl(), + I64 => lang_items.i64_impl(), + I128 => lang_items.i128_impl(), + Usize => lang_items.usize_impl(), + U8 => lang_items.u8_impl(), + U16 => lang_items.u16_impl(), + U32 => lang_items.u32_impl(), + U64 => lang_items.u64_impl(), + U128 => lang_items.u128_impl(), + F32 => lang_items.f32_impl(), + F64 => lang_items.f64_impl(), + Char => lang_items.char_impl(), + Bool => lang_items.bool_impl(), + Str => lang_items.str_impl(), + Slice => lang_items.slice_impl(), + Array => lang_items.array_impl(), Tuple => None, Unit => None, - RawPointer => tcx.lang_items().const_ptr_impl(), + RawPointer => lang_items.const_ptr_impl(), Reference => None, Fn => None, Never => None, From dae3a4fe6adb4e6180c462803b223134715274d4 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 18:21:31 -0400 Subject: [PATCH 22/36] Return all impls, not just the primary one --- Cargo.lock | 1 + src/librustdoc/Cargo.toml | 1 + src/librustdoc/clean/utils.rs | 41 ++++++++++++++++++++++++++--------- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c1fa3bef07c7e..3c46cba4c0760 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4052,6 +4052,7 @@ dependencies = [ "rustc-rayon", "serde", "serde_json", + "smallvec 1.4.0", "tempfile", ] diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index 4af13e4cd587a..1354ef5cbdeb4 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -14,5 +14,6 @@ minifier = "0.0.33" rayon = { version = "0.3.0", package = "rustc-rayon" } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +smallvec = "1.0" tempfile = "3" itertools = "0.8" diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index e911a406d6cae..cd022491095a7 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -17,6 +17,7 @@ use rustc_middle::mir::interpret::{sign_extend, ConstValue, Scalar}; use rustc_middle::ty::subst::{GenericArgKind, SubstsRef}; use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt}; use rustc_span::symbol::{kw, sym, Symbol}; +use smallvec::SmallVec; use std::mem; pub fn krate(mut cx: &mut DocContext<'_>) -> Crate { @@ -349,11 +350,14 @@ pub fn qpath_to_string(p: &hir::QPath<'_>) -> String { s } -pub fn impl_for_type(tcx: TyCtxt<'_>, primitive: PrimitiveType) -> Option { +pub fn impl_for_type(tcx: TyCtxt<'_>, primitive: PrimitiveType) -> SmallVec<[DefId; 4]> { use self::PrimitiveType::*; + let both = + |a: Option, b: Option| -> SmallVec<_> { a.into_iter().chain(b).collect() }; + let lang_items = tcx.lang_items(); - match primitive { + let primary_impl = match primitive { Isize => lang_items.isize_impl(), I8 => lang_items.i8_impl(), I16 => lang_items.i16_impl(), @@ -366,20 +370,38 @@ pub fn impl_for_type(tcx: TyCtxt<'_>, primitive: PrimitiveType) -> Option U32 => lang_items.u32_impl(), U64 => lang_items.u64_impl(), U128 => lang_items.u128_impl(), - F32 => lang_items.f32_impl(), - F64 => lang_items.f64_impl(), + F32 => return both(lang_items.f32_impl(), lang_items.f32_runtime_impl()), + F64 => return both(lang_items.f64_impl(), lang_items.f64_runtime_impl()), Char => lang_items.char_impl(), Bool => lang_items.bool_impl(), - Str => lang_items.str_impl(), - Slice => lang_items.slice_impl(), + Str => return both(lang_items.str_impl(), lang_items.str_alloc_impl()), + Slice => { + return lang_items + .slice_impl() + .into_iter() + .chain(lang_items.slice_u8_impl()) + .chain(lang_items.slice_alloc_impl()) + .chain(lang_items.slice_u8_alloc_impl()) + .collect(); + } Array => lang_items.array_impl(), Tuple => None, Unit => None, - RawPointer => lang_items.const_ptr_impl(), + RawPointer => { + return lang_items + .const_ptr_impl() + .into_iter() + .chain(lang_items.mut_ptr_impl()) + .chain(lang_items.const_slice_ptr_impl()) + .chain(lang_items.mut_slice_ptr_impl()) + .collect(); + } Reference => None, Fn => None, Never => None, - } + }; + + primary_impl.into_iter().collect() } pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut Vec) { @@ -401,8 +423,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V None => continue, }, }; - let did = impl_for_type(tcx, primitive); - if let Some(did) = did { + for did in impl_for_type(tcx, primitive) { if !did.is_local() { inline::build_impl(cx, did, None, ret); } From c6ac860b32fd2b295342b5964c71ea4da77c0cb1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 18:26:10 -0400 Subject: [PATCH 23/36] impl_for_type -> PrimitiveType::impls --- src/librustdoc/clean/types.rs | 57 +++++++++++++++++++++++++++++++++ src/librustdoc/clean/utils.rs | 59 ++--------------------------------- 2 files changed, 59 insertions(+), 57 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 627f88df45c9f..a3c56fcb0b0b8 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -19,12 +19,14 @@ use rustc_hir::lang_items; use rustc_hir::Mutability; use rustc_index::vec::IndexVec; use rustc_middle::middle::stability; +use rustc_middle::ty::TyCtxt; use rustc_span::hygiene::MacroKind; use rustc_span::source_map::DUMMY_SP; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{self, FileName}; use rustc_target::abi::VariantIdx; use rustc_target::spec::abi::Abi; +use smallvec::SmallVec; use crate::clean::cfg::Cfg; use crate::clean::external_path; @@ -1264,6 +1266,61 @@ impl PrimitiveType { } } + pub fn impls(&self, tcx: TyCtxt<'_>) -> SmallVec<[DefId; 4]> { + use self::PrimitiveType::*; + + let both = + |a: Option, b: Option| -> SmallVec<_> { a.into_iter().chain(b).collect() }; + + let lang_items = tcx.lang_items(); + let primary_impl = match self { + Isize => lang_items.isize_impl(), + I8 => lang_items.i8_impl(), + I16 => lang_items.i16_impl(), + I32 => lang_items.i32_impl(), + I64 => lang_items.i64_impl(), + I128 => lang_items.i128_impl(), + Usize => lang_items.usize_impl(), + U8 => lang_items.u8_impl(), + U16 => lang_items.u16_impl(), + U32 => lang_items.u32_impl(), + U64 => lang_items.u64_impl(), + U128 => lang_items.u128_impl(), + F32 => return both(lang_items.f32_impl(), lang_items.f32_runtime_impl()), + F64 => return both(lang_items.f64_impl(), lang_items.f64_runtime_impl()), + Char => lang_items.char_impl(), + Bool => lang_items.bool_impl(), + Str => return both(lang_items.str_impl(), lang_items.str_alloc_impl()), + Slice => { + return lang_items + .slice_impl() + .into_iter() + .chain(lang_items.slice_u8_impl()) + .chain(lang_items.slice_alloc_impl()) + .chain(lang_items.slice_u8_alloc_impl()) + .collect(); + } + Array => lang_items.array_impl(), + Tuple => None, + Unit => None, + RawPointer => { + return lang_items + .const_ptr_impl() + .into_iter() + .chain(lang_items.mut_ptr_impl()) + .chain(lang_items.const_slice_ptr_impl()) + .chain(lang_items.mut_slice_ptr_impl()) + .collect(); + } + Reference => None, + Fn => None, + Never => None, + }; + + primary_impl.into_iter().collect() + } + + pub fn to_url_str(&self) -> &'static str { self.as_str() } diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index cd022491095a7..b1d56631c2051 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -15,9 +15,8 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_middle::mir::interpret::{sign_extend, ConstValue, Scalar}; use rustc_middle::ty::subst::{GenericArgKind, SubstsRef}; -use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt}; +use rustc_middle::ty::{self, DefIdTree, Ty}; use rustc_span::symbol::{kw, sym, Symbol}; -use smallvec::SmallVec; use std::mem; pub fn krate(mut cx: &mut DocContext<'_>) -> Crate { @@ -350,60 +349,6 @@ pub fn qpath_to_string(p: &hir::QPath<'_>) -> String { s } -pub fn impl_for_type(tcx: TyCtxt<'_>, primitive: PrimitiveType) -> SmallVec<[DefId; 4]> { - use self::PrimitiveType::*; - - let both = - |a: Option, b: Option| -> SmallVec<_> { a.into_iter().chain(b).collect() }; - - let lang_items = tcx.lang_items(); - let primary_impl = match primitive { - Isize => lang_items.isize_impl(), - I8 => lang_items.i8_impl(), - I16 => lang_items.i16_impl(), - I32 => lang_items.i32_impl(), - I64 => lang_items.i64_impl(), - I128 => lang_items.i128_impl(), - Usize => lang_items.usize_impl(), - U8 => lang_items.u8_impl(), - U16 => lang_items.u16_impl(), - U32 => lang_items.u32_impl(), - U64 => lang_items.u64_impl(), - U128 => lang_items.u128_impl(), - F32 => return both(lang_items.f32_impl(), lang_items.f32_runtime_impl()), - F64 => return both(lang_items.f64_impl(), lang_items.f64_runtime_impl()), - Char => lang_items.char_impl(), - Bool => lang_items.bool_impl(), - Str => return both(lang_items.str_impl(), lang_items.str_alloc_impl()), - Slice => { - return lang_items - .slice_impl() - .into_iter() - .chain(lang_items.slice_u8_impl()) - .chain(lang_items.slice_alloc_impl()) - .chain(lang_items.slice_u8_alloc_impl()) - .collect(); - } - Array => lang_items.array_impl(), - Tuple => None, - Unit => None, - RawPointer => { - return lang_items - .const_ptr_impl() - .into_iter() - .chain(lang_items.mut_ptr_impl()) - .chain(lang_items.const_slice_ptr_impl()) - .chain(lang_items.mut_slice_ptr_impl()) - .collect(); - } - Reference => None, - Fn => None, - Never => None, - }; - - primary_impl.into_iter().collect() -} - pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut Vec) { let tcx = cx.tcx; @@ -423,7 +368,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V None => continue, }, }; - for did in impl_for_type(tcx, primitive) { + for did in primitive.impls(tcx) { if !did.is_local() { inline::build_impl(cx, did, None, ret); } From 3b1d2e37a784d6eb578195d3bf34f2b541f4d73e Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 18:45:18 -0400 Subject: [PATCH 24/36] Allow reusing the code in `collect_trait_impls` --- src/librustdoc/clean/types.rs | 129 +++++++++++-------- src/librustdoc/clean/utils.rs | 2 +- src/librustdoc/lib.rs | 1 + src/librustdoc/passes/collect_trait_impls.rs | 35 +---- 4 files changed, 80 insertions(+), 87 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index a3c56fcb0b0b8..eabe60c14cfdf 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -7,6 +7,7 @@ use std::num::NonZeroU32; use std::rc::Rc; use std::sync::Arc; use std::{slice, vec}; +use std::lazy::SyncOnceCell as OnceCell; use rustc_ast::ast::{self, AttrStyle}; use rustc_ast::attr; @@ -26,7 +27,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{self, FileName}; use rustc_target::abi::VariantIdx; use rustc_target::spec::abi::Abi; -use smallvec::SmallVec; +use smallvec::{SmallVec, smallvec}; use crate::clean::cfg::Cfg; use crate::clean::external_path; @@ -1266,61 +1267,85 @@ impl PrimitiveType { } } - pub fn impls(&self, tcx: TyCtxt<'_>) -> SmallVec<[DefId; 4]> { - use self::PrimitiveType::*; - - let both = - |a: Option, b: Option| -> SmallVec<_> { a.into_iter().chain(b).collect() }; - - let lang_items = tcx.lang_items(); - let primary_impl = match self { - Isize => lang_items.isize_impl(), - I8 => lang_items.i8_impl(), - I16 => lang_items.i16_impl(), - I32 => lang_items.i32_impl(), - I64 => lang_items.i64_impl(), - I128 => lang_items.i128_impl(), - Usize => lang_items.usize_impl(), - U8 => lang_items.u8_impl(), - U16 => lang_items.u16_impl(), - U32 => lang_items.u32_impl(), - U64 => lang_items.u64_impl(), - U128 => lang_items.u128_impl(), - F32 => return both(lang_items.f32_impl(), lang_items.f32_runtime_impl()), - F64 => return both(lang_items.f64_impl(), lang_items.f64_runtime_impl()), - Char => lang_items.char_impl(), - Bool => lang_items.bool_impl(), - Str => return both(lang_items.str_impl(), lang_items.str_alloc_impl()), - Slice => { - return lang_items - .slice_impl() - .into_iter() - .chain(lang_items.slice_u8_impl()) - .chain(lang_items.slice_alloc_impl()) - .chain(lang_items.slice_u8_alloc_impl()) - .collect(); + pub fn impls(&self, tcx: TyCtxt<'_>) -> &SmallVec<[DefId; 4]> { + Self::all_impls(tcx).get(self).expect("missing impl for primitive type") + } + + pub fn all_impls(tcx: TyCtxt<'_>) -> &'static FxHashMap> { + static CELL: OnceCell>> = OnceCell::new(); + + CELL.get_or_init(move || { + use self::PrimitiveType::*; + + /// A macro to create a FxHashMap. + /// + /// Example: + /// + /// ``` + /// let letters = map!{"a" => "b", "c" => "d"}; + /// ``` + /// + /// Trailing commas are allowed. + /// Commas between elements are required (even if the expression is a block). + macro_rules! map { + ($( $key: expr => $val: expr ),* $(,)*) => {{ + let mut map = ::rustc_data_structures::fx::FxHashMap::default(); + $( map.insert($key, $val); )* + map + }} } - Array => lang_items.array_impl(), - Tuple => None, - Unit => None, - RawPointer => { - return lang_items - .const_ptr_impl() - .into_iter() - .chain(lang_items.mut_ptr_impl()) - .chain(lang_items.const_slice_ptr_impl()) - .chain(lang_items.mut_slice_ptr_impl()) - .collect(); - } - Reference => None, - Fn => None, - Never => None, - }; - primary_impl.into_iter().collect() + let single = |a: Option| a.into_iter().collect(); + let both = + |a: Option, b: Option| -> SmallVec<_> { a.into_iter().chain(b).collect() }; + + let lang_items = tcx.lang_items(); + map! { + Isize => single(lang_items.isize_impl()), + I8 => single(lang_items.i8_impl()), + I16 => single(lang_items.i16_impl()), + I32 => single(lang_items.i32_impl()), + I64 => single(lang_items.i64_impl()), + I128 => single(lang_items.i128_impl()), + Usize => single(lang_items.usize_impl()), + U8 => single(lang_items.u8_impl()), + U16 => single(lang_items.u16_impl()), + U32 => single(lang_items.u32_impl()), + U64 => single(lang_items.u64_impl()), + U128 => single(lang_items.u128_impl()), + F32 => both(lang_items.f32_impl(), lang_items.f32_runtime_impl()), + F64 => both(lang_items.f64_impl(), lang_items.f64_runtime_impl()), + Char => single(lang_items.char_impl()), + Bool => single(lang_items.bool_impl()), + Str => both(lang_items.str_impl(), lang_items.str_alloc_impl()), + Slice => { + lang_items + .slice_impl() + .into_iter() + .chain(lang_items.slice_u8_impl()) + .chain(lang_items.slice_alloc_impl()) + .chain(lang_items.slice_u8_alloc_impl()) + .collect() + }, + Array => single(lang_items.array_impl()), + Tuple => smallvec![], + Unit => smallvec![], + RawPointer => { + lang_items + .const_ptr_impl() + .into_iter() + .chain(lang_items.mut_ptr_impl()) + .chain(lang_items.const_slice_ptr_impl()) + .chain(lang_items.mut_slice_ptr_impl()) + .collect() + }, + Reference => smallvec![], + Fn => smallvec![], + Never => smallvec![], + } + }) } - pub fn to_url_str(&self) -> &'static str { self.as_str() } diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index b1d56631c2051..8d06469311829 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -368,7 +368,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V None => continue, }, }; - for did in primitive.impls(tcx) { + for &did in primitive.impls(tcx) { if !did.is_local() { inline::build_impl(cx, did, None, ret); } diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index d5f7ddcbdfbde..3dfa7b529e34c 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -12,6 +12,7 @@ #![feature(ptr_offset_from)] #![feature(crate_visibility_modifier)] #![feature(never_type)] +#![feature(once_cell)] #![recursion_limit = "256"] #[macro_use] diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index a40b45f9a7e2c..24baff46dcfa5 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -34,40 +34,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { } // Also try to inline primitive impls from other crates. - let lang_items = cx.tcx.lang_items(); - let primitive_impls = [ - lang_items.isize_impl(), - lang_items.i8_impl(), - lang_items.i16_impl(), - lang_items.i32_impl(), - lang_items.i64_impl(), - lang_items.i128_impl(), - lang_items.usize_impl(), - lang_items.u8_impl(), - lang_items.u16_impl(), - lang_items.u32_impl(), - lang_items.u64_impl(), - lang_items.u128_impl(), - lang_items.f32_impl(), - lang_items.f64_impl(), - lang_items.f32_runtime_impl(), - lang_items.f64_runtime_impl(), - lang_items.bool_impl(), - lang_items.char_impl(), - lang_items.str_impl(), - lang_items.array_impl(), - lang_items.slice_impl(), - lang_items.slice_u8_impl(), - lang_items.str_alloc_impl(), - lang_items.slice_alloc_impl(), - lang_items.slice_u8_alloc_impl(), - lang_items.const_ptr_impl(), - lang_items.mut_ptr_impl(), - lang_items.const_slice_ptr_impl(), - lang_items.mut_slice_ptr_impl(), - ]; - - for def_id in primitive_impls.iter().filter_map(|&def_id| def_id) { + for &def_id in PrimitiveType::all_impls(cx.tcx).values().flatten() { if !def_id.is_local() { inline::build_impl(cx, def_id, None, &mut new_items); From 1b74f04409c167f27e3bee2141fe78d492769967 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 18:54:29 -0400 Subject: [PATCH 25/36] Use `impls` for intra doc links as well --- src/librustdoc/clean/types.rs | 2 +- .../passes/collect_intra_doc_links.rs | 52 +++++++------------ 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index eabe60c14cfdf..5f262d061fa44 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1267,7 +1267,7 @@ impl PrimitiveType { } } - pub fn impls(&self, tcx: TyCtxt<'_>) -> &SmallVec<[DefId; 4]> { + pub fn impls(&self, tcx: TyCtxt<'_>) -> &'static SmallVec<[DefId; 4]> { Self::all_impls(tcx).get(self).expect("missing impl for primitive type") } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f7fc3579d67a7..84d429cc6a8b8 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -16,6 +16,7 @@ use rustc_span::hygiene::MacroKind; use rustc_span::symbol::Ident; use rustc_span::symbol::Symbol; use rustc_span::DUMMY_SP; +use smallvec::SmallVec; use std::cell::Cell; use std::ops::Range; @@ -270,18 +271,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .ok_or(ErrorKind::ResolutionFailure)?; if let Some(prim) = is_primitive(&path, TypeNS) { - let did = primitive_impl(cx, &path).ok_or(ErrorKind::ResolutionFailure)?; - return cx - .tcx - .associated_items(did) - .filter_by_name_unhygienic(item_name) - .next() - .and_then(|item| match item.kind { - ty::AssocKind::Fn => Some("method"), - _ => None, - }) - .map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_name)))) - .ok_or(ErrorKind::ResolutionFailure); + for &impl_ in primitive_impl(cx, &path).ok_or(ErrorKind::ResolutionFailure)? { + let link = cx + .tcx + .associated_items(impl_) + .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, impl_) + .and_then(|item| match item.kind { + ty::AssocKind::Fn => Some("method"), + _ => None, + }) + .map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_name)))); + if let Some(link) = link { + return Ok(link); + } + } + return Err(ErrorKind::ResolutionFailure); } let (_, ty_res) = cx @@ -1227,26 +1231,6 @@ fn is_primitive(path_str: &str, ns: Namespace) -> Option { if ns == TypeNS { PRIMITIVES.iter().find(|x| x.0 == path_str).map(|x| x.1) } else { None } } -fn primitive_impl(cx: &DocContext<'_>, path_str: &str) -> Option { - let tcx = cx.tcx; - match path_str { - "u8" => tcx.lang_items().u8_impl(), - "u16" => tcx.lang_items().u16_impl(), - "u32" => tcx.lang_items().u32_impl(), - "u64" => tcx.lang_items().u64_impl(), - "u128" => tcx.lang_items().u128_impl(), - "usize" => tcx.lang_items().usize_impl(), - "i8" => tcx.lang_items().i8_impl(), - "i16" => tcx.lang_items().i16_impl(), - "i32" => tcx.lang_items().i32_impl(), - "i64" => tcx.lang_items().i64_impl(), - "i128" => tcx.lang_items().i128_impl(), - "isize" => tcx.lang_items().isize_impl(), - "f32" => tcx.lang_items().f32_impl(), - "f64" => tcx.lang_items().f64_impl(), - "str" => tcx.lang_items().str_impl(), - "bool" => tcx.lang_items().bool_impl(), - "char" => tcx.lang_items().char_impl(), - _ => None, - } +fn primitive_impl(cx: &DocContext<'_>, path_str: &str) -> Option<&'static SmallVec<[DefId; 4]>> { + Some(PrimitiveType::from_symbol(Symbol::intern(path_str))?.impls(cx.tcx)) } From 28580749ccab89f7a81c206263790f6135eac234 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 19:36:29 -0400 Subject: [PATCH 26/36] xpy fmt --- src/librustdoc/clean/types.rs | 9 +++++---- src/librustdoc/passes/collect_intra_doc_links.rs | 7 ++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 5f262d061fa44..094a3954067c4 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -3,11 +3,11 @@ use std::default::Default; use std::fmt; use std::hash::{Hash, Hasher}; use std::iter::FromIterator; +use std::lazy::SyncOnceCell as OnceCell; use std::num::NonZeroU32; use std::rc::Rc; use std::sync::Arc; use std::{slice, vec}; -use std::lazy::SyncOnceCell as OnceCell; use rustc_ast::ast::{self, AttrStyle}; use rustc_ast::attr; @@ -27,7 +27,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{self, FileName}; use rustc_target::abi::VariantIdx; use rustc_target::spec::abi::Abi; -use smallvec::{SmallVec, smallvec}; +use smallvec::{smallvec, SmallVec}; use crate::clean::cfg::Cfg; use crate::clean::external_path; @@ -1296,8 +1296,9 @@ impl PrimitiveType { } let single = |a: Option| a.into_iter().collect(); - let both = - |a: Option, b: Option| -> SmallVec<_> { a.into_iter().chain(b).collect() }; + let both = |a: Option, b: Option| -> SmallVec<_> { + a.into_iter().chain(b).collect() + }; let lang_items = tcx.lang_items(); map! { diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 84d429cc6a8b8..f79639b35d89f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -275,7 +275,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let link = cx .tcx .associated_items(impl_) - .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, impl_) + .find_by_name_and_namespace( + cx.tcx, + Ident::with_dummy_span(item_name), + ns, + impl_, + ) .and_then(|item| match item.kind { ty::AssocKind::Fn => Some("method"), _ => None, From 7be824e7eacf96227ac12527d6ac309c40404f2c Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 19:42:39 -0400 Subject: [PATCH 27/36] Add a test --- .../intra-link-primitive-non-default-impl.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 src/test/rustdoc/intra-link-primitive-non-default-impl.rs diff --git a/src/test/rustdoc/intra-link-primitive-non-default-impl.rs b/src/test/rustdoc/intra-link-primitive-non-default-impl.rs new file mode 100644 index 0000000000000..427cd2124a926 --- /dev/null +++ b/src/test/rustdoc/intra-link-primitive-non-default-impl.rs @@ -0,0 +1,14 @@ +#![deny(broken_intra_doc_links)] + +// ignore-tidy-linelength + +// @has intra_link_primitive_non_default_impl/fn.f.html +/// [`str::trim`] +// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim"]' 'str::trim' +/// [`str::to_lowercase`] +// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html#method.to_lowercase"]' 'str::to_lowercase' +/// [`str::into_boxed_bytes`] +// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html#method.into_boxed_bytes"]' 'str::into_boxed_bytes' +/// [`str::replace`] +// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html#method.replace"]' 'str::replace' +pub fn f() {} From 38b920d16f092b9d8142a075ab1a3552567db9b5 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 4 Aug 2020 18:42:36 -0700 Subject: [PATCH 28/36] Capture output from threads spawned in tests Fixes #42474. --- library/std/src/io/impls.rs | 6 ++-- library/std/src/io/mod.rs | 4 ++- library/std/src/io/stdio.rs | 33 +++++++++++++++++--- library/std/src/panicking.rs | 2 +- library/std/src/thread/mod.rs | 5 +++ library/test/src/helpers/sink.rs | 7 +++++ src/librustc_interface/util.rs | 5 +++ src/test/ui/panic-while-printing.rs | 19 +++++++++-- src/test/ui/test-thread-capture.rs | 30 ++++++++++++++++++ src/test/ui/test-thread-capture.run.stdout | 21 +++++++++++++ src/test/ui/test-thread-nocapture.rs | 30 ++++++++++++++++++ src/test/ui/test-thread-nocapture.run.stderr | 2 ++ src/test/ui/test-thread-nocapture.run.stdout | 20 ++++++++++++ src/test/ui/threads-sendsync/task-stderr.rs | 5 +++ 14 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/test-thread-capture.rs create mode 100644 src/test/ui/test-thread-capture.run.stdout create mode 100644 src/test/ui/test-thread-nocapture.rs create mode 100644 src/test/ui/test-thread-nocapture.run.stderr create mode 100644 src/test/ui/test-thread-nocapture.run.stdout diff --git a/library/std/src/io/impls.rs b/library/std/src/io/impls.rs index 01dff0b3eb390..a13aa61990fc1 100644 --- a/library/std/src/io/impls.rs +++ b/library/std/src/io/impls.rs @@ -210,13 +210,13 @@ impl BufRead for Box { #[cfg(test)] /// This impl is only used by printing logic, so any error returned is always /// of kind `Other`, and should be ignored. -impl Write for Box { +impl Write for dyn ::realstd::io::LocalOutput { fn write(&mut self, buf: &[u8]) -> io::Result { - (**self).write(buf).map_err(|_| ErrorKind::Other.into()) + (*self).write(buf).map_err(|_| ErrorKind::Other.into()) } fn flush(&mut self) -> io::Result<()> { - (**self).flush().map_err(|_| ErrorKind::Other.into()) + (*self).flush().map_err(|_| ErrorKind::Other.into()) } } diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 9eb54c2cc0044..12336dcf9bf3e 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -274,10 +274,12 @@ pub use self::stdio::{StderrLock, StdinLock, StdoutLock}; pub use self::stdio::{_eprint, _print}; #[unstable(feature = "libstd_io_internals", issue = "42788")] #[doc(no_inline, hidden)] -pub use self::stdio::{set_panic, set_print}; +pub use self::stdio::{set_panic, set_print, LocalOutput}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::util::{copy, empty, repeat, sink, Empty, Repeat, Sink}; +pub(crate) use self::stdio::clone_io; + mod buffered; mod cursor; mod error; diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 156f555be02d8..b3a611c73eac4 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -13,14 +13,14 @@ use crate::thread::LocalKey; thread_local! { /// Stdout used by print! and println! macros - static LOCAL_STDOUT: RefCell>> = { + static LOCAL_STDOUT: RefCell>> = { RefCell::new(None) } } thread_local! { /// Stderr used by eprint! and eprintln! macros, and panics - static LOCAL_STDERR: RefCell>> = { + static LOCAL_STDERR: RefCell>> = { RefCell::new(None) } } @@ -903,6 +903,18 @@ impl fmt::Debug for StderrLock<'_> { } } +/// A writer than can be cloned to new threads. +#[unstable( + feature = "set_stdio", + reason = "this trait may disappear completely or be replaced \ + with a more general mechanism", + issue = "none" +)] +#[doc(hidden)] +pub trait LocalOutput: Write + Send { + fn clone_box(&self) -> Box; +} + /// Resets the thread-local stderr handle to the specified writer /// /// This will replace the current thread's stderr handle, returning the old @@ -918,7 +930,7 @@ impl fmt::Debug for StderrLock<'_> { issue = "none" )] #[doc(hidden)] -pub fn set_panic(sink: Option>) -> Option> { +pub fn set_panic(sink: Option>) -> Option> { use crate::mem; LOCAL_STDERR.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)).and_then(|mut s| { let _ = s.flush(); @@ -941,7 +953,7 @@ pub fn set_panic(sink: Option>) -> Option>) -> Option> { +pub fn set_print(sink: Option>) -> Option> { use crate::mem; LOCAL_STDOUT.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)).and_then(|mut s| { let _ = s.flush(); @@ -949,6 +961,17 @@ pub fn set_print(sink: Option>) -> Option (Option>, Option>) { + LOCAL_STDOUT.with(|stdout| { + LOCAL_STDERR.with(|stderr| { + ( + stdout.borrow().as_ref().map(|o| o.clone_box()), + stderr.borrow().as_ref().map(|o| o.clone_box()), + ) + }) + }) +} + /// Write `args` to output stream `local_s` if possible, `global_s` /// otherwise. `label` identifies the stream in a panic message. /// @@ -961,7 +984,7 @@ pub fn set_print(sink: Option>) -> Option( args: fmt::Arguments<'_>, - local_s: &'static LocalKey>>>, + local_s: &'static LocalKey>>>, global_s: fn() -> T, label: &str, ) where diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index ab2a60103069d..0d753b5f6b35b 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -210,7 +210,7 @@ fn default_hook(info: &PanicInfo<'_>) { if let Some(mut local) = set_panic(None) { // NB. In `cfg(test)` this uses the forwarding impl - // for `Box`. + // for `dyn ::realstd::io::LocalOutput`. write(&mut local); set_panic(Some(local)); } else if let Some(mut out) = panic_output() { diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 202867258f1e4..ee37eb9c69a2d 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -465,11 +465,16 @@ impl Builder { let my_packet: Arc>>> = Arc::new(UnsafeCell::new(None)); let their_packet = my_packet.clone(); + let (stdout, stderr) = crate::io::clone_io(); + let main = move || { if let Some(name) = their_thread.cname() { imp::Thread::set_name(name); } + crate::io::set_print(stdout); + crate::io::set_panic(stderr); + thread_info::set(imp::guard::current(), their_thread); let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { crate::sys_common::backtrace::__rust_begin_short_backtrace(f) diff --git a/library/test/src/helpers/sink.rs b/library/test/src/helpers/sink.rs index aa7fe2487730e..de63faeb4beab 100644 --- a/library/test/src/helpers/sink.rs +++ b/library/test/src/helpers/sink.rs @@ -6,6 +6,7 @@ use std::{ sync::{Arc, Mutex}, }; +#[derive(Clone)] pub struct Sink(Arc>>); impl Sink { @@ -14,6 +15,12 @@ impl Sink { } } +impl io::LocalOutput for Sink { + fn clone_box(&self) -> Box { + Box::new(Self(self.0.clone())) + } +} + impl Write for Sink { fn write(&mut self, data: &[u8]) -> io::Result { Write::write(&mut *self.0.lock().unwrap(), data) diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index bbb2f9d8b2500..c22934ed1a613 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -100,6 +100,11 @@ impl Write for Sink { Ok(()) } } +impl io::LocalOutput for Sink { + fn clone_box(&self) -> Box { + Box::new(Self(self.0.clone())) + } +} /// Like a `thread::Builder::spawn` followed by a `join()`, but avoids the need /// for `'static` bounds. diff --git a/src/test/ui/panic-while-printing.rs b/src/test/ui/panic-while-printing.rs index 7e9fa16b0847a..21fc12759f87c 100644 --- a/src/test/ui/panic-while-printing.rs +++ b/src/test/ui/panic-while-printing.rs @@ -5,7 +5,7 @@ use std::fmt; use std::fmt::{Display, Formatter}; -use std::io::set_panic; +use std::io::{self, set_panic, LocalOutput, Write}; pub struct A; @@ -15,8 +15,23 @@ impl Display for A { } } +struct Sink; +impl Write for Sink { + fn write(&mut self, buf: &[u8]) -> io::Result { + Ok(buf.len()) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} +impl LocalOutput for Sink { + fn clone_box(&self) -> Box { + Box::new(Sink) + } +} + fn main() { - set_panic(Some(Box::new(Vec::new()))); + set_panic(Some(Box::new(Sink))); assert!(std::panic::catch_unwind(|| { eprintln!("{}", A); }) diff --git a/src/test/ui/test-thread-capture.rs b/src/test/ui/test-thread-capture.rs new file mode 100644 index 0000000000000..2f8f3163deb9e --- /dev/null +++ b/src/test/ui/test-thread-capture.rs @@ -0,0 +1,30 @@ +// compile-flags: --test +// run-fail +// run-flags: --test-threads=1 +// check-run-results +// exec-env:RUST_BACKTRACE=0 + +#[test] +fn thready_pass() { + println!("fee"); + std::thread::spawn(|| { + println!("fie"); + println!("foe"); + }) + .join() + .unwrap(); + println!("fum"); +} + +#[test] +fn thready_fail() { + println!("fee"); + std::thread::spawn(|| { + println!("fie"); + println!("foe"); + }) + .join() + .unwrap(); + println!("fum"); + panic!(); +} diff --git a/src/test/ui/test-thread-capture.run.stdout b/src/test/ui/test-thread-capture.run.stdout new file mode 100644 index 0000000000000..6a49de5f4ab9e --- /dev/null +++ b/src/test/ui/test-thread-capture.run.stdout @@ -0,0 +1,21 @@ + +running 2 tests +test thready_fail ... FAILED +test thready_pass ... ok + +failures: + +---- thready_fail stdout ---- +fee +fie +foe +fum +thread 'main' panicked at 'explicit panic', $DIR/test-thread-capture.rs:29:5 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + +failures: + thready_fail + +test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out + diff --git a/src/test/ui/test-thread-nocapture.rs b/src/test/ui/test-thread-nocapture.rs new file mode 100644 index 0000000000000..c107c6c9dd327 --- /dev/null +++ b/src/test/ui/test-thread-nocapture.rs @@ -0,0 +1,30 @@ +// compile-flags: --test +// run-fail +// run-flags: --test-threads=1 --nocapture +// check-run-results +// exec-env:RUST_BACKTRACE=0 + +#[test] +fn thready_pass() { + println!("fee"); + std::thread::spawn(|| { + println!("fie"); + println!("foe"); + }) + .join() + .unwrap(); + println!("fum"); +} + +#[test] +fn thready_fail() { + println!("fee"); + std::thread::spawn(|| { + println!("fie"); + println!("foe"); + }) + .join() + .unwrap(); + println!("fum"); + panic!(); +} diff --git a/src/test/ui/test-thread-nocapture.run.stderr b/src/test/ui/test-thread-nocapture.run.stderr new file mode 100644 index 0000000000000..ce5244c13ab26 --- /dev/null +++ b/src/test/ui/test-thread-nocapture.run.stderr @@ -0,0 +1,2 @@ +thread 'main' panicked at 'explicit panic', $DIR/test-thread-nocapture.rs:29:5 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/src/test/ui/test-thread-nocapture.run.stdout b/src/test/ui/test-thread-nocapture.run.stdout new file mode 100644 index 0000000000000..77b42ed88d63f --- /dev/null +++ b/src/test/ui/test-thread-nocapture.run.stdout @@ -0,0 +1,20 @@ + +running 2 tests +test thready_fail ... fee +fie +foe +fum +FAILED +test thready_pass ... fee +fie +foe +fum +ok + +failures: + +failures: + thready_fail + +test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out + diff --git a/src/test/ui/threads-sendsync/task-stderr.rs b/src/test/ui/threads-sendsync/task-stderr.rs index d474084bf20ce..bc4bedac196ec 100644 --- a/src/test/ui/threads-sendsync/task-stderr.rs +++ b/src/test/ui/threads-sendsync/task-stderr.rs @@ -16,6 +16,11 @@ impl Write for Sink { } fn flush(&mut self) -> io::Result<()> { Ok(()) } } +impl io::LocalOutput for Sink { + fn clone_box(&self) -> Box { + Box::new(Sink(self.0.clone())) + } +} fn main() { let data = Arc::new(Mutex::new(Vec::new())); From e24aa733f1e3eb207106b1053e8dee50e6289b30 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 15 Jun 2020 11:50:58 -0400 Subject: [PATCH 29/36] rewrite old test so that its attributes are consistent with what we want in the language. (Note that the fact this test existed is a slight sign that we may need a crater run on this bugfix...) --- .../ui/rfc-2565-param-attrs/param-attrs-allowed.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs b/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs index 1217f89cb3168..a547d09d04822 100644 --- a/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs +++ b/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs @@ -8,8 +8,8 @@ extern "C" { #[allow(unused_mut)] a: i32, #[cfg(something)] b: i32, #[cfg_attr(something, cfg(nothing))] c: i32, - #[deny(unused_mut)] d: i32, - #[forbid(unused_mut)] #[warn(unused_mut)] ... + #[forbid(unused_mut)] d: i32, + #[deny(unused_mut)] #[warn(unused_mut)] ... ); } @@ -17,16 +17,16 @@ type FnType = fn( #[allow(unused_mut)] a: i32, #[cfg(something)] b: i32, #[cfg_attr(something, cfg(nothing))] c: i32, - #[deny(unused_mut)] d: i32, - #[forbid(unused_mut)] #[warn(unused_mut)] e: i32 + #[forbid(unused_mut)] d: i32, + #[deny(unused_mut)] #[warn(unused_mut)] e: i32 ); pub fn foo( #[allow(unused_mut)] a: i32, #[cfg(something)] b: i32, #[cfg_attr(something, cfg(nothing))] c: i32, - #[deny(unused_mut)] d: i32, - #[forbid(unused_mut)] #[warn(unused_mut)] _e: i32 + #[forbid(unused_mut)] d: i32, + #[deny(unused_mut)] #[warn(unused_mut)] _e: i32 ) {} // self From 01e8e9ffaa4243827cde09844ad39540323e3b88 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 15 Jun 2020 14:17:35 -0400 Subject: [PATCH 30/36] When building lint specs map for a given scope, check if forbid present on each insert. Drive-by changes: 1. make `LintLevelsBuilder::push` private to the crate. 2. Add methods to `LintSource` for extracting its name symbol or its span. --- src/librustc_lint/levels.rs | 47 +++++++++++++++++++++++++++++++++---- src/librustc_middle/lint.rs | 20 +++++++++++++++- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/librustc_lint/levels.rs b/src/librustc_lint/levels.rs index 48254dcee82fe..222333a578b7d 100644 --- a/src/librustc_lint/levels.rs +++ b/src/librustc_lint/levels.rs @@ -10,6 +10,7 @@ use rustc_hir as hir; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; use rustc_hir::{intravisit, HirId}; use rustc_middle::hir::map::Map; +use rustc_middle::lint::LevelSource; use rustc_middle::lint::LintDiagnosticBuilder; use rustc_middle::lint::{struct_lint_level, LintLevelMap, LintLevelSets, LintSet, LintSource}; use rustc_middle::ty::query::Providers; @@ -95,6 +96,44 @@ impl<'s> LintLevelsBuilder<'s> { self.sets.list.push(LintSet::CommandLine { specs }); } + /// Attempts to insert the `id` to `level_src` map entry. If unsuccessful + /// (e.g. if a forbid was already inserted on the same scope), then emits a + /// diagnostic with no change to `specs`. + fn insert_spec( + &mut self, + specs: &mut FxHashMap, + id: LintId, + (level, src): LevelSource, + ) { + if let Some((old_level, old_src)) = specs.get(&id) { + if old_level == &Level::Forbid && level != Level::Forbid { + let mut diag_builder = struct_span_err!( + self.sess, + src.span(), + E0453, + "{}({}) incompatible with previous forbid in same scope", + level.as_str(), + src.name(), + ); + match *old_src { + LintSource::Default => {} + LintSource::Node(_, forbid_source_span, reason) => { + diag_builder.span_label(forbid_source_span, "`forbid` level set here"); + if let Some(rationale) = reason { + diag_builder.note(&rationale.as_str()); + } + } + LintSource::CommandLine(_) => { + diag_builder.note("`forbid` lint level was set on command line"); + } + } + diag_builder.emit(); + return; + } + } + specs.insert(id, (level, src)); + } + /// Pushes a list of AST lint attributes onto this context. /// /// This function will return a `BuilderPush` object which should be passed @@ -109,7 +148,7 @@ impl<'s> LintLevelsBuilder<'s> { /// `#[allow]` /// /// Don't forget to call `pop`! - pub fn push( + pub(crate) fn push( &mut self, attrs: &[ast::Attribute], store: &LintStore, @@ -221,7 +260,7 @@ impl<'s> LintLevelsBuilder<'s> { let src = LintSource::Node(name, li.span(), reason); for &id in ids { self.check_gated_lint(id, attr.span); - specs.insert(id, (level, src)); + self.insert_spec(&mut specs, id, (level, src)); } } @@ -235,7 +274,7 @@ impl<'s> LintLevelsBuilder<'s> { reason, ); for id in ids { - specs.insert(*id, (level, src)); + self.insert_spec(&mut specs, *id, (level, src)); } } Err((Some(ids), new_lint_name)) => { @@ -272,7 +311,7 @@ impl<'s> LintLevelsBuilder<'s> { reason, ); for id in ids { - specs.insert(*id, (level, src)); + self.insert_spec(&mut specs, *id, (level, src)); } } Err((None, _)) => { diff --git a/src/librustc_middle/lint.rs b/src/librustc_middle/lint.rs index 25e5379881e70..7be7d5c07b4a8 100644 --- a/src/librustc_middle/lint.rs +++ b/src/librustc_middle/lint.rs @@ -9,7 +9,7 @@ use rustc_session::lint::{builtin, Level, Lint, LintId}; use rustc_session::{DiagnosticMessageId, Session}; use rustc_span::hygiene::MacroKind; use rustc_span::source_map::{DesugaringKind, ExpnKind, MultiSpan}; -use rustc_span::{Span, Symbol}; +use rustc_span::{Span, Symbol, DUMMY_SP}; /// How a lint level was set. #[derive(Clone, Copy, PartialEq, Eq, HashStable)] @@ -25,6 +25,24 @@ pub enum LintSource { CommandLine(Symbol), } +impl LintSource { + pub fn name(&self) -> Symbol { + match *self { + LintSource::Default => Symbol::intern("default"), + LintSource::Node(name, _, _) => name, + LintSource::CommandLine(name) => name, + } + } + + pub fn span(&self) -> Span { + match *self { + LintSource::Default => DUMMY_SP, + LintSource::Node(_, span, _) => span, + LintSource::CommandLine(_) => DUMMY_SP, + } + } +} + pub type LevelSource = (Level, LintSource); pub struct LintLevelSets { From ef03a5d059fc77196613fafdefffe990b82f515c Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 15 Jun 2020 14:17:41 -0400 Subject: [PATCH 31/36] Regression test. --- ...0819-dont-override-forbid-in-same-scope.rs | 49 +++++++++++++++++++ ...-dont-override-forbid-in-same-scope.stderr | 29 +++++++++++ 2 files changed, 78 insertions(+) create mode 100644 src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs create mode 100644 src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr diff --git a/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs b/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs new file mode 100644 index 0000000000000..8e25227b59e63 --- /dev/null +++ b/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs @@ -0,0 +1,49 @@ +// This test is checking that you cannot override a `forbid` by adding in other +// attributes later in the same scope. (We already ensure that you cannot +// override it in nested scopes). + +// If you turn off deduplicate diagnostics (which rustc turns on by default but +// compiletest turns off when it runs ui tests), then the errors are +// (unfortunately) repeated here because the checking is done as we read in the +// errors, and curretly that happens two or three different times, depending on +// compiler flags. +// +// I decided avoiding the redundant output was not worth the time in engineering +// effort for bug like this, which 1. end users are unlikely to run into in the +// first place, and 2. they won't see the redundant output anyway. + +// compile-flags: -Z deduplicate-diagnostics=yes + +fn forbid_first(num: i32) -> i32 { + #![forbid(unused)] + #![deny(unused)] + //~^ ERROR: deny(unused) incompatible with previous forbid in same scope [E0453] + #![warn(unused)] + //~^ ERROR: warn(unused) incompatible with previous forbid in same scope [E0453] + #![allow(unused)] + //~^ ERROR: allow(unused) incompatible with previous forbid in same scope [E0453] + + num * num +} + +fn forbid_last(num: i32) -> i32 { + #![deny(unused)] + #![warn(unused)] + #![allow(unused)] + #![forbid(unused)] + + num * num +} + +fn forbid_multiple(num: i32) -> i32 { + #![forbid(unused)] + #![forbid(unused)] + + num * num +} + +fn main() { + forbid_first(10); + forbid_last(10); + forbid_multiple(10); +} diff --git a/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr b/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr new file mode 100644 index 0000000000000..3951c511bf432 --- /dev/null +++ b/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr @@ -0,0 +1,29 @@ +error[E0453]: deny(unused) incompatible with previous forbid in same scope + --> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:19:13 + | +LL | #![forbid(unused)] + | ------ `forbid` level set here +LL | #![deny(unused)] + | ^^^^^^ + +error[E0453]: warn(unused) incompatible with previous forbid in same scope + --> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:21:13 + | +LL | #![forbid(unused)] + | ------ `forbid` level set here +... +LL | #![warn(unused)] + | ^^^^^^ + +error[E0453]: allow(unused) incompatible with previous forbid in same scope + --> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:23:14 + | +LL | #![forbid(unused)] + | ------ `forbid` level set here +... +LL | #![allow(unused)] + | ^^^^^^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0453`. From 4ec4c4f04ebe8514d3f164b1ec7c78773ef0a5c7 Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Tue, 30 Jun 2020 12:10:52 -0400 Subject: [PATCH 32/36] review suggestion: use existing defn rather than new intern call Co-authored-by: Vadim Petrochenkov --- src/librustc_middle/lint.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_middle/lint.rs b/src/librustc_middle/lint.rs index 7be7d5c07b4a8..91e1d6e0b0b72 100644 --- a/src/librustc_middle/lint.rs +++ b/src/librustc_middle/lint.rs @@ -9,7 +9,7 @@ use rustc_session::lint::{builtin, Level, Lint, LintId}; use rustc_session::{DiagnosticMessageId, Session}; use rustc_span::hygiene::MacroKind; use rustc_span::source_map::{DesugaringKind, ExpnKind, MultiSpan}; -use rustc_span::{Span, Symbol, DUMMY_SP}; +use rustc_span::{symbol, Span, Symbol, DUMMY_SP}; /// How a lint level was set. #[derive(Clone, Copy, PartialEq, Eq, HashStable)] @@ -28,7 +28,7 @@ pub enum LintSource { impl LintSource { pub fn name(&self) -> Symbol { match *self { - LintSource::Default => Symbol::intern("default"), + LintSource::Default => symbol::kw::Default, LintSource::Node(name, _, _) => name, LintSource::CommandLine(name) => name, } From 8c4641b37f84f5491d07789d87aa31d835353e60 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Tue, 18 Aug 2020 12:01:43 +0200 Subject: [PATCH 33/36] BTreeMap: check some invariants, avoid recursion in depth first search --- library/alloc/src/collections/btree/map.rs | 35 +-- .../alloc/src/collections/btree/map/tests.rs | 232 +++++++++++++++++- .../alloc/src/collections/btree/navigate.rs | 76 ++++++ 3 files changed, 302 insertions(+), 41 deletions(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index c6b55840db993..f8729c33c6713 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -1236,10 +1236,10 @@ impl BTreeMap { right_root.fix_left_border(); if left_root.height() < right_root.height() { - self.recalc_length(); + self.length = left_root.node_as_ref().calc_length(); right.length = total_num - self.len(); } else { - right.recalc_length(); + right.length = right_root.node_as_ref().calc_length(); self.length = total_num - right.len(); } @@ -1283,42 +1283,13 @@ impl BTreeMap { { DrainFilter { pred, inner: self.drain_filter_inner() } } + pub(super) fn drain_filter_inner(&mut self) -> DrainFilterInner<'_, K, V> { let root_node = self.root.as_mut().map(|r| r.node_as_mut()); let front = root_node.map(|rn| rn.first_leaf_edge()); DrainFilterInner { length: &mut self.length, cur_leaf_edge: front } } - /// Calculates the number of elements if it is incorrect. - fn recalc_length(&mut self) { - fn dfs<'a, K, V>(node: NodeRef, K, V, marker::LeafOrInternal>) -> usize - where - K: 'a, - V: 'a, - { - let mut res = node.len(); - - if let Internal(node) = node.force() { - let mut edge = node.first_edge(); - loop { - res += dfs(edge.reborrow().descend()); - match edge.right_kv() { - Ok(right_kv) => { - edge = right_kv.right_edge(); - } - Err(_) => { - break; - } - } - } - } - - res - } - - self.length = dfs(self.root.as_ref().unwrap().node_as_ref()); - } - /// Creates a consuming iterator visiting all the keys, in sorted order. /// The map cannot be used after calling this. /// The iterator element type is `K`. diff --git a/library/alloc/src/collections/btree/map/tests.rs b/library/alloc/src/collections/btree/map/tests.rs index 910e7043092a5..eb8d86b9693fd 100644 --- a/library/alloc/src/collections/btree/map/tests.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -1,4 +1,6 @@ use crate::boxed::Box; +use crate::collections::btree::navigate::Position; +use crate::collections::btree::node; use crate::collections::btree_map::Entry::{Occupied, Vacant}; use crate::collections::BTreeMap; use crate::fmt::Debug; @@ -16,19 +18,19 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use super::super::DeterministicRng; -// Value of node::CAPACITY, thus capacity of a tree with a single level, +// Capacity of a tree with a single level, // i.e. a tree who's root is a leaf node at height 0. -const NODE_CAPACITY: usize = 11; +const NODE_CAPACITY: usize = node::CAPACITY; -// Minimum number of elements to insert in order to guarantee a tree with 2 levels, +// Minimum number of elements to insert, to guarantee a tree with 2 levels, // i.e. a tree who's root is an internal node at height 1, with edges to leaf nodes. // It's not the minimum size: removing an element from such a tree does not always reduce height. const MIN_INSERTS_HEIGHT_1: usize = NODE_CAPACITY + 1; -// Minimum number of elements to insert in order to guarantee a tree with 3 levels, +// Minimum number of elements to insert in ascending order, to guarantee a tree with 3 levels, // i.e. a tree who's root is an internal node at height 2, with edges to more internal nodes. // It's not the minimum size: removing an element from such a tree does not always reduce height. -const MIN_INSERTS_HEIGHT_2: usize = NODE_CAPACITY + (NODE_CAPACITY + 1) * NODE_CAPACITY + 1; +const MIN_INSERTS_HEIGHT_2: usize = 89; // Gather all references from a mutable iterator and make sure Miri notices if // using them is dangerous. @@ -44,11 +46,141 @@ fn test_all_refs<'a, T: 'a>(dummy: &mut T, iter: impl Iterator } } +struct SeriesChecker { + previous: Option, +} + +impl SeriesChecker { + fn is_ascending(&mut self, next: T) { + if let Some(previous) = self.previous { + assert!(previous < next, "{:?} >= {:?}", previous, next); + } + self.previous = Some(next); + } +} + +impl<'a, K: 'a, V: 'a> BTreeMap { + /// Panics if the map (or the code navigating it) is corrupted. + fn check(&self) + where + K: Copy + Debug + Ord, + { + if let Some(root) = &self.root { + let root_node = root.node_as_ref(); + let mut checker = SeriesChecker { previous: None }; + let mut internal_length = 0; + let mut internal_kv_count = 0; + let mut leaf_length = 0; + root_node.visit_nodes_in_order(|pos| match pos { + Position::Leaf(node) => { + let is_root = root_node.height() == 0; + let min_len = if is_root { 0 } else { node::MIN_LEN }; + assert!(node.len() >= min_len, "{} < {}", node.len(), min_len); + + for &key in node.keys() { + checker.is_ascending(key); + } + leaf_length += node.len(); + } + Position::Internal(node) => { + let is_root = root_node.height() == node.height(); + let min_len = if is_root { 1 } else { node::MIN_LEN }; + assert!(node.len() >= min_len, "{} < {}", node.len(), min_len); + + internal_length += node.len(); + } + Position::InternalKV(kv) => { + let key = *kv.into_kv().0; + checker.is_ascending(key); + + internal_kv_count += 1; + } + }); + assert_eq!(internal_length, internal_kv_count); + assert_eq!(root_node.calc_length(), internal_length + leaf_length); + assert_eq!(self.length, internal_length + leaf_length); + } else { + assert_eq!(self.length, 0); + } + } + + /// Returns the height of the root, if any. + fn height(&self) -> Option { + self.root.as_ref().map(node::Root::height) + } + + fn dump_keys(&self) -> String + where + K: Debug, + { + if let Some(root) = self.root.as_ref() { + let mut result = String::new(); + let root_node = root.node_as_ref(); + root_node.visit_nodes_in_order(|pos| match pos { + Position::Leaf(leaf) => { + let depth = root_node.height(); + let indent = " ".repeat(depth); + result += &format!("\n{}{:?}", indent, leaf.keys()) + } + Position::Internal(_) => {} + Position::InternalKV(kv) => { + let depth = root_node.height() - kv.into_node().height(); + let indent = " ".repeat(depth); + result += &format!("\n{}{:?}", indent, kv.into_kv().0); + } + }); + result + } else { + String::from("not yet allocated") + } + } +} + +// Test our value of MIN_INSERTS_HEIGHT_2. It may change according to the +// implementation of insertion, but it's best to be aware of when it does. +#[test] +fn test_levels() { + let mut map = BTreeMap::new(); + map.check(); + assert_eq!(map.height(), None); + assert_eq!(map.len(), 0); + + map.insert(0, ()); + while map.height() == Some(0) { + let last_key = *map.last_key_value().unwrap().0; + map.insert(last_key + 1, ()); + } + map.check(); + // Structure: + // - 1 element in internal root node with 2 children + // - 6 elements in left leaf child + // - 5 elements in right leaf child + assert_eq!(map.height(), Some(1)); + assert_eq!(map.len(), MIN_INSERTS_HEIGHT_1, "{}", map.dump_keys()); + + while map.height() == Some(1) { + let last_key = *map.last_key_value().unwrap().0; + map.insert(last_key + 1, ()); + } + println!("{}", map.dump_keys()); + map.check(); + // Structure: + // - 1 element in internal root node with 2 children + // - 6 elements in left internal child with 7 grandchildren + // - 42 elements in left child's 7 grandchildren with 6 elements each + // - 5 elements in right internal child with 6 grandchildren + // - 30 elements in right child's 5 first grandchildren with 6 elements each + // - 5 elements in right child's last grandchild + assert_eq!(map.height(), Some(2)); + assert_eq!(map.len(), MIN_INSERTS_HEIGHT_2, "{}", map.dump_keys()); +} + #[test] fn test_basic_large() { let mut map = BTreeMap::new(); // Miri is too slow let size = if cfg!(miri) { MIN_INSERTS_HEIGHT_2 } else { 10000 }; + let size = size + (size % 2); // round up to even number assert_eq!(map.len(), 0); for i in 0..size { @@ -93,6 +225,7 @@ fn test_basic_large() { assert_eq!(map.remove(&(2 * i + 1)), Some(i * 200 + 100)); assert_eq!(map.len(), size / 2 - i - 1); } + map.check(); } #[test] @@ -112,7 +245,10 @@ fn test_basic_small() { assert_eq!(map.range(1..).next(), None); assert_eq!(map.range(1..=1).next(), None); assert_eq!(map.range(1..2).next(), None); + assert_eq!(map.height(), None); assert_eq!(map.insert(1, 1), None); + assert_eq!(map.height(), Some(0)); + map.check(); // 1 key-value pair: assert_eq!(map.len(), 1); @@ -131,6 +267,8 @@ fn test_basic_small() { assert_eq!(map.keys().collect::>(), vec![&1]); assert_eq!(map.values().collect::>(), vec![&2]); assert_eq!(map.insert(2, 4), None); + assert_eq!(map.height(), Some(0)); + map.check(); // 2 key-value pairs: assert_eq!(map.len(), 2); @@ -141,6 +279,8 @@ fn test_basic_small() { assert_eq!(map.keys().collect::>(), vec![&1, &2]); assert_eq!(map.values().collect::>(), vec![&2, &4]); assert_eq!(map.remove(&1), Some(2)); + assert_eq!(map.height(), Some(0)); + map.check(); // 1 key-value pair: assert_eq!(map.len(), 1); @@ -153,6 +293,8 @@ fn test_basic_small() { assert_eq!(map.keys().collect::>(), vec![&2]); assert_eq!(map.values().collect::>(), vec![&4]); assert_eq!(map.remove(&2), Some(4)); + assert_eq!(map.height(), Some(0)); + map.check(); // Empty but root is owned (Some(...)): assert_eq!(map.len(), 0); @@ -168,6 +310,8 @@ fn test_basic_small() { assert_eq!(map.range(1..=1).next(), None); assert_eq!(map.range(1..2).next(), None); assert_eq!(map.remove(&1), None); + assert_eq!(map.height(), Some(0)); + map.check(); } #[test] @@ -248,6 +392,7 @@ where assert_eq!(*k, T::try_from(i).unwrap()); assert_eq!(*v, T::try_from(size + i + 1).unwrap()); } + map.check(); } #[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord)] @@ -268,7 +413,7 @@ fn test_iter_mut_mutation() { do_test_iter_mut_mutation::(0); do_test_iter_mut_mutation::(1); do_test_iter_mut_mutation::(MIN_INSERTS_HEIGHT_1); - do_test_iter_mut_mutation::(127); // not enough unique values to test MIN_INSERTS_HEIGHT_2 + do_test_iter_mut_mutation::(MIN_INSERTS_HEIGHT_2); do_test_iter_mut_mutation::(1); do_test_iter_mut_mutation::(MIN_INSERTS_HEIGHT_1); do_test_iter_mut_mutation::(MIN_INSERTS_HEIGHT_2); @@ -291,6 +436,7 @@ fn test_iter_mut_mutation() { fn test_values_mut() { let mut a: BTreeMap<_, _> = (0..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)).collect(); test_all_refs(&mut 13, a.values_mut()); + a.check(); } #[test] @@ -305,6 +451,7 @@ fn test_values_mut_mutation() { let values: Vec = a.values().cloned().collect(); assert_eq!(values, [String::from("hello!"), String::from("goodbye!")]); + a.check(); } #[test] @@ -320,6 +467,7 @@ fn test_iter_entering_root_twice() { *back.1 = 42; assert_eq!(front, (&0, &mut 24)); assert_eq!(back, (&1, &mut 42)); + map.check(); } #[test] @@ -335,6 +483,7 @@ fn test_iter_descending_to_same_node_twice() { assert_eq!(front, (&0, &mut 0)); // Perform mutable access. *front.1 = 42; + map.check(); } #[test] @@ -399,6 +548,7 @@ fn test_iter_min_max() { assert_eq!(a.values().max(), Some(&42)); assert_eq!(a.values_mut().min(), Some(&mut 24)); assert_eq!(a.values_mut().max(), Some(&mut 42)); + a.check(); } fn range_keys(map: &BTreeMap, range: impl RangeBounds) -> Vec { @@ -685,6 +835,7 @@ fn test_range_mut() { assert_eq!(pairs.next(), None); } } + map.check(); } mod test_drain_filter { @@ -695,6 +846,7 @@ mod test_drain_filter { let mut map: BTreeMap = BTreeMap::new(); map.drain_filter(|_, _| unreachable!("there's nothing to decide on")); assert!(map.is_empty()); + map.check(); } #[test] @@ -702,6 +854,7 @@ mod test_drain_filter { let pairs = (0..3).map(|i| (i, i)); let mut map: BTreeMap<_, _> = pairs.collect(); assert!(map.drain_filter(|_, _| false).eq(std::iter::empty())); + map.check(); } #[test] @@ -709,6 +862,7 @@ mod test_drain_filter { let pairs = (0..3).map(|i| (i, i)); let mut map: BTreeMap<_, _> = pairs.clone().collect(); assert!(map.drain_filter(|_, _| true).eq(pairs)); + map.check(); } #[test] @@ -724,6 +878,7 @@ mod test_drain_filter { ); assert!(map.keys().copied().eq(0..3)); assert!(map.values().copied().eq(6..9)); + map.check(); } #[test] @@ -738,6 +893,7 @@ mod test_drain_filter { .eq((0..3).map(|i| (i, i + 6))) ); assert!(map.is_empty()); + map.check(); } #[test] @@ -746,6 +902,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.collect(); map.drain_filter(|_, _| false); assert!(map.keys().copied().eq(0..3)); + map.check(); } #[test] @@ -755,6 +912,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.clone().collect(); map.drain_filter(|i, _| *i == doomed); assert_eq!(map.len(), 2); + map.check(); } } @@ -765,6 +923,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.clone().collect(); map.drain_filter(|i, _| *i != sacred); assert!(map.keys().copied().eq(sacred..=sacred)); + map.check(); } } @@ -774,6 +933,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.collect(); map.drain_filter(|_, _| true); assert!(map.is_empty()); + map.check(); } #[test] @@ -782,6 +942,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.collect(); map.drain_filter(|_, _| false); assert!(map.keys().copied().eq(0..NODE_CAPACITY)); + map.check(); } #[test] @@ -791,6 +952,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.clone().collect(); map.drain_filter(|i, _| *i == doomed); assert_eq!(map.len(), NODE_CAPACITY - 1); + map.check(); } } @@ -801,6 +963,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.clone().collect(); map.drain_filter(|i, _| *i != sacred); assert!(map.keys().copied().eq(sacred..=sacred)); + map.check(); } } @@ -810,6 +973,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.collect(); map.drain_filter(|_, _| true); assert!(map.is_empty()); + map.check(); } #[test] @@ -817,6 +981,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = (0..16).map(|i| (i, i)).collect(); assert_eq!(map.drain_filter(|i, _| *i % 2 == 0).count(), 8); assert_eq!(map.len(), 8); + map.check(); } #[test] @@ -825,6 +990,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.collect(); map.drain_filter(|_, _| true); assert!(map.is_empty()); + map.check(); } #[test] @@ -834,6 +1000,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.clone().collect(); map.drain_filter(|i, _| *i == doomed); assert_eq!(map.len(), MIN_INSERTS_HEIGHT_1 - 1); + map.check(); } } @@ -844,10 +1011,10 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.clone().collect(); map.drain_filter(|i, _| *i != sacred); assert!(map.keys().copied().eq(sacred..=sacred)); + map.check(); } } - #[cfg(not(miri))] // Miri is too slow #[test] fn height_2_removing_one() { let pairs = (0..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)); @@ -855,10 +1022,10 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.clone().collect(); map.drain_filter(|i, _| *i == doomed); assert_eq!(map.len(), MIN_INSERTS_HEIGHT_2 - 1); + map.check(); } } - #[cfg(not(miri))] // Miri is too slow #[test] fn height_2_keeping_one() { let pairs = (0..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)); @@ -866,6 +1033,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.clone().collect(); map.drain_filter(|i, _| *i != sacred); assert!(map.keys().copied().eq(sacred..=sacred)); + map.check(); } } @@ -875,6 +1043,7 @@ mod test_drain_filter { let mut map: BTreeMap<_, _> = pairs.collect(); map.drain_filter(|_, _| true); assert!(map.is_empty()); + map.check(); } #[test] @@ -937,6 +1106,7 @@ mod test_drain_filter { assert_eq!(map.len(), 2); assert_eq!(map.first_entry().unwrap().key(), &4); assert_eq!(map.last_entry().unwrap().key(), &8); + map.check(); } // Same as above, but attempt to use the iterator again after the panic in the predicate @@ -975,6 +1145,7 @@ mod test_drain_filter { assert_eq!(map.len(), 2); assert_eq!(map.first_entry().unwrap().key(), &4); assert_eq!(map.last_entry().unwrap().key(), &8); + map.check(); } } @@ -1033,6 +1204,7 @@ fn test_entry() { } assert_eq!(map.get(&2).unwrap(), &200); assert_eq!(map.len(), 6); + map.check(); // Existing key (take) match map.entry(3) { @@ -1043,6 +1215,7 @@ fn test_entry() { } assert_eq!(map.get(&3), None); assert_eq!(map.len(), 5); + map.check(); // Inexistent key (insert) match map.entry(10) { @@ -1053,6 +1226,7 @@ fn test_entry() { } assert_eq!(map.get(&10).unwrap(), &1000); assert_eq!(map.len(), 6); + map.check(); } #[test] @@ -1069,6 +1243,7 @@ fn test_extend_ref() { assert_eq!(a[&1], "one"); assert_eq!(a[&2], "two"); assert_eq!(a[&3], "three"); + a.check(); } #[test] @@ -1092,6 +1267,7 @@ fn test_zst() { assert_eq!(m.len(), 1); assert_eq!(m.iter().count(), 1); + m.check(); } // This test's only purpose is to ensure that zero-sized keys with nonsensical orderings @@ -1101,6 +1277,7 @@ fn test_zst() { fn test_bad_zst() { use std::cmp::Ordering; + #[derive(Clone, Copy, Debug)] struct Bad; impl PartialEq for Bad { @@ -1128,6 +1305,7 @@ fn test_bad_zst() { for _ in 0..100 { m.insert(Bad, Bad); } + m.check(); } #[test] @@ -1139,18 +1317,21 @@ fn test_clone() { for i in 0..size { assert_eq!(map.insert(i, 10 * i), None); assert_eq!(map.len(), i + 1); + map.check(); assert_eq!(map, map.clone()); } for i in 0..size { assert_eq!(map.insert(i, 100 * i), Some(10 * i)); assert_eq!(map.len(), size); + map.check(); assert_eq!(map, map.clone()); } for i in 0..size / 2 { assert_eq!(map.remove(&(i * 2)), Some(i * 200)); assert_eq!(map.len(), size - i - 1); + map.check(); assert_eq!(map, map.clone()); } @@ -1158,16 +1339,18 @@ fn test_clone() { assert_eq!(map.remove(&(2 * i)), None); assert_eq!(map.remove(&(2 * i + 1)), Some(i * 200 + 100)); assert_eq!(map.len(), size / 2 - i - 1); + map.check(); assert_eq!(map, map.clone()); } - // Test a tree with 2 chock-full levels and a tree with 3 levels. + // Test a tree with 2 semi-full levels and a tree with 3 levels. map = (1..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)).collect(); assert_eq!(map.len(), MIN_INSERTS_HEIGHT_2 - 1); assert_eq!(map, map.clone()); map.insert(0, 0); assert_eq!(map.len(), MIN_INSERTS_HEIGHT_2); assert_eq!(map, map.clone()); + map.check(); } #[test] @@ -1188,8 +1371,10 @@ fn test_clone_from() { map2.insert(100 * j + 1, 2 * j + 1); } map2.clone_from(&map1); // same length + map2.check(); assert_eq!(map2, map1); map1.insert(i, 10 * i); + map1.check(); } } @@ -1246,6 +1431,7 @@ fn test_occupied_entry_key() { } assert_eq!(a.len(), 1); assert_eq!(a[key], value); + a.check(); } #[test] @@ -1264,6 +1450,7 @@ fn test_vacant_entry_key() { } assert_eq!(a.len(), 1); assert_eq!(a[key], value); + a.check(); } #[test] @@ -1288,6 +1475,21 @@ fn test_first_last_entry() { assert_eq!(v2, 24); assert_eq!(a.first_entry().unwrap().key(), &1); assert_eq!(a.last_entry().unwrap().key(), &1); + a.check(); +} + +#[test] +fn test_insert_into_full_left() { + let mut map: BTreeMap<_, _> = (0..NODE_CAPACITY).map(|i| (i * 2, ())).collect(); + assert!(map.insert(NODE_CAPACITY, ()).is_none()); + map.check(); +} + +#[test] +fn test_insert_into_full_right() { + let mut map: BTreeMap<_, _> = (0..NODE_CAPACITY).map(|i| (i * 2, ())).collect(); + assert!(map.insert(NODE_CAPACITY + 2, ()).is_none()); + map.check(); } macro_rules! create_append_test { @@ -1317,8 +1519,10 @@ macro_rules! create_append_test { } } + a.check(); assert_eq!(a.remove(&($len - 1)), Some(2 * ($len - 1))); assert_eq!(a.insert($len - 1, 20), None); + a.check(); } }; } @@ -1355,6 +1559,8 @@ fn test_split_off_empty_right() { let mut map = BTreeMap::from_iter(data.clone()); let right = map.split_off(&(data.iter().max().unwrap().0 + 1)); + map.check(); + right.check(); data.sort(); assert!(map.into_iter().eq(data)); @@ -1367,6 +1573,8 @@ fn test_split_off_empty_left() { let mut map = BTreeMap::from_iter(data.clone()); let right = map.split_off(&data.iter().min().unwrap().0); + map.check(); + right.check(); data.sort(); assert!(map.into_iter().eq(None)); @@ -1380,6 +1588,8 @@ fn test_split_off_tiny_left_height_2() { let pairs = (0..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)); let mut left: BTreeMap<_, _> = pairs.clone().collect(); let right = left.split_off(&1); + left.check(); + right.check(); assert_eq!(left.len(), 1); assert_eq!(right.len(), MIN_INSERTS_HEIGHT_2 - 1); assert_eq!(*left.first_key_value().unwrap().0, 0); @@ -1395,6 +1605,8 @@ fn test_split_off_tiny_right_height_2() { let mut left: BTreeMap<_, _> = pairs.clone().collect(); assert_eq!(*left.last_key_value().unwrap().0, last); let right = left.split_off(&last); + left.check(); + right.check(); assert_eq!(left.len(), MIN_INSERTS_HEIGHT_2 - 1); assert_eq!(right.len(), 1); assert_eq!(*left.last_key_value().unwrap().0, last - 1); @@ -1411,6 +1623,8 @@ fn test_split_off_large_random_sorted() { let mut map = BTreeMap::from_iter(data.clone()); let key = data[data.len() / 2].0; let right = map.split_off(&key); + map.check(); + right.check(); assert!(map.into_iter().eq(data.clone().into_iter().filter(|x| x.0 < key))); assert!(right.into_iter().eq(data.into_iter().filter(|x| x.0 >= key))); diff --git a/library/alloc/src/collections/btree/navigate.rs b/library/alloc/src/collections/btree/navigate.rs index 33b1ee003ed3a..b7b66ac7ceccd 100644 --- a/library/alloc/src/collections/btree/navigate.rs +++ b/library/alloc/src/collections/btree/navigate.rs @@ -49,6 +49,29 @@ impl Handle, marker::E } } +impl Handle, marker::Edge> { + /// Given an internal edge handle, returns [`Result::Ok`] with a handle to the neighboring KV + /// on the right side, which is either in the same internal node or in an ancestor node. + /// If the internal edge is the last one in the tree, returns [`Result::Err`] with the root node. + pub fn next_kv( + self, + ) -> Result< + Handle, marker::KV>, + NodeRef, + > { + let mut edge = self; + loop { + edge = match edge.right_kv() { + Ok(internal_kv) => return Ok(internal_kv), + Err(last_edge) => match last_edge.into_node().ascend() { + Ok(parent_edge) => parent_edge, + Err(root) => return Err(root), + }, + } + } + } +} + macro_rules! def_next_kv_uncheched_dealloc { { unsafe fn $name:ident : $adjacent_kv:ident } => { /// Given a leaf edge handle into an owned tree, returns a handle to the next KV, @@ -232,6 +255,59 @@ impl NodeRef { } } +pub enum Position { + Leaf(NodeRef), + Internal(NodeRef), + InternalKV(Handle, marker::KV>), +} + +impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> { + /// Visits leaf nodes and internal KVs in order of ascending keys, and also + /// visits internal nodes as a whole in a depth first order, meaning that + /// internal nodes precede their individual KVs and their child nodes. + pub fn visit_nodes_in_order(self, mut visit: F) + where + F: FnMut(Position, K, V>), + { + match self.force() { + Leaf(leaf) => visit(Position::Leaf(leaf)), + Internal(internal) => { + visit(Position::Internal(internal)); + let mut edge = internal.first_edge(); + loop { + edge = match edge.descend().force() { + Leaf(leaf) => { + visit(Position::Leaf(leaf)); + match edge.next_kv() { + Ok(kv) => { + visit(Position::InternalKV(kv)); + kv.right_edge() + } + Err(_) => return, + } + } + Internal(internal) => { + visit(Position::Internal(internal)); + internal.first_edge() + } + } + } + } + } + } + + /// Calculates the number of elements in a (sub)tree. + pub fn calc_length(self) -> usize { + let mut result = 0; + self.visit_nodes_in_order(|pos| match pos { + Position::Leaf(node) => result += node.len(), + Position::Internal(node) => result += node.len(), + Position::InternalKV(_) => (), + }); + result + } +} + impl Handle, marker::KV> { /// Returns the leaf edge closest to a KV for forward navigation. pub fn next_leaf_edge(self) -> Handle, marker::Edge> { From d9d84dca8eaf463a0d878bbd4916665500a071d1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 18 Aug 2020 13:31:23 +0200 Subject: [PATCH 34/36] Add doc examples count for --show-coverage --- src/librustdoc/html/render/cache.rs | 6 +- .../passes/calculate_doc_coverage.rs | 80 ++++++++++++++++--- src/librustdoc/passes/doc_test_lints.rs | 28 ++++--- 3 files changed, 88 insertions(+), 26 deletions(-) diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index 5a9e9dda6771e..ccc07645620a9 100644 --- a/src/librustdoc/html/render/cache.rs +++ b/src/librustdoc/html/render/cache.rs @@ -200,10 +200,12 @@ fn get_index_type_name(clean_type: &clean::Type, accept_generic: bool) -> Option match *clean_type { clean::ResolvedPath { ref path, .. } => { let segments = &path.segments; - let path_segment = segments.iter().last().unwrap_or_else(|| panic!( + let path_segment = segments.iter().last().unwrap_or_else(|| { + panic!( "get_index_type_name(clean_type: {:?}, accept_generic: {:?}) had length zero path", clean_type, accept_generic - )); + ) + }); Some(path_segment.name.clone()) } clean::Generic(ref s) if accept_generic => Some(s.clone()), diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index b722cfc8f7521..0a836f46c0eb8 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -2,8 +2,9 @@ use crate::clean; use crate::config::OutputFormat; use crate::core::DocContext; use crate::fold::{self, DocFolder}; +use crate::html::markdown::{find_testable_code, ErrorCodes}; +use crate::passes::doc_test_lints::Tests; use crate::passes::Pass; - use rustc_span::symbol::sym; use rustc_span::FileName; use serde::Serialize; @@ -30,15 +31,19 @@ fn calculate_doc_coverage(krate: clean::Crate, ctx: &DocContext<'_>) -> clean::C struct ItemCount { total: u64, with_docs: u64, + with_examples: u64, } impl ItemCount { - fn count_item(&mut self, has_docs: bool) { + fn count_item(&mut self, has_docs: bool, has_doc_example: bool) { self.total += 1; if has_docs { self.with_docs += 1; } + if has_doc_example { + self.with_examples += 1; + } } fn percentage(&self) -> Option { @@ -48,13 +53,25 @@ impl ItemCount { None } } + + fn examples_percentage(&self) -> Option { + if self.total > 0 { + Some((self.with_examples as f64 * 100.0) / self.total as f64) + } else { + None + } + } } impl ops::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 } + ItemCount { + total: self.total - rhs.total, + with_docs: self.with_docs - rhs.with_docs, + with_examples: self.with_examples - rhs.with_examples, + } } } @@ -62,6 +79,7 @@ impl ops::AddAssign for ItemCount { fn add_assign(&mut self, rhs: Self) { self.total += rhs.total; self.with_docs += rhs.with_docs; + self.with_examples += rhs.with_examples; } } @@ -103,33 +121,55 @@ impl CoverageCalculator { let mut total = ItemCount::default(); fn print_table_line() { - println!("+-{0:->35}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); + println!("+-{0:->35}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+-{0:->10}-+", ""); } - fn print_table_record(name: &str, count: ItemCount, percentage: f64) { + fn print_table_record( + name: &str, + count: ItemCount, + percentage: f64, + examples_percentage: f64, + ) { println!( - "| {:<35} | {:>10} | {:>10} | {:>9.1}% |", - name, count.with_docs, count.total, percentage + "| {:<35} | {:>10} | {:>10} | {:>9.1}% | {:>10} | {:>9.1}% |", + name, + count.with_docs, + count.total, + percentage, + count.with_examples, + examples_percentage, ); } print_table_line(); println!( - "| {:<35} | {:>10} | {:>10} | {:>10} |", - "File", "Documented", "Total", "Percentage" + "| {:<35} | {:>10} | {:>10} | {:>10} | {:>10} | {:>10} |", + "File", "Documented", "Total", "Percentage", "Examples", "Percentage", ); print_table_line(); for (file, &count) in &self.items { - if let Some(percentage) = count.percentage() { - print_table_record(&limit_filename_len(file.to_string()), count, percentage); + if let (Some(percentage), Some(examples_percentage)) = + (count.percentage(), count.examples_percentage()) + { + print_table_record( + &limit_filename_len(file.to_string()), + count, + percentage, + examples_percentage, + ); total += count; } } print_table_line(); - print_table_record("Total", total, total.percentage().unwrap_or(0.0)); + print_table_record( + "Total", + total, + total.percentage().unwrap_or(0.0), + total.examples_percentage().unwrap_or(0.0), + ); print_table_line(); } } @@ -137,6 +177,17 @@ impl CoverageCalculator { impl fold::DocFolder for CoverageCalculator { fn fold_item(&mut self, i: clean::Item) -> Option { let has_docs = !i.attrs.doc_strings.is_empty(); + let mut tests = Tests { found_tests: 0 }; + + find_testable_code( + &i.attrs.doc_strings.iter().map(|d| d.as_str()).collect::>().join("\n"), + &mut tests, + ErrorCodes::No, + false, + None, + ); + + let has_doc_example = tests.found_tests != 0; match i.inner { _ if !i.def_id.is_local() => { @@ -187,7 +238,10 @@ impl fold::DocFolder for CoverageCalculator { } _ => { debug!("counting {:?} {:?} in {}", i.type_(), i.name, i.source.filename); - self.items.entry(i.source.filename.clone()).or_default().count_item(has_docs); + self.items + .entry(i.source.filename.clone()) + .or_default() + .count_item(has_docs, has_doc_example); } } diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index aced7d55281ad..1fdc4ee247adf 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -43,6 +43,22 @@ impl<'a, 'tcx> DocFolder for PrivateItemDocTestLinter<'a, 'tcx> { } } +pub(crate) struct Tests { + pub(crate) found_tests: usize, +} + +impl Tests { + pub(crate) fn new() -> Tests { + Tests { found_tests: 0 } + } +} + +impl crate::test::Tester for Tests { + fn add_test(&mut self, _: String, _: LangString, _: usize) { + self.found_tests += 1; + } +} + pub fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { let hir_id = match cx.as_local_hir_id(item.def_id) { Some(hir_id) => hir_id, @@ -52,17 +68,7 @@ pub fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { } }; - struct Tests { - found_tests: usize, - } - - impl crate::test::Tester for Tests { - fn add_test(&mut self, _: String, _: LangString, _: usize) { - self.found_tests += 1; - } - } - - let mut tests = Tests { found_tests: 0 }; + let mut tests = Tests::new(); find_testable_code(&dox, &mut tests, ErrorCodes::No, false, None); From f957bae74eb4e8847b0c0af5cd2c01812b110fc0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 18 Aug 2020 13:31:40 +0200 Subject: [PATCH 35/36] Update rustdoc-ui tests --- src/test/rustdoc-ui/coverage/basic.stdout | 14 +++++----- src/test/rustdoc-ui/coverage/doc-examples.rs | 27 +++++++++++++++++++ .../rustdoc-ui/coverage/doc-examples.stdout | 7 +++++ src/test/rustdoc-ui/coverage/empty.stdout | 14 +++++----- src/test/rustdoc-ui/coverage/enums.stdout | 14 +++++----- src/test/rustdoc-ui/coverage/exotic.stdout | 16 +++++------ src/test/rustdoc-ui/coverage/json.stdout | 2 +- src/test/rustdoc-ui/coverage/private.stdout | 14 +++++----- .../rustdoc-ui/coverage/statics-consts.stdout | 14 +++++----- src/test/rustdoc-ui/coverage/traits.stdout | 14 +++++----- 10 files changed, 85 insertions(+), 51 deletions(-) create mode 100644 src/test/rustdoc-ui/coverage/doc-examples.rs create mode 100644 src/test/rustdoc-ui/coverage/doc-examples.stdout diff --git a/src/test/rustdoc-ui/coverage/basic.stdout b/src/test/rustdoc-ui/coverage/basic.stdout index 3e91660631626..7e795acc575bc 100644 --- a/src/test/rustdoc-ui/coverage/basic.stdout +++ b/src/test/rustdoc-ui/coverage/basic.stdout @@ -1,7 +1,7 @@ -+-------------------------------------+------------+------------+------------+ -| File | Documented | Total | Percentage | -+-------------------------------------+------------+------------+------------+ -| ...est/rustdoc-ui/coverage/basic.rs | 7 | 14 | 50.0% | -+-------------------------------------+------------+------------+------------+ -| Total | 7 | 14 | 50.0% | -+-------------------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+------------+------------+ +| File | Documented | Total | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/basic.rs | 7 | 14 | 50.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ +| Total | 7 | 14 | 50.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/doc-examples.rs b/src/test/rustdoc-ui/coverage/doc-examples.rs new file mode 100644 index 0000000000000..cd718f8a34d11 --- /dev/null +++ b/src/test/rustdoc-ui/coverage/doc-examples.rs @@ -0,0 +1,27 @@ +// compile-flags:-Z unstable-options --show-coverage +// check-pass + +//! This test ensure that only rust code examples are counted. + +/// Doc +/// +/// ``` +/// let x = 2; +/// ``` +pub struct Foo; + +/// Doc +/// +/// ```text +/// yolo +/// ``` +pub trait Bar {} + +/// Doc +/// +/// ```ignore (just for the sake of this test) +/// let x = 2; +/// ``` +pub fn foo(a: Foo, b: u32, c: T, d: D) -> u32 { + 0 +} diff --git a/src/test/rustdoc-ui/coverage/doc-examples.stdout b/src/test/rustdoc-ui/coverage/doc-examples.stdout new file mode 100644 index 0000000000000..f25cf79a3f35d --- /dev/null +++ b/src/test/rustdoc-ui/coverage/doc-examples.stdout @@ -0,0 +1,7 @@ ++-------------------------------------+------------+------------+------------+------------+------------+ +| File | Documented | Total | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+------------+ +| ...tdoc-ui/coverage/doc-examples.rs | 4 | 4 | 100.0% | 2 | 50.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ +| Total | 4 | 4 | 100.0% | 2 | 50.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/empty.stdout b/src/test/rustdoc-ui/coverage/empty.stdout index 11b514fbfeaef..2a6a2231e5b57 100644 --- a/src/test/rustdoc-ui/coverage/empty.stdout +++ b/src/test/rustdoc-ui/coverage/empty.stdout @@ -1,7 +1,7 @@ -+-------------------------------------+------------+------------+------------+ -| File | Documented | Total | Percentage | -+-------------------------------------+------------+------------+------------+ -| ...est/rustdoc-ui/coverage/empty.rs | 0 | 1 | 0.0% | -+-------------------------------------+------------+------------+------------+ -| Total | 0 | 1 | 0.0% | -+-------------------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+------------+------------+ +| File | Documented | Total | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/empty.rs | 0 | 1 | 0.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ +| Total | 0 | 1 | 0.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/enums.stdout b/src/test/rustdoc-ui/coverage/enums.stdout index 87e2ad9f20df6..dd86f61f8d501 100644 --- a/src/test/rustdoc-ui/coverage/enums.stdout +++ b/src/test/rustdoc-ui/coverage/enums.stdout @@ -1,7 +1,7 @@ -+-------------------------------------+------------+------------+------------+ -| File | Documented | Total | Percentage | -+-------------------------------------+------------+------------+------------+ -| ...est/rustdoc-ui/coverage/enums.rs | 6 | 8 | 75.0% | -+-------------------------------------+------------+------------+------------+ -| Total | 6 | 8 | 75.0% | -+-------------------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+------------+------------+ +| File | Documented | Total | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+------------+ +| ...est/rustdoc-ui/coverage/enums.rs | 6 | 8 | 75.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ +| Total | 6 | 8 | 75.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/exotic.stdout b/src/test/rustdoc-ui/coverage/exotic.stdout index 2bacfcfcecabe..f920a3abd36bb 100644 --- a/src/test/rustdoc-ui/coverage/exotic.stdout +++ b/src/test/rustdoc-ui/coverage/exotic.stdout @@ -1,8 +1,8 @@ -+-------------------------------------+------------+------------+------------+ -| File | Documented | Total | Percentage | -+-------------------------------------+------------+------------+------------+ -| ...st/rustdoc-ui/coverage/exotic.rs | 1 | 1 | 100.0% | -| | 2 | 2 | 100.0% | -+-------------------------------------+------------+------------+------------+ -| Total | 3 | 3 | 100.0% | -+-------------------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+------------+------------+ +| File | Documented | Total | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+------------+ +| ...st/rustdoc-ui/coverage/exotic.rs | 1 | 1 | 100.0% | 0 | 0.0% | +| | 2 | 2 | 100.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ +| Total | 3 | 3 | 100.0% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/json.stdout b/src/test/rustdoc-ui/coverage/json.stdout index 63b22a7d94b00..7b5b083e1584c 100644 --- a/src/test/rustdoc-ui/coverage/json.stdout +++ b/src/test/rustdoc-ui/coverage/json.stdout @@ -1 +1 @@ -{"$DIR/json.rs":{"total":13,"with_docs":7}} +{"$DIR/json.rs":{"total":13,"with_docs":7,"with_examples":0}} diff --git a/src/test/rustdoc-ui/coverage/private.stdout b/src/test/rustdoc-ui/coverage/private.stdout index 0d4c7c68fd05e..bca3d51da59d0 100644 --- a/src/test/rustdoc-ui/coverage/private.stdout +++ b/src/test/rustdoc-ui/coverage/private.stdout @@ -1,7 +1,7 @@ -+-------------------------------------+------------+------------+------------+ -| File | Documented | Total | Percentage | -+-------------------------------------+------------+------------+------------+ -| ...t/rustdoc-ui/coverage/private.rs | 4 | 7 | 57.1% | -+-------------------------------------+------------+------------+------------+ -| Total | 4 | 7 | 57.1% | -+-------------------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+------------+------------+ +| File | Documented | Total | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+------------+ +| ...t/rustdoc-ui/coverage/private.rs | 4 | 7 | 57.1% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ +| Total | 4 | 7 | 57.1% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/statics-consts.stdout b/src/test/rustdoc-ui/coverage/statics-consts.stdout index 8459f90ae7b31..31b48cc602a76 100644 --- a/src/test/rustdoc-ui/coverage/statics-consts.stdout +++ b/src/test/rustdoc-ui/coverage/statics-consts.stdout @@ -1,7 +1,7 @@ -+-------------------------------------+------------+------------+------------+ -| File | Documented | Total | Percentage | -+-------------------------------------+------------+------------+------------+ -| ...oc-ui/coverage/statics-consts.rs | 6 | 7 | 85.7% | -+-------------------------------------+------------+------------+------------+ -| Total | 6 | 7 | 85.7% | -+-------------------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+------------+------------+ +| File | Documented | Total | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+------------+ +| ...oc-ui/coverage/statics-consts.rs | 6 | 7 | 85.7% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ +| Total | 6 | 7 | 85.7% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ diff --git a/src/test/rustdoc-ui/coverage/traits.stdout b/src/test/rustdoc-ui/coverage/traits.stdout index e347a4da0b978..ac63b65023d0b 100644 --- a/src/test/rustdoc-ui/coverage/traits.stdout +++ b/src/test/rustdoc-ui/coverage/traits.stdout @@ -1,7 +1,7 @@ -+-------------------------------------+------------+------------+------------+ -| File | Documented | Total | Percentage | -+-------------------------------------+------------+------------+------------+ -| ...st/rustdoc-ui/coverage/traits.rs | 6 | 7 | 85.7% | -+-------------------------------------+------------+------------+------------+ -| Total | 6 | 7 | 85.7% | -+-------------------------------------+------------+------------+------------+ ++-------------------------------------+------------+------------+------------+------------+------------+ +| File | Documented | Total | Percentage | Examples | Percentage | ++-------------------------------------+------------+------------+------------+------------+------------+ +| ...st/rustdoc-ui/coverage/traits.rs | 6 | 7 | 85.7% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ +| Total | 6 | 7 | 85.7% | 0 | 0.0% | ++-------------------------------------+------------+------------+------------+------------+------------+ From 522d177f34e683b16d3548446aa3ff7ddd2c531c Mon Sep 17 00:00:00 2001 From: Jan Riemer Date: Tue, 18 Aug 2020 18:40:19 +0200 Subject: [PATCH 36/36] docs: add another `derive` for `Copy`able struct This adds another `derive` for a `Copy`able struct, so that we are consistent with `derive` annotations. --- library/core/src/marker.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index f113f9392301a..9326aaf56847c 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -291,6 +291,7 @@ pub trait StructuralEq { /// /// ``` /// # #[allow(dead_code)] +/// #[derive(Copy, Clone)] /// struct Point { /// x: i32, /// y: i32,