From e720f4237a39dba0a4b741812de59d1654b3b253 Mon Sep 17 00:00:00 2001 From: Ryan Scheel Date: Sat, 1 Aug 2020 22:56:25 -0700 Subject: [PATCH 01/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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 4f9cd749261cf95f48a9473a2b44a5cfcd27e10f Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 17 Aug 2020 19:51:09 -0400 Subject: [PATCH 20/31] Resolve true and false as booleans --- .../passes/collect_intra_doc_links.rs | 25 +++++++++++++------ src/test/rustdoc/intra-doc-link-true-false.rs | 10 ++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) create mode 100644 src/test/rustdoc/intra-doc-link-true-false.rs diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f7fc3579d67a7..8df089b6d81e5 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -222,11 +222,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { disambiguator, None | Some(Disambiguator::Namespace(Namespace::TypeNS)) ) { - if let Some(prim) = is_primitive(path_str, ns) { + if let Some((path, prim)) = is_primitive(path_str, ns) { if extra_fragment.is_some() { return Err(ErrorKind::AnchorFailure(AnchorFailure::Primitive)); } - return Ok((prim, Some(path_str.to_owned()))); + return Ok((prim, Some(path.to_owned()))); } } return Ok((res, extra_fragment.clone())); @@ -239,11 +239,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if value != (ns == ValueNS) { return Err(ErrorKind::ResolutionFailure); } - } else if let Some(prim) = is_primitive(path_str, ns) { + } else if let Some((path, prim)) = is_primitive(path_str, ns) { if extra_fragment.is_some() { return Err(ErrorKind::AnchorFailure(AnchorFailure::Primitive)); } - return Ok((prim, Some(path_str.to_owned()))); + return Ok((prim, Some(path.to_owned()))); } else { // If resolution failed, it may still be a method // because methods are not handled by the resolver @@ -269,7 +269,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) .ok_or(ErrorKind::ResolutionFailure)?; - if let Some(prim) = is_primitive(&path, TypeNS) { + if let Some((path, prim)) = is_primitive(&path, TypeNS) { let did = primitive_impl(cx, &path).ok_or(ErrorKind::ResolutionFailure)?; return cx .tcx @@ -1220,11 +1220,22 @@ const PRIMITIVES: &[(&str, Res)] = &[ ("f64", Res::PrimTy(hir::PrimTy::Float(rustc_ast::ast::FloatTy::F64))), ("str", Res::PrimTy(hir::PrimTy::Str)), ("bool", Res::PrimTy(hir::PrimTy::Bool)), + ("true", Res::PrimTy(hir::PrimTy::Bool)), + ("false", Res::PrimTy(hir::PrimTy::Bool)), ("char", Res::PrimTy(hir::PrimTy::Char)), ]; -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 is_primitive(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> { + if ns == TypeNS { + PRIMITIVES + .iter() + .filter(|x| x.0 == path_str) + .copied() + .map(|x| if x.0 == "true" || x.0 == "false" { ("bool", x.1) } else { x }) + .next() + } else { + None + } } fn primitive_impl(cx: &DocContext<'_>, path_str: &str) -> Option { diff --git a/src/test/rustdoc/intra-doc-link-true-false.rs b/src/test/rustdoc/intra-doc-link-true-false.rs new file mode 100644 index 0000000000000..7b21e93414740 --- /dev/null +++ b/src/test/rustdoc/intra-doc-link-true-false.rs @@ -0,0 +1,10 @@ +#![deny(broken_intra_doc_links)] +#![crate_name = "foo"] + +// ignore-tidy-linelength + +// @has foo/index.html +// @has - '//*[@id="main"]//a[@href="https://doc.rust-lang.org/nightly/std/primitive.bool.html"]' 'true' +// @has - '//*[@id="main"]//a[@href="https://doc.rust-lang.org/nightly/std/primitive.bool.html"]' 'false' + +//! A `bool` is either [`true`] or [`false`]. 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 21/31] 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 22/31] 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 23/31] 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 24/31] 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 25/31] 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 26/31] 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 27/31] 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 28/31] 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, From ff73a409957d704aba735fb29ab4be2d77efe958 Mon Sep 17 00:00:00 2001 From: Tyson Nottingham Date: Mon, 17 Aug 2020 19:15:51 -0700 Subject: [PATCH 29/31] Don't emit "is not a logical operator" error outside of associative expressions Avoid showing this error where it doesn't make sense by not assuming "and" and "or" were intended to mean "&&" and "||" until after we decide to continue parsing input as an associative expression. Note that the decision of whether or not to continue parsing input as an associative expression doesn't actually depend on this assumption. Fixes #75599 --- src/librustc_parse/parser/expr.rs | 2 +- .../issue-54109-and_instead_of_ampersands.rs | 8 -- ...sue-54109-and_instead_of_ampersands.stderr | 82 ++----------------- .../issue-54109-without-witness.fixed | 8 -- .../issue-54109-without-witness.rs | 8 -- .../issue-54109-without-witness.stderr | 80 ++---------------- src/test/ui/issues/issue-75599.rs | 24 ++++++ 7 files changed, 42 insertions(+), 170 deletions(-) create mode 100644 src/test/ui/issues/issue-75599.rs diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index 0ceb588b3afe7..d16347954faa1 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -308,7 +308,7 @@ impl<'a> Parser<'a> { } fn should_continue_as_assoc_expr(&mut self, lhs: &Expr) -> bool { - match (self.expr_is_complete(lhs), self.check_assoc_op().map(|op| op.node)) { + match (self.expr_is_complete(lhs), AssocOp::from_token(&self.token)) { // Semi-statement forms are odd: // See https://github.com/rust-lang/rust/issues/29071 (true, None) => false, diff --git a/src/test/ui/did_you_mean/issue-54109-and_instead_of_ampersands.rs b/src/test/ui/did_you_mean/issue-54109-and_instead_of_ampersands.rs index 467daef63f6a6..44421b077fa26 100644 --- a/src/test/ui/did_you_mean/issue-54109-and_instead_of_ampersands.rs +++ b/src/test/ui/did_you_mean/issue-54109-and_instead_of_ampersands.rs @@ -5,10 +5,8 @@ fn test_and() { let b = false; let _ = a and b; //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator if a and b { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } @@ -20,10 +18,8 @@ fn test_or() { let b = false; let _ = a or b; //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator if a or b { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } @@ -32,7 +28,6 @@ fn test_and_par() { let a = true; let b = false; if (a and b) { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } } @@ -41,7 +36,6 @@ fn test_or_par() { let a = true; let b = false; if (a or b) { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } @@ -50,7 +44,6 @@ fn test_while_and() { let a = true; let b = false; while a and b { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } } @@ -59,7 +52,6 @@ fn test_while_or() { let a = true; let b = false; while a or b { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } diff --git a/src/test/ui/did_you_mean/issue-54109-and_instead_of_ampersands.stderr b/src/test/ui/did_you_mean/issue-54109-and_instead_of_ampersands.stderr index e8731cf238ec4..528c62f501e0d 100644 --- a/src/test/ui/did_you_mean/issue-54109-and_instead_of_ampersands.stderr +++ b/src/test/ui/did_you_mean/issue-54109-and_instead_of_ampersands.stderr @@ -7,23 +7,7 @@ LL | let _ = a and b; = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `and` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:7:15 - | -LL | let _ = a and b; - | ^^^ help: use `&&` to perform logical conjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `and` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:10:10 - | -LL | if a and b { - | ^^^ help: use `&&` to perform logical conjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `and` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:10:10 + --> $DIR/issue-54109-and_instead_of_ampersands.rs:9:10 | LL | if a and b { | ^^^ help: use `&&` to perform logical conjunction @@ -31,7 +15,7 @@ LL | if a and b { = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `or` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:22:15 + --> $DIR/issue-54109-and_instead_of_ampersands.rs:20:15 | LL | let _ = a or b; | ^^ help: use `||` to perform logical disjunction @@ -39,23 +23,7 @@ LL | let _ = a or b; = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `or` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:22:15 - | -LL | let _ = a or b; - | ^^ help: use `||` to perform logical disjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `or` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:25:10 - | -LL | if a or b { - | ^^ help: use `||` to perform logical disjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `or` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:25:10 + --> $DIR/issue-54109-and_instead_of_ampersands.rs:22:10 | LL | if a or b { | ^^ help: use `||` to perform logical disjunction @@ -63,31 +31,15 @@ LL | if a or b { = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `and` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:34:11 + --> $DIR/issue-54109-and_instead_of_ampersands.rs:30:11 | LL | if (a and b) { | ^^^ help: use `&&` to perform logical conjunction | = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators -error: `and` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:34:11 - | -LL | if (a and b) { - | ^^^ help: use `&&` to perform logical conjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `or` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:43:11 - | -LL | if (a or b) { - | ^^ help: use `||` to perform logical disjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - error: `or` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:43:11 + --> $DIR/issue-54109-and_instead_of_ampersands.rs:38:11 | LL | if (a or b) { | ^^ help: use `||` to perform logical disjunction @@ -95,31 +47,15 @@ LL | if (a or b) { = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `and` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:52:13 + --> $DIR/issue-54109-and_instead_of_ampersands.rs:46:13 | LL | while a and b { | ^^^ help: use `&&` to perform logical conjunction | = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators -error: `and` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:52:13 - | -LL | while a and b { - | ^^^ help: use `&&` to perform logical conjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `or` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:61:13 - | -LL | while a or b { - | ^^ help: use `||` to perform logical disjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - error: `or` is not a logical operator - --> $DIR/issue-54109-and_instead_of_ampersands.rs:61:13 + --> $DIR/issue-54109-and_instead_of_ampersands.rs:54:13 | LL | while a or b { | ^^ help: use `||` to perform logical disjunction @@ -127,13 +63,13 @@ LL | while a or b { = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error[E0308]: mismatched types - --> $DIR/issue-54109-and_instead_of_ampersands.rs:15:33 + --> $DIR/issue-54109-and_instead_of_ampersands.rs:13:33 | LL | let _recovery_witness: () = 0; | -- ^ expected `()`, found integer | | | expected due to this -error: aborting due to 17 previous errors +error: aborting due to 9 previous errors For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/did_you_mean/issue-54109-without-witness.fixed b/src/test/ui/did_you_mean/issue-54109-without-witness.fixed index 21471d75c8215..5079a37f4da7f 100644 --- a/src/test/ui/did_you_mean/issue-54109-without-witness.fixed +++ b/src/test/ui/did_you_mean/issue-54109-without-witness.fixed @@ -11,10 +11,8 @@ fn test_and() { let b = false; let _ = a && b; //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator if a && b { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } } @@ -24,10 +22,8 @@ fn test_or() { let b = false; let _ = a || b; //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator if a || b { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } @@ -36,7 +32,6 @@ fn test_and_par() { let a = true; let b = false; if (a && b) { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } } @@ -45,7 +40,6 @@ fn test_or_par() { let a = true; let b = false; if (a || b) { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } @@ -54,7 +48,6 @@ fn test_while_and() { let a = true; let b = false; while a && b { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } } @@ -63,7 +56,6 @@ fn test_while_or() { let a = true; let b = false; while a || b { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } diff --git a/src/test/ui/did_you_mean/issue-54109-without-witness.rs b/src/test/ui/did_you_mean/issue-54109-without-witness.rs index bb9a3a195962e..00660a938d5d6 100644 --- a/src/test/ui/did_you_mean/issue-54109-without-witness.rs +++ b/src/test/ui/did_you_mean/issue-54109-without-witness.rs @@ -11,10 +11,8 @@ fn test_and() { let b = false; let _ = a and b; //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator if a and b { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } } @@ -24,10 +22,8 @@ fn test_or() { let b = false; let _ = a or b; //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator if a or b { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } @@ -36,7 +32,6 @@ fn test_and_par() { let a = true; let b = false; if (a and b) { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } } @@ -45,7 +40,6 @@ fn test_or_par() { let a = true; let b = false; if (a or b) { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } @@ -54,7 +48,6 @@ fn test_while_and() { let a = true; let b = false; while a and b { //~ ERROR `and` is not a logical operator - //~| ERROR `and` is not a logical operator println!("both"); } } @@ -63,7 +56,6 @@ fn test_while_or() { let a = true; let b = false; while a or b { //~ ERROR `or` is not a logical operator - //~| ERROR `or` is not a logical operator println!("both"); } } diff --git a/src/test/ui/did_you_mean/issue-54109-without-witness.stderr b/src/test/ui/did_you_mean/issue-54109-without-witness.stderr index fe48af592db91..0350890c1fde0 100644 --- a/src/test/ui/did_you_mean/issue-54109-without-witness.stderr +++ b/src/test/ui/did_you_mean/issue-54109-without-witness.stderr @@ -7,23 +7,7 @@ LL | let _ = a and b; = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `and` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:13:15 - | -LL | let _ = a and b; - | ^^^ help: use `&&` to perform logical conjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `and` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:16:10 - | -LL | if a and b { - | ^^^ help: use `&&` to perform logical conjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `and` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:16:10 + --> $DIR/issue-54109-without-witness.rs:15:10 | LL | if a and b { | ^^^ help: use `&&` to perform logical conjunction @@ -31,7 +15,7 @@ LL | if a and b { = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `or` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:26:15 + --> $DIR/issue-54109-without-witness.rs:24:15 | LL | let _ = a or b; | ^^ help: use `||` to perform logical disjunction @@ -39,23 +23,7 @@ LL | let _ = a or b; = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `or` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:26:15 - | -LL | let _ = a or b; - | ^^ help: use `||` to perform logical disjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `or` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:29:10 - | -LL | if a or b { - | ^^ help: use `||` to perform logical disjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `or` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:29:10 + --> $DIR/issue-54109-without-witness.rs:26:10 | LL | if a or b { | ^^ help: use `||` to perform logical disjunction @@ -63,31 +31,15 @@ LL | if a or b { = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `and` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:38:11 + --> $DIR/issue-54109-without-witness.rs:34:11 | LL | if (a and b) { | ^^^ help: use `&&` to perform logical conjunction | = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators -error: `and` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:38:11 - | -LL | if (a and b) { - | ^^^ help: use `&&` to perform logical conjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `or` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:47:11 - | -LL | if (a or b) { - | ^^ help: use `||` to perform logical disjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - error: `or` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:47:11 + --> $DIR/issue-54109-without-witness.rs:42:11 | LL | if (a or b) { | ^^ help: use `||` to perform logical disjunction @@ -95,36 +47,20 @@ LL | if (a or b) { = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators error: `and` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:56:13 + --> $DIR/issue-54109-without-witness.rs:50:13 | LL | while a and b { | ^^^ help: use `&&` to perform logical conjunction | = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators -error: `and` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:56:13 - | -LL | while a and b { - | ^^^ help: use `&&` to perform logical conjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - -error: `or` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:65:13 - | -LL | while a or b { - | ^^ help: use `||` to perform logical disjunction - | - = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators - error: `or` is not a logical operator - --> $DIR/issue-54109-without-witness.rs:65:13 + --> $DIR/issue-54109-without-witness.rs:58:13 | LL | while a or b { | ^^ help: use `||` to perform logical disjunction | = note: unlike in e.g., python and PHP, `&&` and `||` are used for logical operators -error: aborting due to 16 previous errors +error: aborting due to 8 previous errors diff --git a/src/test/ui/issues/issue-75599.rs b/src/test/ui/issues/issue-75599.rs new file mode 100644 index 0000000000000..0857676e4ed58 --- /dev/null +++ b/src/test/ui/issues/issue-75599.rs @@ -0,0 +1,24 @@ +// check-pass +#![allow(non_upper_case_globals)] + +const or: usize = 1; +const and: usize = 2; + +mod or { + pub const X: usize = 3; +} + +mod and { + pub const X: usize = 4; +} + +fn main() { + match 0 { + 0 => {} + or => {} + and => {} + or::X => {} + and::X => {} + _ => {} + } +} From 3e3a2c82f3ab29f855911f2ecf57ef15a3a4fe03 Mon Sep 17 00:00:00 2001 From: Prabakaran Kumaresshan <4676330+nixphix@users.noreply.github.com> Date: Tue, 18 Aug 2020 22:11:20 +0530 Subject: [PATCH 30/31] Switch to intra-doc links in /src/sys/unix/ext/*.rs --- library/std/src/sys/unix/ext/fs.rs | 45 +++++++-------------- library/std/src/sys/unix/ext/net.rs | 52 ++++++++++--------------- library/std/src/sys/unix/ext/process.rs | 8 +--- library/std/src/sys/unix/ext/thread.rs | 4 +- 4 files changed, 35 insertions(+), 74 deletions(-) diff --git a/library/std/src/sys/unix/ext/fs.rs b/library/std/src/sys/unix/ext/fs.rs index f174a59b49a6b..b590a0280d138 100644 --- a/library/std/src/sys/unix/ext/fs.rs +++ b/library/std/src/sys/unix/ext/fs.rs @@ -9,9 +9,7 @@ use crate::sys; use crate::sys::platform::fs::MetadataExt as UnixMetadataExt; use crate::sys_common::{AsInner, AsInnerMut, FromInner}; -/// Unix-specific extensions to [`File`]. -/// -/// [`File`]: ../../../../std/fs/struct.File.html +/// Unix-specific extensions to [`fs::File`]. #[stable(feature = "file_offset", since = "1.15.0")] pub trait FileExt { /// Reads a number of bytes starting from a given offset. @@ -55,19 +53,18 @@ pub trait FileExt { /// /// The current file cursor is not affected by this function. /// - /// Similar to [`Read::read_exact`] but uses [`read_at`] instead of `read`. + /// Similar to [`io::Read::read_exact`] but uses [`read_at`] instead of `read`. /// - /// [`Read::read_exact`]: ../../../../std/io/trait.Read.html#method.read_exact - /// [`read_at`]: #tymethod.read_at + /// [`read_at`]: FileExt::read_at /// /// # Errors /// /// If this function encounters an error of the kind - /// [`ErrorKind::Interrupted`] then the error is ignored and the operation + /// [`io::ErrorKind::Interrupted`] then the error is ignored and the operation /// will continue. /// /// If this function encounters an "end of file" before completely filling - /// the buffer, it returns an error of the kind [`ErrorKind::UnexpectedEof`]. + /// the buffer, it returns an error of the kind [`io::ErrorKind::UnexpectedEof`]. /// The contents of `buf` are unspecified in this case. /// /// If any other read error is encountered then this function immediately @@ -77,9 +74,6 @@ pub trait FileExt { /// has read, but it will never read more than would be necessary to /// completely fill the buffer. /// - /// [`ErrorKind::Interrupted`]: ../../../../std/io/enum.ErrorKind.html#variant.Interrupted - /// [`ErrorKind::UnexpectedEof`]: ../../../../std/io/enum.ErrorKind.html#variant.UnexpectedEof - /// /// # Examples /// /// ```no_run @@ -161,19 +155,18 @@ pub trait FileExt { /// The current file cursor is not affected by this function. /// /// This method will continuously call [`write_at`] until there is no more data - /// to be written or an error of non-[`ErrorKind::Interrupted`] kind is + /// to be written or an error of non-[`io::ErrorKind::Interrupted`] kind is /// returned. This method will not return until the entire buffer has been /// successfully written or such an error occurs. The first error that is - /// not of [`ErrorKind::Interrupted`] kind generated from this method will be + /// not of [`io::ErrorKind::Interrupted`] kind generated from this method will be /// returned. /// /// # Errors /// /// This function will return the first error of - /// non-[`ErrorKind::Interrupted`] kind that [`write_at`] returns. + /// non-[`io::ErrorKind::Interrupted`] kind that [`write_at`] returns. /// - /// [`ErrorKind::Interrupted`]: ../../../../std/io/enum.ErrorKind.html#variant.Interrupted - /// [`write_at`]: #tymethod.write_at + /// [`write_at`]: FileExt::write_at /// /// # Examples /// @@ -223,8 +216,6 @@ impl FileExt for fs::File { } /// Unix-specific extensions to [`fs::Permissions`]. -/// -/// [`fs::Permissions`]: ../../../../std/fs/struct.Permissions.html #[stable(feature = "fs_ext", since = "1.1.0")] pub trait PermissionsExt { /// Returns the underlying raw `st_mode` bits that contain the standard @@ -302,8 +293,6 @@ impl PermissionsExt for Permissions { } /// Unix-specific extensions to [`fs::OpenOptions`]. -/// -/// [`fs::OpenOptions`]: ../../../../std/fs/struct.OpenOptions.html #[stable(feature = "fs_ext", since = "1.1.0")] pub trait OpenOptionsExt { /// Sets the mode bits that a new file will be created with. @@ -372,8 +361,6 @@ impl OpenOptionsExt for OpenOptions { } /// Unix-specific extensions to [`fs::Metadata`]. -/// -/// [`fs::Metadata`]: ../../../../std/fs/struct.Metadata.html #[stable(feature = "metadata_ext", since = "1.1.0")] pub trait MetadataExt { /// Returns the ID of the device containing the file. @@ -535,7 +522,7 @@ pub trait MetadataExt { fn atime(&self) -> i64; /// Returns the last access time of the file, in nanoseconds since [`atime`]. /// - /// [`atime`]: #tymethod.atime + /// [`atime`]: MetadataExt::atime /// /// # Examples /// @@ -571,7 +558,7 @@ pub trait MetadataExt { fn mtime(&self) -> i64; /// Returns the last modification time of the file, in nanoseconds since [`mtime`]. /// - /// [`mtime`]: #tymethod.mtime + /// [`mtime`]: MetadataExt::mtime /// /// # Examples /// @@ -607,7 +594,7 @@ pub trait MetadataExt { fn ctime(&self) -> i64; /// Returns the last status change time of the file, in nanoseconds since [`ctime`]. /// - /// [`ctime`]: #tymethod.ctime + /// [`ctime`]: MetadataExt::ctime /// /// # Examples /// @@ -714,12 +701,10 @@ impl MetadataExt for fs::Metadata { } } -/// Unix-specific extensions for [`FileType`]. +/// Unix-specific extensions for [`fs::FileType`]. /// /// Adds support for special Unix file types such as block/character devices, /// pipes, and sockets. -/// -/// [`FileType`]: ../../../../std/fs/struct.FileType.html #[stable(feature = "file_type_ext", since = "1.5.0")] pub trait FileTypeExt { /// Returns `true` if this file type is a block device. @@ -813,8 +798,6 @@ impl FileTypeExt for fs::FileType { } /// Unix-specific extension methods for [`fs::DirEntry`]. -/// -/// [`fs::DirEntry`]: ../../../../std/fs/struct.DirEntry.html #[stable(feature = "dir_entry_ext", since = "1.1.0")] pub trait DirEntryExt { /// Returns the underlying `d_ino` field in the contained `dirent` @@ -875,8 +858,6 @@ pub fn symlink, Q: AsRef>(src: P, dst: Q) -> io::Result<()> } /// Unix-specific extensions to [`fs::DirBuilder`]. -/// -/// [`fs::DirBuilder`]: ../../../../std/fs/struct.DirBuilder.html #[stable(feature = "dir_builder", since = "1.6.0")] pub trait DirBuilderExt { /// Sets the mode to create new directories with. This option defaults to diff --git a/library/std/src/sys/unix/ext/net.rs b/library/std/src/sys/unix/ext/net.rs index ada8eaa1c9745..f43869a1b2c83 100644 --- a/library/std/src/sys/unix/ext/net.rs +++ b/library/std/src/sys/unix/ext/net.rs @@ -408,10 +408,9 @@ impl UnixStream { /// indefinitely. An [`Err`] is returned if the zero [`Duration`] is passed to this /// method. /// - /// [`None`]: ../../../../std/option/enum.Option.html#variant.None - /// [`Err`]: ../../../../std/result/enum.Result.html#variant.Err - /// [`read`]: ../../../../std/io/trait.Read.html#tymethod.read - /// [`Duration`]: ../../../../std/time/struct.Duration.html + /// [`None`]: crate::option::Option::None + /// [`Err`]: crate::result::Result::Err + /// [`read`]: io::Read::read /// /// # Examples /// @@ -453,10 +452,9 @@ impl UnixStream { /// indefinitely. An [`Err`] is returned if the zero [`Duration`] is /// passed to this method. /// - /// [`None`]: ../../../../std/option/enum.Option.html#variant.None - /// [`Err`]: ../../../../std/result/enum.Result.html#variant.Err - /// [`write`]: ../../../../std/io/trait.Write.html#tymethod.write - /// [`Duration`]: ../../../../std/time/struct.Duration.html + /// [`None`]: crate::option::Option::None + /// [`Err`]: crate::result::Result::Err + /// [`read`]: io::Read::read /// /// # Examples /// @@ -581,8 +579,6 @@ impl UnixStream { /// specified portions to immediately return with an appropriate value /// (see the documentation of [`Shutdown`]). /// - /// [`Shutdown`]: ../../../../std/net/enum.Shutdown.html - /// /// # Examples /// /// ```no_run @@ -852,7 +848,7 @@ impl UnixListener { /// is established. When established, the corresponding [`UnixStream`] and /// the remote peer's address will be returned. /// - /// [`UnixStream`]: ../../../../std/os/unix/net/struct.UnixStream.html + /// [`UnixStream`]: crate::os::unix::net::UnixStream /// /// # Examples /// @@ -937,8 +933,6 @@ impl UnixListener { /// Ok(()) /// } /// ``` - /// - /// [`io::ErrorKind::WouldBlock`]: ../../../io/enum.ErrorKind.html#variant.WouldBlock #[stable(feature = "unix_socket", since = "1.10.0")] pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { self.0.set_nonblocking(nonblocking) @@ -973,8 +967,7 @@ impl UnixListener { /// The iterator will never return [`None`] and will also not yield the /// peer's [`SocketAddr`] structure. /// - /// [`None`]: ../../../../std/option/enum.Option.html#variant.None - /// [`SocketAddr`]: struct.SocketAddr.html + /// [`None`]: crate::option::Option::None /// /// # Examples /// @@ -1043,8 +1036,7 @@ impl<'a> IntoIterator for &'a UnixListener { /// /// It will never return [`None`]. /// -/// [`None`]: ../../../../std/option/enum.Option.html#variant.None -/// [`UnixListener`]: struct.UnixListener.html +/// [`None`]: crate::option::Option::None /// /// # Examples /// @@ -1205,9 +1197,9 @@ impl UnixDatagram { /// The [`send`] method may be used to send data to the specified address. /// [`recv`] and [`recv_from`] will only receive data from that address. /// - /// [`send`]: #method.send - /// [`recv`]: #method.recv - /// [`recv_from`]: #method.recv_from + /// [`send`]: UnixDatagram::send + /// [`recv`]: UnixDatagram::recv + /// [`recv_from`]: UnixDatagram::recv_from /// /// # Examples /// @@ -1284,7 +1276,7 @@ impl UnixDatagram { /// /// The [`connect`] method will connect the socket to a peer. /// - /// [`connect`]: #method.connect + /// [`connect`]: UnixDatagram::connect /// /// # Examples /// @@ -1432,11 +1424,10 @@ impl UnixDatagram { /// block indefinitely. An [`Err`] is returned if the zero [`Duration`] /// is passed to this method. /// - /// [`None`]: ../../../../std/option/enum.Option.html#variant.None - /// [`Err`]: ../../../../std/result/enum.Result.html#variant.Err - /// [`recv`]: #method.recv - /// [`recv_from`]: #method.recv_from - /// [`Duration`]: ../../../../std/time/struct.Duration.html + /// [`None`]: crate::option::Option::None + /// [`Err`]: crate::result::Result::Err + /// [`recv`]: UnixDatagram::recv + /// [`recv_from`]: UnixDatagram::recv_from /// /// # Examples /// @@ -1479,10 +1470,9 @@ impl UnixDatagram { /// block indefinitely. An [`Err`] is returned if the zero [`Duration`] is passed to this /// method. /// - /// [`None`]: ../../../../std/option/enum.Option.html#variant.None - /// [`send`]: #method.send - /// [`send_to`]: #method.send_to - /// [`Duration`]: ../../../../std/time/struct.Duration.html + /// [`None`]: crate::option::Option::None + /// [`send`]: UnixDatagram::send + /// [`send_to`]: UnixDatagram::send_to /// /// # Examples /// @@ -1605,8 +1595,6 @@ impl UnixDatagram { /// specified portions to immediately return with an appropriate value /// (see the documentation of [`Shutdown`]). /// - /// [`Shutdown`]: ../../../../std/net/enum.Shutdown.html - /// /// ```no_run /// use std::os::unix::net::UnixDatagram; /// use std::net::Shutdown; diff --git a/library/std/src/sys/unix/ext/process.rs b/library/std/src/sys/unix/ext/process.rs index 048ce24d6ba88..82527c40e9138 100644 --- a/library/std/src/sys/unix/ext/process.rs +++ b/library/std/src/sys/unix/ext/process.rs @@ -10,8 +10,6 @@ use crate::sys; use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; /// Unix-specific extensions to the [`process::Command`] builder. -/// -/// [`process::Command`]: ../../../../std/process/struct.Command.html #[stable(feature = "rust1", since = "1.0.0")] pub trait CommandExt { /// Sets the child process's user ID. This translates to a @@ -65,7 +63,7 @@ pub trait CommandExt { /// This method is stable and usable, but it should be unsafe. To fix /// that, it got deprecated in favor of the unsafe [`pre_exec`]. /// - /// [`pre_exec`]: #tymethod.pre_exec + /// [`pre_exec`]: CommandExt::pre_exec #[stable(feature = "process_exec", since = "1.15.0")] #[rustc_deprecated(since = "1.37.0", reason = "should be unsafe, use `pre_exec` instead")] fn before_exec(&mut self, f: F) -> &mut process::Command @@ -94,8 +92,6 @@ pub trait CommandExt { /// a new child. Like spawn, however, the default behavior for the stdio /// descriptors will be to inherited from the current process. /// - /// [`process::exit`]: ../../../process/fn.exit.html - /// /// # Notes /// /// The process may be in a "broken state" if this function returns in @@ -151,8 +147,6 @@ impl CommandExt for process::Command { } /// Unix-specific extensions to [`process::ExitStatus`]. -/// -/// [`process::ExitStatus`]: ../../../../std/process/struct.ExitStatus.html #[stable(feature = "rust1", since = "1.0.0")] pub trait ExitStatusExt { /// Creates a new `ExitStatus` from the raw underlying `i32` return value of diff --git a/library/std/src/sys/unix/ext/thread.rs b/library/std/src/sys/unix/ext/thread.rs index 759ef6236e804..7221da1a9a7bb 100644 --- a/library/std/src/sys/unix/ext/thread.rs +++ b/library/std/src/sys/unix/ext/thread.rs @@ -11,9 +11,7 @@ use crate::thread::JoinHandle; #[allow(deprecated)] pub type RawPthread = pthread_t; -/// Unix-specific extensions to [`thread::JoinHandle`]. -/// -/// [`thread::JoinHandle`]: ../../../../std/thread/struct.JoinHandle.html +/// Unix-specific extensions to [`JoinHandle`]. #[stable(feature = "thread_extensions", since = "1.9.0")] pub trait JoinHandleExt { /// Extracts the raw pthread_t without taking ownership From 63d2e9b05f98ff49d593cc392cb31c8d82552726 Mon Sep 17 00:00:00 2001 From: Prabakaran Kumaresshan <4676330+nixphix@users.noreply.github.com> Date: Wed, 19 Aug 2020 06:19:35 +0530 Subject: [PATCH 31/31] resolve comments --- library/std/src/sys/unix/ext/net.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/library/std/src/sys/unix/ext/net.rs b/library/std/src/sys/unix/ext/net.rs index f43869a1b2c83..55803ddfc4323 100644 --- a/library/std/src/sys/unix/ext/net.rs +++ b/library/std/src/sys/unix/ext/net.rs @@ -408,8 +408,6 @@ impl UnixStream { /// indefinitely. An [`Err`] is returned if the zero [`Duration`] is passed to this /// method. /// - /// [`None`]: crate::option::Option::None - /// [`Err`]: crate::result::Result::Err /// [`read`]: io::Read::read /// /// # Examples @@ -452,8 +450,6 @@ impl UnixStream { /// indefinitely. An [`Err`] is returned if the zero [`Duration`] is /// passed to this method. /// - /// [`None`]: crate::option::Option::None - /// [`Err`]: crate::result::Result::Err /// [`read`]: io::Read::read /// /// # Examples @@ -967,8 +963,6 @@ impl UnixListener { /// The iterator will never return [`None`] and will also not yield the /// peer's [`SocketAddr`] structure. /// - /// [`None`]: crate::option::Option::None - /// /// # Examples /// /// ```no_run @@ -1036,8 +1030,6 @@ impl<'a> IntoIterator for &'a UnixListener { /// /// It will never return [`None`]. /// -/// [`None`]: crate::option::Option::None -/// /// # Examples /// /// ```no_run @@ -1424,8 +1416,6 @@ impl UnixDatagram { /// block indefinitely. An [`Err`] is returned if the zero [`Duration`] /// is passed to this method. /// - /// [`None`]: crate::option::Option::None - /// [`Err`]: crate::result::Result::Err /// [`recv`]: UnixDatagram::recv /// [`recv_from`]: UnixDatagram::recv_from /// @@ -1470,7 +1460,6 @@ impl UnixDatagram { /// block indefinitely. An [`Err`] is returned if the zero [`Duration`] is passed to this /// method. /// - /// [`None`]: crate::option::Option::None /// [`send`]: UnixDatagram::send /// [`send_to`]: UnixDatagram::send_to ///