From 644550f3de4805b3dec6ade4a60ecf48c1d2487b Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Sat, 27 Jul 2024 16:47:10 +0200 Subject: [PATCH 01/15] Improve panic message and surrounding documentation for Ord violations The new sort implementations have the ability to detect Ord violations in many cases. This commit improves the message in a way that should help users realize what went wrong in their program. --- .../core/src/slice/sort/shared/smallsort.rs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/library/core/src/slice/sort/shared/smallsort.rs b/library/core/src/slice/sort/shared/smallsort.rs index 5111ed8756bf1..f3b0bf8b018e6 100644 --- a/library/core/src/slice/sort/shared/smallsort.rs +++ b/library/core/src/slice/sort/shared/smallsort.rs @@ -834,9 +834,10 @@ unsafe fn bidirectional_merge bool>( right = right.add((!left_nonempty) as usize); } - // We now should have consumed the full input exactly once. This can - // only fail if the comparison operator fails to be Ord, in which case - // we will panic and never access the inconsistent state in dst. + // We now should have consumed the full input exactly once. This can only fail if the + // user-provided comparison operator fails implements a strict weak ordering as required by + // Ord incorrectly, in which case we will panic and never access the inconsistent state in + // dst. if left != left_end || right != right_end { panic_on_ord_violation(); } @@ -845,7 +846,21 @@ unsafe fn bidirectional_merge bool>( #[inline(never)] fn panic_on_ord_violation() -> ! { - panic!("Ord violation"); + // This is indicative of a logic bug in the user-provided comparison function or Ord + // implementation. They are expected to implement a total order as explained in the Ord + // documentation. + // + // By raising this panic we inform the user, that they have a logic bug in their program. If a + // strict weak ordering is not given, the concept of comparison based sorting makes no sense. + // E.g.: a < b < c < a + // + // The Ord documentation requires users to implement a total order, arguably that's + // unnecessarily strict in the context of sorting. Issues only arise if the weaker requirement + // of a strict weak ordering is violated. + // + // The panic message talks about a total order because that's what the Ord documentation talks + // about and requires, so as to not confuse users. + panic!("user-provided comparison function does not correctly implement a total order"); } #[must_use] From 00ce238885825e5e987dbcd4fef266e96157303d Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Sat, 27 Jul 2024 16:48:42 +0200 Subject: [PATCH 02/15] Improve panic sections for sort*, sort_unstable* and select_nth_unstable* - Move panic information into # Panics section - Fix mentions of T: Ord that should be compare - Add missing information --- library/alloc/src/slice.rs | 16 +++++++++++++--- library/core/src/slice/mod.rs | 27 +++++++++++++++------------ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index c7960b3fb49c3..9e222c6072fd7 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -199,7 +199,9 @@ impl [T] { /// handled without allocation, medium sized slices allocate `self.len()` and beyond that it /// clamps at `self.len() / 2`. /// - /// If `T: Ord` does not implement a total order, the implementation may panic. + /// # Panics + /// + /// May panic if `T: Ord` does not implement a total order. /// /// # Examples /// @@ -258,7 +260,9 @@ impl [T] { /// handled without allocation, medium sized slices allocate `self.len()` and beyond that it /// clamps at `self.len() / 2`. /// - /// If `T: Ord` does not implement a total order, the implementation may panic. + /// # Panics + /// + /// May panic if `compare` does not implement a total order. /// /// # Examples /// @@ -304,7 +308,9 @@ impl [T] { /// handled without allocation, medium sized slices allocate `self.len()` and beyond that it /// clamps at `self.len() / 2`. /// - /// If `K: Ord` does not implement a total order, the implementation may panic. + /// # Panics + /// + /// May panic if `K: Ord` does not implement a total order. /// /// # Examples /// @@ -356,6 +362,10 @@ impl [T] { /// In the worst case, the algorithm allocates temporary storage in a `Vec<(K, usize)>` the /// length of the slice. /// + /// # Panics + /// + /// May panic if `K: Ord` does not implement a total order. + /// /// # Examples /// /// ``` diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 6d3e625bef428..8916d43a8f236 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -2898,7 +2898,9 @@ impl [T] { /// It is typically faster than stable sorting, except in a few special cases, e.g., when the /// slice is partially sorted. /// - /// If `T: Ord` does not implement a total order, the implementation may panic. + /// # Panics + /// + /// May panic if `T: Ord` does not implement a total order. /// /// # Examples /// @@ -2955,7 +2957,9 @@ impl [T] { /// It is typically faster than stable sorting, except in a few special cases, e.g., when the /// slice is partially sorted. /// - /// If `T: Ord` does not implement a total order, the implementation may panic. + /// # Panics + /// + /// May panic if `compare` does not implement a total order. /// /// # Examples /// @@ -2999,7 +3003,9 @@ impl [T] { /// It is typically faster than stable sorting, except in a few special cases, e.g., when the /// slice is partially sorted. /// - /// If `K: Ord` does not implement a total order, the implementation may panic. + /// # Panics + /// + /// May panic if `K: Ord` does not implement a total order. /// /// # Examples /// @@ -3042,15 +3048,14 @@ impl [T] { /// Median of Medians using Tukey's Ninther for pivot selection, which guarantees linear runtime /// for all inputs. /// - /// It is typically faster than stable sorting, except in a few special cases, e.g., when the - /// slice is nearly fully sorted, where `slice::sort` may be faster. - /// /// [`sort_unstable`]: slice::sort_unstable /// /// # Panics /// /// Panics when `index >= len()`, meaning it always panics on empty slices. /// + /// May panic if `T: Ord` does not implement a total order. + /// /// # Examples /// /// ``` @@ -3103,15 +3108,14 @@ impl [T] { /// Median of Medians using Tukey's Ninther for pivot selection, which guarantees linear runtime /// for all inputs. /// - /// It is typically faster than stable sorting, except in a few special cases, e.g., when the - /// slice is nearly fully sorted, where `slice::sort` may be faster. - /// /// [`sort_unstable`]: slice::sort_unstable /// /// # Panics /// /// Panics when `index >= len()`, meaning it always panics on empty slices. /// + /// May panic if `compare` does not implement a total order. + /// /// # Examples /// /// ``` @@ -3168,15 +3172,14 @@ impl [T] { /// Median of Medians using Tukey's Ninther for pivot selection, which guarantees linear runtime /// for all inputs. /// - /// It is typically faster than stable sorting, except in a few special cases, e.g., when the - /// slice is nearly fully sorted, where `slice::sort` may be faster. - /// /// [`sort_unstable`]: slice::sort_unstable /// /// # Panics /// /// Panics when `index >= len()`, meaning it always panics on empty slices. /// + /// May panic if `K: Ord` does not implement a total order. + /// /// # Examples /// /// ``` From afc404fdfc1559b4ba2846a054347e806bb8f465 Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Wed, 31 Jul 2024 11:28:11 +0200 Subject: [PATCH 03/15] Apply review comments - Use if the implementation of [`Ord`] for `T` language - Link to total order wiki page - Rework total order help and examples - Improve language to be more precise and less prone to misunderstandings. - Fix usage of `sort_unstable_by` in `sort_by` example - Fix missing author mention - Use more consistent example input for sort - Use more idiomatic assert_eq! in examples - Use more natural "comparison function" language instead of "comparator function" --- library/alloc/src/slice.rs | 88 ++++++++++--------- library/core/src/slice/mod.rs | 80 +++++++++-------- .../core/src/slice/sort/shared/smallsort.rs | 13 ++- library/core/src/slice/sort/unstable/mod.rs | 2 +- 4 files changed, 97 insertions(+), 86 deletions(-) diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index 9e222c6072fd7..21270dbed0c61 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -179,15 +179,25 @@ impl [T] { /// This sort is stable (i.e., does not reorder equal elements) and *O*(*n* \* log(*n*)) /// worst-case. /// - /// If `T: Ord` does not implement a total order the resulting order is unspecified. All - /// original elements will remain in the slice and any possible modifications via interior - /// mutability are observed in the input. Same is true if `T: Ord` panics. + /// If the implementation of [`Ord`] for `T` does not implement a [total order] the resulting + /// order of elements in the slice is unspecified. All original elements will remain in the + /// slice and any possible modifications via interior mutability are observed in the input. Same + /// is true if the implementation of [`Ord`] for `T` panics. /// /// When applicable, unstable sorting is preferred because it is generally faster than stable /// sorting and it doesn't allocate auxiliary memory. See /// [`sort_unstable`](slice::sort_unstable). The exception are partially sorted slices, which /// may be better served with `slice::sort`. /// + /// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] requires + /// additional precautions. For example Rust defines `NaN != NaN`, which doesn't fulfill the + /// reflexivity requirement posed by [`Ord`]. By using an alternative comparison function with + /// [`slice::sort_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a [total + /// order] users can sort slices containing floating point numbers. Alternatively, if one can + /// guarantee that all values in the slice are comparable with [`PartialOrd::partial_cmp`] *and* + /// the implementation forms a [total order], it's possible to sort the slice with `sort_by(|a, + /// b| a.partial_cmp(b).unwrap())`. + /// /// # Current implementation /// /// The current implementation is based on [driftsort] by Orson Peters and Lukas Bergdoll, which @@ -201,18 +211,19 @@ impl [T] { /// /// # Panics /// - /// May panic if `T: Ord` does not implement a total order. + /// May panic if the implementation of [`Ord`] for `T` does not implement a [total order]. /// /// # Examples /// /// ``` - /// let mut v = [-5, 4, 1, -3, 2]; + /// let mut v = [4, -5, 1, -3, 2]; /// /// v.sort(); - /// assert!(v == [-5, -3, 1, 2, 4]); + /// assert_eq!(v, [-5, -3, 1, 2, 4]); /// ``` /// /// [driftsort]: https://github.com/Voultapher/driftsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[cfg(not(no_global_oom_handling))] #[rustc_allow_incoherent_impl] #[stable(feature = "rust1", since = "1.0.0")] @@ -224,30 +235,19 @@ impl [T] { stable_sort(self, T::lt); } - /// Sorts the slice with a comparator function, preserving initial order of equal elements. + /// Sorts the slice with a comparison function, preserving initial order of equal elements. /// /// This sort is stable (i.e., does not reorder equal elements) and *O*(*n* \* log(*n*)) /// worst-case. /// - /// The comparator function should define a total ordering for the elements in the slice. If the - /// ordering is not total, the order of the elements is unspecified. - /// - /// If the comparator function does not implement a total order the resulting order is - /// unspecified. All original elements will remain in the slice and any possible modifications - /// via interior mutability are observed in the input. Same is true if the comparator function - /// panics. A total order (for all `a`, `b` and `c`): - /// - /// * total and antisymmetric: exactly one of `a < b`, `a == b` or `a > b` is true, and - /// * transitive, `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`. + /// If the comparison function `compare` does not implement a [total order] the resulting order + /// of elements in the slice is unspecified. All original elements will remain in the slice and + /// any possible modifications via interior mutability are observed in the input. Same is true + /// if `compare` panics. /// - /// For example, while [`f64`] doesn't implement [`Ord`] because `NaN != NaN`, we can use - /// `partial_cmp` as our sort function when we know the slice doesn't contain a `NaN`. - /// - /// ``` - /// let mut floats = [5f64, 4.0, 1.0, 3.0, 2.0]; - /// floats.sort_unstable_by(|a, b| a.partial_cmp(b).unwrap()); - /// assert_eq!(floats, [1.0, 2.0, 3.0, 4.0, 5.0]); - /// ``` + /// For example `|a, b| (a - b).cmp(a)` is a comparison function that is neither transitive nor + /// reflexive nor total, `a < b < c < a` with `a = 1, b = 2, c = 3`. For more information and + /// examples see the [`Ord`] documentation. /// /// # Current implementation /// @@ -262,21 +262,22 @@ impl [T] { /// /// # Panics /// - /// May panic if `compare` does not implement a total order. + /// May panic if `compare` does not implement a [total order]. /// /// # Examples /// /// ``` - /// let mut v = [5, 4, 1, 3, 2]; + /// let mut v = [4, -5, 1, -3, 2]; /// v.sort_by(|a, b| a.cmp(b)); - /// assert!(v == [1, 2, 3, 4, 5]); + /// assert_eq!(v, [-5, -3, 1, 2, 4]); /// /// // reverse sorting /// v.sort_by(|a, b| b.cmp(a)); - /// assert!(v == [5, 4, 3, 2, 1]); + /// assert_eq!(v, [4, 2, 1, -3, -5]); /// ``` /// /// [driftsort]: https://github.com/Voultapher/driftsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[cfg(not(no_global_oom_handling))] #[rustc_allow_incoherent_impl] #[stable(feature = "rust1", since = "1.0.0")] @@ -293,9 +294,10 @@ impl [T] { /// This sort is stable (i.e., does not reorder equal elements) and *O*(*m* \* *n* \* log(*n*)) /// worst-case, where the key function is *O*(*m*). /// - /// If `K: Ord` does not implement a total order the resulting order is unspecified. - /// All original elements will remain in the slice and any possible modifications via interior - /// mutability are observed in the input. Same is true if `K: Ord` panics. + /// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting + /// order of elements in the slice is unspecified. All original elements will remain in the + /// slice and any possible modifications via interior mutability are observed in the input. Same + /// is true if the implementation of [`Ord`] for `K` panics. /// /// # Current implementation /// @@ -310,18 +312,19 @@ impl [T] { /// /// # Panics /// - /// May panic if `K: Ord` does not implement a total order. + /// May panic if the implementation of [`Ord`] for `K` does not implement a [total order]. /// /// # Examples /// /// ``` - /// let mut v = [-5i32, 4, 1, -3, 2]; + /// let mut v = [4i32, -5, 1, -3, 2]; /// /// v.sort_by_key(|k| k.abs()); - /// assert!(v == [1, 2, -3, 4, -5]); + /// assert_eq!(v, [1, 2, -3, 4, -5]); /// ``` /// /// [driftsort]: https://github.com/Voultapher/driftsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[cfg(not(no_global_oom_handling))] #[rustc_allow_incoherent_impl] #[stable(feature = "slice_sort_by_key", since = "1.7.0")] @@ -343,9 +346,10 @@ impl [T] { /// storage to remember the results of key evaluation. The order of calls to the key function is /// unspecified and may change in future versions of the standard library. /// - /// If `K: Ord` does not implement a total order the resulting order is unspecified. - /// All original elements will remain in the slice and any possible modifications via interior - /// mutability are observed in the input. Same is true if `K: Ord` panics. + /// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting + /// order of elements in the slice is unspecified. All original elements will remain in the + /// slice and any possible modifications via interior mutability are observed in the input. Same + /// is true if the implementation of [`Ord`] for `K` panics. /// /// For simple key functions (e.g., functions that are property accesses or basic operations), /// [`sort_by_key`](slice::sort_by_key) is likely to be faster. @@ -364,18 +368,20 @@ impl [T] { /// /// # Panics /// - /// May panic if `K: Ord` does not implement a total order. + /// May panic if the implementation of [`Ord`] for `K` does not implement a [total order]. /// /// # Examples /// /// ``` - /// let mut v = [-5i32, 4, 32, -3, 2]; + /// let mut v = [4i32, -5, 1, -3, 2, 10]; /// + /// // Strings are sorted by lexicographical order. /// v.sort_by_cached_key(|k| k.to_string()); - /// assert!(v == [-3, -5, 2, 32, 4]); + /// assert_eq!(v, [-3, -5, 1, 10, 2, 4]); /// ``` /// /// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[cfg(not(no_global_oom_handling))] #[rustc_allow_incoherent_impl] #[stable(feature = "slice_sort_by_cached_key", since = "1.34.0")] diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 8916d43a8f236..223ba96c4755b 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -2884,9 +2884,19 @@ impl [T] { /// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not /// allocate), and *O*(*n* \* log(*n*)) worst-case. /// - /// If `T: Ord` does not implement a total order the resulting order is unspecified. All - /// original elements will remain in the slice and any possible modifications via interior - /// mutability are observed in the input. Same is true if `T: Ord` panics. + /// If the implementation of [`Ord`] for `T` does not implement a [total order] the resulting + /// order of elements in the slice is unspecified. All original elements will remain in the + /// slice and any possible modifications via interior mutability are observed in the input. Same + /// is true if the implementation of [`Ord`] for `T` panics. + /// + /// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] requires + /// additional precautions. For example Rust defines `NaN != NaN`, which doesn't fulfill the + /// reflexivity requirement posed by [`Ord`]. By using an alternative comparison function with + /// [`slice::sort_unstable_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a + /// [total order] users can sort slices containing floating point numbers. Alternatively, if one + /// can guarantee that all values in the slice are comparable with [`PartialOrd::partial_cmp`] + /// *and* the implementation forms a [total order], it's possible to sort the slice with + /// `sort_unstable_by(|a, b| a.partial_cmp(b).unwrap())`. /// /// # Current implementation /// @@ -2900,18 +2910,19 @@ impl [T] { /// /// # Panics /// - /// May panic if `T: Ord` does not implement a total order. + /// May panic if the implementation of [`Ord`] for `T` does not implement a [total order]. /// /// # Examples /// /// ``` - /// let mut v = [-5, 4, 1, -3, 2]; + /// let mut v = [4, -5, 1, -3, 2]; /// /// v.sort_unstable(); - /// assert!(v == [-5, -3, 1, 2, 4]); + /// assert_eq!(v, [-5, -3, 1, 2, 4]); /// ``` /// /// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[stable(feature = "sort_unstable", since = "1.20.0")] #[inline] pub fn sort_unstable(&mut self) @@ -2921,31 +2932,20 @@ impl [T] { sort::unstable::sort(self, &mut T::lt); } - /// Sorts the slice with a comparator function, **without** preserving the initial order of + /// Sorts the slice with a comparison function, **without** preserving the initial order of /// equal elements. /// /// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not /// allocate), and *O*(*n* \* log(*n*)) worst-case. /// - /// The comparator function should define a total ordering for the elements in the slice. If the - /// ordering is not total, the order of the elements is unspecified. + /// If the comparison function `compare` does not implement a [total order] the resulting order + /// of elements in the slice is unspecified. All original elements will remain in the slice and + /// any possible modifications via interior mutability are observed in the input. Same is true + /// if `compare` panics. /// - /// If the comparator function does not implement a total order the resulting order is - /// unspecified. All original elements will remain in the slice and any possible modifications - /// via interior mutability are observed in the input. Same is true if the comparator function - /// panics. A total order (for all `a`, `b` and `c`): - /// - /// * total and antisymmetric: exactly one of `a < b`, `a == b` or `a > b` is true, and - /// * transitive, `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`. - /// - /// For example, while [`f64`] doesn't implement [`Ord`] because `NaN != NaN`, we can use - /// `partial_cmp` as our sort function when we know the slice doesn't contain a `NaN`. - /// - /// ``` - /// let mut floats = [5f64, 4.0, 1.0, 3.0, 2.0]; - /// floats.sort_unstable_by(|a, b| a.partial_cmp(b).unwrap()); - /// assert_eq!(floats, [1.0, 2.0, 3.0, 4.0, 5.0]); - /// ``` + /// For example `|a, b| (a - b).cmp(a)` is a comparison function that is neither transitive nor + /// reflexive nor total, `a < b < c < a` with `a = 1, b = 2, c = 3`. For more information and + /// examples see the [`Ord`] documentation. /// /// # Current implementation /// @@ -2959,21 +2959,22 @@ impl [T] { /// /// # Panics /// - /// May panic if `compare` does not implement a total order. + /// May panic if `compare` does not implement a [total order]. /// /// # Examples /// /// ``` - /// let mut v = [5, 4, 1, 3, 2]; + /// let mut v = [4, -5, 1, -3, 2]; /// v.sort_unstable_by(|a, b| a.cmp(b)); - /// assert!(v == [1, 2, 3, 4, 5]); + /// assert_eq!(v, [-5, -3, 1, 2, 4]); /// /// // reverse sorting /// v.sort_unstable_by(|a, b| b.cmp(a)); - /// assert!(v == [5, 4, 3, 2, 1]); + /// assert_eq!(v, [4, 2, 1, -3, -5]); /// ``` /// /// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[stable(feature = "sort_unstable", since = "1.20.0")] #[inline] pub fn sort_unstable_by(&mut self, mut compare: F) @@ -2989,9 +2990,10 @@ impl [T] { /// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not /// allocate), and *O*(*n* \* log(*n*)) worst-case. /// - /// If `K: Ord` does not implement a total order the resulting order is unspecified. - /// All original elements will remain in the slice and any possible modifications via interior - /// mutability are observed in the input. Same is true if `K: Ord` panics. + /// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting + /// order of elements in the slice is unspecified. All original elements will remain in the + /// slice and any possible modifications via interior mutability are observed in the input. Same + /// is true if the implementation of [`Ord`] for `K` panics. /// /// # Current implementation /// @@ -3005,18 +3007,19 @@ impl [T] { /// /// # Panics /// - /// May panic if `K: Ord` does not implement a total order. + /// May panic if the implementation of [`Ord`] for `K` does not implement a [total order]. /// /// # Examples /// /// ``` - /// let mut v = [-5i32, 4, 1, -3, 2]; + /// let mut v = [4i32, -5, 1, -3, 2]; /// /// v.sort_unstable_by_key(|k| k.abs()); - /// assert!(v == [1, 2, -3, 4, -5]); + /// assert_eq!(v, [1, 2, -3, 4, -5]); /// ``` /// /// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[stable(feature = "sort_unstable", since = "1.20.0")] #[inline] pub fn sort_unstable_by_key(&mut self, mut f: F) @@ -3054,7 +3057,7 @@ impl [T] { /// /// Panics when `index >= len()`, meaning it always panics on empty slices. /// - /// May panic if `T: Ord` does not implement a total order. + /// May panic if the implementation of [`Ord`] for `T` does not implement a [total order]. /// /// # Examples /// @@ -3078,6 +3081,7 @@ impl [T] { /// ``` /// /// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[stable(feature = "slice_select_nth_unstable", since = "1.49.0")] #[inline] pub fn select_nth_unstable(&mut self, index: usize) -> (&mut [T], &mut T, &mut [T]) @@ -3114,7 +3118,7 @@ impl [T] { /// /// Panics when `index >= len()`, meaning it always panics on empty slices. /// - /// May panic if `compare` does not implement a total order. + /// May panic if `compare` does not implement a [total order]. /// /// # Examples /// @@ -3138,6 +3142,7 @@ impl [T] { /// ``` /// /// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[stable(feature = "slice_select_nth_unstable", since = "1.49.0")] #[inline] pub fn select_nth_unstable_by( @@ -3202,6 +3207,7 @@ impl [T] { /// ``` /// /// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort + /// [total order]: https://en.wikipedia.org/wiki/Total_order #[stable(feature = "slice_select_nth_unstable", since = "1.49.0")] #[inline] pub fn select_nth_unstable_by_key( diff --git a/library/core/src/slice/sort/shared/smallsort.rs b/library/core/src/slice/sort/shared/smallsort.rs index f3b0bf8b018e6..e168e8a4478b8 100644 --- a/library/core/src/slice/sort/shared/smallsort.rs +++ b/library/core/src/slice/sort/shared/smallsort.rs @@ -835,9 +835,8 @@ unsafe fn bidirectional_merge bool>( } // We now should have consumed the full input exactly once. This can only fail if the - // user-provided comparison operator fails implements a strict weak ordering as required by - // Ord incorrectly, in which case we will panic and never access the inconsistent state in - // dst. + // user-provided comparison function fails to implement a strict weak ordering. In that case + // we panic and never access the inconsistent state in dst. if left != left_end || right != right_end { panic_on_ord_violation(); } @@ -850,11 +849,11 @@ fn panic_on_ord_violation() -> ! { // implementation. They are expected to implement a total order as explained in the Ord // documentation. // - // By raising this panic we inform the user, that they have a logic bug in their program. If a - // strict weak ordering is not given, the concept of comparison based sorting makes no sense. - // E.g.: a < b < c < a + // By panicking we inform the user, that they have a logic bug in their program. If a strict + // weak ordering is not given, the concept of comparison based sorting cannot yield a sorted + // result. E.g.: a < b < c < a // - // The Ord documentation requires users to implement a total order, arguably that's + // The Ord documentation requires users to implement a total order. Arguably that's // unnecessarily strict in the context of sorting. Issues only arise if the weaker requirement // of a strict weak ordering is violated. // diff --git a/library/core/src/slice/sort/unstable/mod.rs b/library/core/src/slice/sort/unstable/mod.rs index 692c2d8f7c7ba..17aa3994bf20d 100644 --- a/library/core/src/slice/sort/unstable/mod.rs +++ b/library/core/src/slice/sort/unstable/mod.rs @@ -9,7 +9,7 @@ use crate::slice::sort::shared::smallsort::insertion_sort_shift_left; pub(crate) mod heapsort; pub(crate) mod quicksort; -/// Unstable sort called ipnsort by Lukas Bergdoll. +/// Unstable sort called ipnsort by Lukas Bergdoll and Orson Peters. /// Design document: /// /// From eae7a186b221bb2b4a4bedeb19d5aca658e91c25 Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Thu, 1 Aug 2024 17:37:07 +0200 Subject: [PATCH 04/15] Hide internal sort module --- library/core/src/slice/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 223ba96c4755b..862a18c3f4a4f 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -31,6 +31,7 @@ pub mod memchr; issue = "none", reason = "exposed from core to be reused in std;" )] +#[doc(hidden)] pub mod sort; mod ascii; From 613155c96ab32daf12514e76476132e6b84adaf8 Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Sat, 3 Aug 2024 15:10:27 +0200 Subject: [PATCH 05/15] Apply review comments to PartialOrd section --- library/alloc/src/slice.rs | 14 +++++++------- library/core/src/slice/mod.rs | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index 21270dbed0c61..aaa6a2abbd904 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -189,14 +189,14 @@ impl [T] { /// [`sort_unstable`](slice::sort_unstable). The exception are partially sorted slices, which /// may be better served with `slice::sort`. /// - /// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] requires - /// additional precautions. For example Rust defines `NaN != NaN`, which doesn't fulfill the - /// reflexivity requirement posed by [`Ord`]. By using an alternative comparison function with + /// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] require + /// additional precautions. For example, `f32::NAN != f32::NAN`, which doesn't fulfill the + /// reflexivity requirement of [`Ord`]. By using an alternative comparison function with /// [`slice::sort_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a [total - /// order] users can sort slices containing floating point numbers. Alternatively, if one can - /// guarantee that all values in the slice are comparable with [`PartialOrd::partial_cmp`] *and* - /// the implementation forms a [total order], it's possible to sort the slice with `sort_by(|a, - /// b| a.partial_cmp(b).unwrap())`. + /// order] users can sort slices containing floating-point values. Alternatively, if all values + /// in the slice are guaranteed to be in a subset for which [`PartialOrd::partial_cmp`] forms a + /// [total order], it's possible to sort the slice with `sort_by(|a, b| + /// a.partial_cmp(b).unwrap())`. /// /// # Current implementation /// diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 862a18c3f4a4f..e75a9a88045e9 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -2890,14 +2890,14 @@ impl [T] { /// slice and any possible modifications via interior mutability are observed in the input. Same /// is true if the implementation of [`Ord`] for `T` panics. /// - /// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] requires - /// additional precautions. For example Rust defines `NaN != NaN`, which doesn't fulfill the - /// reflexivity requirement posed by [`Ord`]. By using an alternative comparison function with + /// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] require + /// additional precautions. For example, `f32::NAN != f32::NAN`, which doesn't fulfill the + /// reflexivity requirement of [`Ord`]. By using an alternative comparison function with /// [`slice::sort_unstable_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a - /// [total order] users can sort slices containing floating point numbers. Alternatively, if one - /// can guarantee that all values in the slice are comparable with [`PartialOrd::partial_cmp`] - /// *and* the implementation forms a [total order], it's possible to sort the slice with - /// `sort_unstable_by(|a, b| a.partial_cmp(b).unwrap())`. + /// [total order] users can sort slices containing floating-point values. Alternatively, if all + /// values in the slice are guaranteed to be in a subset for which [`PartialOrd::partial_cmp`] + /// forms a [total order], it's possible to sort the slice with `sort_unstable_by(|a, b| + /// a.partial_cmp(b).unwrap())`. /// /// # Current implementation /// From 1be60b5d2b6ac569d51abd376e6f04e2fc07692c Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Fri, 9 Aug 2024 15:05:37 +0200 Subject: [PATCH 06/15] Fix linkchecker issue --- library/alloc/src/slice.rs | 2 +- library/core/src/slice/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index aaa6a2abbd904..ef7469c68de62 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -192,7 +192,7 @@ impl [T] { /// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] require /// additional precautions. For example, `f32::NAN != f32::NAN`, which doesn't fulfill the /// reflexivity requirement of [`Ord`]. By using an alternative comparison function with - /// [`slice::sort_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a [total + /// `slice::sort_by` such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a [total /// order] users can sort slices containing floating-point values. Alternatively, if all values /// in the slice are guaranteed to be in a subset for which [`PartialOrd::partial_cmp`] forms a /// [total order], it's possible to sort the slice with `sort_by(|a, b| diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index e75a9a88045e9..d9945e860e6fc 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -2893,7 +2893,7 @@ impl [T] { /// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] require /// additional precautions. For example, `f32::NAN != f32::NAN`, which doesn't fulfill the /// reflexivity requirement of [`Ord`]. By using an alternative comparison function with - /// [`slice::sort_unstable_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a + /// `slice::sort_unstable_by` such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a /// [total order] users can sort slices containing floating-point values. Alternatively, if all /// values in the slice are guaranteed to be in a subset for which [`PartialOrd::partial_cmp`] /// forms a [total order], it's possible to sort the slice with `sort_unstable_by(|a, b| From b735547025dad853829a7dbf6b6697090e3f657c Mon Sep 17 00:00:00 2001 From: kraktus <56031107+kraktus@users.noreply.github.com> Date: Fri, 9 Aug 2024 20:50:00 +0200 Subject: [PATCH 07/15] rustdoc-json-types `Discriminant`: fix typo "when to complex" should obviously be "too complex" --- src/rustdoc-json-types/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rustdoc-json-types/lib.rs b/src/rustdoc-json-types/lib.rs index a93ac6ccaf1df..999134a40909d 100644 --- a/src/rustdoc-json-types/lib.rs +++ b/src/rustdoc-json-types/lib.rs @@ -634,7 +634,7 @@ pub struct Discriminant { /// hexadecimal, and underscores), making it unsuitable to be machine /// interpreted. /// - /// In some cases, when the value is to complex, this may be `"{ _ }"`. + /// In some cases, when the value is too complex, this may be `"{ _ }"`. /// When this occurs is unstable, and may change without notice. pub expr: String, /// The numerical value of the discriminant. Stored as a string due to From 6ef0ac2b5c3f1183c42f9f3a3a5970783af10ecc Mon Sep 17 00:00:00 2001 From: Kajetan Puchalski Date: Fri, 9 Aug 2024 21:26:27 +0100 Subject: [PATCH 08/15] gitignore: Add Zed and Helix editors --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index f2cdd8762f230..a36cb51de33ff 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,8 @@ Session.vim .vscode .project .vim/ +.helix/ +.zed/ .favorites.json .settings/ .vs/ From 2cc029edf5239e1754f2cde3de4f69c45c134550 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Fri, 9 Aug 2024 21:17:32 +0000 Subject: [PATCH 09/15] Only link libc on *nix platforms --- tests/run-make/fmt-write-bloat/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run-make/fmt-write-bloat/main.rs b/tests/run-make/fmt-write-bloat/main.rs index e86c48014c3aa..6f206d6515a37 100644 --- a/tests/run-make/fmt-write-bloat/main.rs +++ b/tests/run-make/fmt-write-bloat/main.rs @@ -5,7 +5,7 @@ use core::fmt; use core::fmt::Write; -#[link(name = "c")] +#[cfg_attr(not(windows), link(name = "c"))] extern "C" {} struct Dummy; From ef90df69044df03daece7ad367792a6424420f90 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Fri, 9 Aug 2024 21:21:43 +0000 Subject: [PATCH 10/15] Update reason why fmt-write-bloat ignores windows --- tests/run-make/fmt-write-bloat/rmake.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/run-make/fmt-write-bloat/rmake.rs b/tests/run-make/fmt-write-bloat/rmake.rs index 4ae226ec0e234..6875ef9ddc05e 100644 --- a/tests/run-make/fmt-write-bloat/rmake.rs +++ b/tests/run-make/fmt-write-bloat/rmake.rs @@ -15,9 +15,12 @@ //! `NO_DEBUG_ASSERTIONS=1`). If debug assertions are disabled, then we can check for the absence of //! additional `usize` formatting and padding related symbols. -// Reason: This test is `ignore-windows` because the `no_std` test (using `#[link(name = "c")])` -// doesn't link on windows. //@ ignore-windows +// Reason: +// - MSVC targets really need to parse the .pdb file (aka the debug information). +// On Windows there's an API for that (dbghelp) which maybe we can use +// - MinGW targets have a lot of symbols included in their runtime which we can't avoid. +// We would need to make the symbols we're looking for more specific for this test to work. //@ ignore-cross-compile use run_make_support::env::no_debug_assertions; From 4dc13c54714ed854023a03cb9b09b81a6e01f08b Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 9 Aug 2024 16:15:40 -0700 Subject: [PATCH 11/15] diagnostics: do not warn when a lifetime bound infers itself --- compiler/rustc_lint/src/builtin.rs | 21 +++++++--- .../edition-lint-infer-outlives-macro.rs | 0 .../edition-lint-infer-outlives-macro.fixed | 0 .../edition-lint-infer-outlives-macro.rs | 0 .../edition-lint-infer-outlives-macro.stderr | 0 .../edition-lint-infer-outlives-multispan.rs | 0 ...ition-lint-infer-outlives-multispan.stderr | 0 .../edition-lint-infer-outlives.fixed | 0 .../edition-lint-infer-outlives.rs | 0 .../edition-lint-infer-outlives.stderr | 0 .../explicit-outlives-recursive-119228.fixed | 41 +++++++++++++++++++ .../explicit-outlives-recursive-119228.rs | 41 +++++++++++++++++++ 12 files changed, 97 insertions(+), 6 deletions(-) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/auxiliary/edition-lint-infer-outlives-macro.rs (100%) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/edition-lint-infer-outlives-macro.fixed (100%) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/edition-lint-infer-outlives-macro.rs (100%) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/edition-lint-infer-outlives-macro.stderr (100%) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/edition-lint-infer-outlives-multispan.rs (100%) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/edition-lint-infer-outlives-multispan.stderr (100%) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/edition-lint-infer-outlives.fixed (100%) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/edition-lint-infer-outlives.rs (100%) rename tests/ui/rust-2018/{ => edition-lint-inter-outlives}/edition-lint-infer-outlives.stderr (100%) create mode 100644 tests/ui/rust-2018/edition-lint-inter-outlives/explicit-outlives-recursive-119228.fixed create mode 100644 tests/ui/rust-2018/edition-lint-inter-outlives/explicit-outlives-recursive-119228.rs diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index d8674817cb5eb..6b36944b20889 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1924,14 +1924,13 @@ declare_lint_pass!(ExplicitOutlivesRequirements => [EXPLICIT_OUTLIVES_REQUIREMEN impl ExplicitOutlivesRequirements { fn lifetimes_outliving_lifetime<'tcx>( tcx: TyCtxt<'tcx>, - inferred_outlives: &'tcx [(ty::Clause<'tcx>, Span)], + inferred_outlives: impl Iterator, Span)>, item: DefId, lifetime: DefId, ) -> Vec> { let item_generics = tcx.generics_of(item); inferred_outlives - .iter() .filter_map(|(clause, _)| match clause.kind().skip_binder() { ty::ClauseKind::RegionOutlives(ty::OutlivesPredicate(a, b)) => match *a { ty::ReEarlyParam(ebr) @@ -1947,11 +1946,10 @@ impl ExplicitOutlivesRequirements { } fn lifetimes_outliving_type<'tcx>( - inferred_outlives: &'tcx [(ty::Clause<'tcx>, Span)], + inferred_outlives: impl Iterator, Span)>, index: u32, ) -> Vec> { inferred_outlives - .iter() .filter_map(|(clause, _)| match clause.kind().skip_binder() { ty::ClauseKind::TypeOutlives(ty::OutlivesPredicate(a, b)) => { a.is_param(index).then_some(b) @@ -2094,7 +2092,11 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements { ( Self::lifetimes_outliving_lifetime( cx.tcx, - inferred_outlives, + // don't warn if the inferred span actually came from the predicate we're looking at + // this happens if the type is recursively defined + inferred_outlives + .iter() + .filter(|(_, span)| !predicate.span.contains(*span)), item.owner_id.to_def_id(), region_def_id, ), @@ -2116,7 +2118,14 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements { }; let index = ty_generics.param_def_id_to_index[&def_id]; ( - Self::lifetimes_outliving_type(inferred_outlives, index), + Self::lifetimes_outliving_type( + // don't warn if the inferred span actually came from the predicate we're looking at + // this happens if the type is recursively defined + inferred_outlives.iter().filter(|(_, span)| { + !predicate.span.contains(*span) + }), + index, + ), &predicate.bounds, predicate.span, predicate.origin == PredicateOrigin::WhereClause, diff --git a/tests/ui/rust-2018/auxiliary/edition-lint-infer-outlives-macro.rs b/tests/ui/rust-2018/edition-lint-inter-outlives/auxiliary/edition-lint-infer-outlives-macro.rs similarity index 100% rename from tests/ui/rust-2018/auxiliary/edition-lint-infer-outlives-macro.rs rename to tests/ui/rust-2018/edition-lint-inter-outlives/auxiliary/edition-lint-infer-outlives-macro.rs diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives-macro.fixed b/tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-macro.fixed similarity index 100% rename from tests/ui/rust-2018/edition-lint-infer-outlives-macro.fixed rename to tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-macro.fixed diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives-macro.rs b/tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-macro.rs similarity index 100% rename from tests/ui/rust-2018/edition-lint-infer-outlives-macro.rs rename to tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-macro.rs diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives-macro.stderr b/tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-macro.stderr similarity index 100% rename from tests/ui/rust-2018/edition-lint-infer-outlives-macro.stderr rename to tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-macro.stderr diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives-multispan.rs b/tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-multispan.rs similarity index 100% rename from tests/ui/rust-2018/edition-lint-infer-outlives-multispan.rs rename to tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-multispan.rs diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives-multispan.stderr b/tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-multispan.stderr similarity index 100% rename from tests/ui/rust-2018/edition-lint-infer-outlives-multispan.stderr rename to tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives-multispan.stderr diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives.fixed b/tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives.fixed similarity index 100% rename from tests/ui/rust-2018/edition-lint-infer-outlives.fixed rename to tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives.fixed diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives.rs b/tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives.rs similarity index 100% rename from tests/ui/rust-2018/edition-lint-infer-outlives.rs rename to tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives.rs diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives.stderr b/tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives.stderr similarity index 100% rename from tests/ui/rust-2018/edition-lint-infer-outlives.stderr rename to tests/ui/rust-2018/edition-lint-inter-outlives/edition-lint-infer-outlives.stderr diff --git a/tests/ui/rust-2018/edition-lint-inter-outlives/explicit-outlives-recursive-119228.fixed b/tests/ui/rust-2018/edition-lint-inter-outlives/explicit-outlives-recursive-119228.fixed new file mode 100644 index 0000000000000..7b9fac8f408af --- /dev/null +++ b/tests/ui/rust-2018/edition-lint-inter-outlives/explicit-outlives-recursive-119228.fixed @@ -0,0 +1,41 @@ +//@ run-rustfix +//@ check-pass +#![deny(explicit_outlives_requirements)] + +pub trait TypeCx { + type Ty; +} + +pub struct Pat { + pub ty: Cx::Ty, +} + +// Simple recursive case: no warning +pub struct MyTypeContextSimpleRecursive<'thir, 'tcx: 'thir> { + pub pat: Pat>, +} +impl<'thir, 'tcx: 'thir> TypeCx for MyTypeContextSimpleRecursive<'thir, 'tcx> { + type Ty = (); +} + +// Non-recursive case: we want a warning +pub struct MyTypeContextNotRecursive<'thir, 'tcx: 'thir> { + pub tcx: &'tcx (), + pub thir: &'thir (), +} +impl<'thir, 'tcx: 'thir> TypeCx for MyTypeContextNotRecursive<'thir, 'tcx> { + type Ty = (); +} + + +// Mixed-recursive case: we want a warning +pub struct MyTypeContextMixedRecursive<'thir, 'tcx: 'thir> { + pub pat: Pat>, + pub tcx: &'tcx (), + pub thir: &'thir (), +} +impl<'thir, 'tcx: 'thir> TypeCx for MyTypeContextMixedRecursive<'thir, 'tcx> { + type Ty = (); +} + +fn main() {} diff --git a/tests/ui/rust-2018/edition-lint-inter-outlives/explicit-outlives-recursive-119228.rs b/tests/ui/rust-2018/edition-lint-inter-outlives/explicit-outlives-recursive-119228.rs new file mode 100644 index 0000000000000..7b9fac8f408af --- /dev/null +++ b/tests/ui/rust-2018/edition-lint-inter-outlives/explicit-outlives-recursive-119228.rs @@ -0,0 +1,41 @@ +//@ run-rustfix +//@ check-pass +#![deny(explicit_outlives_requirements)] + +pub trait TypeCx { + type Ty; +} + +pub struct Pat { + pub ty: Cx::Ty, +} + +// Simple recursive case: no warning +pub struct MyTypeContextSimpleRecursive<'thir, 'tcx: 'thir> { + pub pat: Pat>, +} +impl<'thir, 'tcx: 'thir> TypeCx for MyTypeContextSimpleRecursive<'thir, 'tcx> { + type Ty = (); +} + +// Non-recursive case: we want a warning +pub struct MyTypeContextNotRecursive<'thir, 'tcx: 'thir> { + pub tcx: &'tcx (), + pub thir: &'thir (), +} +impl<'thir, 'tcx: 'thir> TypeCx for MyTypeContextNotRecursive<'thir, 'tcx> { + type Ty = (); +} + + +// Mixed-recursive case: we want a warning +pub struct MyTypeContextMixedRecursive<'thir, 'tcx: 'thir> { + pub pat: Pat>, + pub tcx: &'tcx (), + pub thir: &'thir (), +} +impl<'thir, 'tcx: 'thir> TypeCx for MyTypeContextMixedRecursive<'thir, 'tcx> { + type Ty = (); +} + +fn main() {} From f595539b8101f03cfbc6d75c0b3a9ec2ffb72679 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 9 Aug 2024 20:01:25 -0400 Subject: [PATCH 12/15] Fix dump-ice-to-disk for RUSTC_ICE=0 users --- tests/run-make/dump-ice-to-disk/rmake.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run-make/dump-ice-to-disk/rmake.rs b/tests/run-make/dump-ice-to-disk/rmake.rs index 2fb5c825064b0..1e2885fb137e5 100644 --- a/tests/run-make/dump-ice-to-disk/rmake.rs +++ b/tests/run-make/dump-ice-to-disk/rmake.rs @@ -9,7 +9,7 @@ use run_make_support::{cwd, has_extension, has_prefix, rfs, rustc, shallow_find_files}; fn main() { - rustc().input("lib.rs").arg("-Ztreat-err-as-bug=1").run_fail(); + rustc().env("RUSTC_ICE", cwd()).input("lib.rs").arg("-Ztreat-err-as-bug=1").run_fail(); let default = get_text_from_ice(".").lines().count(); clear_ice_files(); From 860c8cdeafb6cad648c5b3cfd79ea07be28b097b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 10 Aug 2024 00:54:16 +0000 Subject: [PATCH 13/15] Differentiate between methods and associated functions Accurately refer to assoc fn without receiver as assoc fn instead of methods. Add `AssocItem::descr` method to centralize where we call methods and associated functions. --- .../src/check/compare_impl_item.rs | 14 +++----------- compiler/rustc_middle/src/hir/map/mod.rs | 18 ++++++++++++------ compiler/rustc_middle/src/ty/assoc.rs | 9 +++++++++ .../async-await/in-trait/generics-mismatch.rs | 2 +- .../in-trait/generics-mismatch.stderr | 2 +- .../mismatched_ty_const_in_trait_impl.rs | 10 +++++----- .../mismatched_ty_const_in_trait_impl.stderr | 10 +++++----- tests/ui/delegation/not-supported.rs | 2 +- tests/ui/delegation/not-supported.stderr | 6 +++--- tests/ui/error-codes/E0049.stderr | 4 ++-- tests/ui/error-codes/E0195.rs | 4 ++-- tests/ui/error-codes/E0195.stderr | 6 +++--- .../gat-trait-path-missing-lifetime.rs | 2 +- .../gat-trait-path-missing-lifetime.stderr | 2 +- .../in-trait/trait-more-generics-than-impl.rs | 2 +- .../trait-more-generics-than-impl.stderr | 2 +- .../const-drop.precise.stderr | 4 ++-- .../const-drop.stock.stderr | 4 ++-- ...lt-bound-non-const-specialized-bound.stderr | 6 +++--- .../const-default-const-specialized.stderr | 4 ++-- .../specialization/default-keyword.stderr | 2 +- ...ssue-95186-specialize-on-tilde-const.stderr | 8 ++++---- ...same-trait-bound-different-constness.stderr | 8 ++++---- .../non-const-default-const-specialized.stderr | 4 ++-- .../specializing-constness-2.stderr | 4 ++-- .../ui/specialization/const_trait_impl.stderr | 10 +++++----- tests/ui/typeck/issue-36708.stderr | 2 +- 27 files changed, 79 insertions(+), 72 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 53cde14f337be..35577613800b2 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -1117,7 +1117,7 @@ fn check_region_bounds_on_impl_item<'tcx>( .dcx() .create_err(LifetimesOrBoundsMismatchOnTrait { span, - item_kind: assoc_item_kind_str(&impl_m), + item_kind: impl_m.descr(), ident: impl_m.ident(tcx), generics_span, bounds_span, @@ -1294,7 +1294,7 @@ fn compare_number_of_generics<'tcx>( ("const", trait_own_counts.consts, impl_own_counts.consts), ]; - let item_kind = assoc_item_kind_str(&impl_); + let item_kind = impl_.descr(); let mut err_occurred = None; for (kind, trait_count, impl_count) in matchings { @@ -1676,7 +1676,7 @@ fn compare_generic_param_kinds<'tcx>( param_impl_span, E0053, "{} `{}` has an incompatible generic parameter for trait `{}`", - assoc_item_kind_str(&impl_item), + impl_item.descr(), trait_item.name, &tcx.def_path_str(tcx.parent(trait_item.def_id)) ); @@ -2249,14 +2249,6 @@ fn param_env_with_gat_bounds<'tcx>( ty::ParamEnv::new(tcx.mk_clauses(&predicates), Reveal::UserFacing) } -fn assoc_item_kind_str(impl_item: &ty::AssocItem) -> &'static str { - match impl_item.kind { - ty::AssocKind::Const => "const", - ty::AssocKind::Fn => "method", - ty::AssocKind::Type => "type", - } -} - /// Manually check here that `async fn foo()` wasn't matched against `fn foo()`, /// and extract a better error if so. fn try_report_async_mismatch<'tcx>( diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 4c243e6330b91..edab6b5ebde84 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -1165,17 +1165,23 @@ fn hir_id_to_string(map: Map<'_>, id: HirId) -> String { } Node::ImplItem(ii) => { let kind = match ii.kind { - ImplItemKind::Const(..) => "assoc const", - ImplItemKind::Fn(..) => "method", - ImplItemKind::Type(_) => "assoc type", + ImplItemKind::Const(..) => "associated constant", + ImplItemKind::Fn(fn_sig, _) => match fn_sig.decl.implicit_self { + ImplicitSelfKind::None => "associated function", + _ => "method", + }, + ImplItemKind::Type(_) => "associated type", }; format!("{id} ({kind} `{}` in {})", ii.ident, path_str(ii.owner_id.def_id)) } Node::TraitItem(ti) => { let kind = match ti.kind { - TraitItemKind::Const(..) => "assoc constant", - TraitItemKind::Fn(..) => "trait method", - TraitItemKind::Type(..) => "assoc type", + TraitItemKind::Const(..) => "associated constant", + TraitItemKind::Fn(fn_sig, _) => match fn_sig.decl.implicit_self { + ImplicitSelfKind::None => "associated function", + _ => "trait method", + }, + TraitItemKind::Type(..) => "associated type", }; format!("{id} ({kind} `{}` in {})", ti.ident, path_str(ti.owner_id.def_id)) diff --git a/compiler/rustc_middle/src/ty/assoc.rs b/compiler/rustc_middle/src/ty/assoc.rs index 0ce5c613c4ce3..db56e0016a2d6 100644 --- a/compiler/rustc_middle/src/ty/assoc.rs +++ b/compiler/rustc_middle/src/ty/assoc.rs @@ -98,6 +98,15 @@ impl AssocItem { } } + pub fn descr(&self) -> &'static str { + match self.kind { + ty::AssocKind::Const => "const", + ty::AssocKind::Fn if self.fn_has_self_parameter => "method", + ty::AssocKind::Fn => "associated function", + ty::AssocKind::Type => "type", + } + } + pub fn is_impl_trait_in_trait(&self) -> bool { self.opt_rpitit_info.is_some() } diff --git a/tests/ui/async-await/in-trait/generics-mismatch.rs b/tests/ui/async-await/in-trait/generics-mismatch.rs index d3d1284982a95..a5c81a9299810 100644 --- a/tests/ui/async-await/in-trait/generics-mismatch.rs +++ b/tests/ui/async-await/in-trait/generics-mismatch.rs @@ -6,7 +6,7 @@ trait Foo { impl Foo for () { async fn foo() {} - //~^ ERROR: method `foo` has an incompatible generic parameter for trait `Foo` [E0053] + //~^ ERROR: associated function `foo` has an incompatible generic parameter for trait `Foo` [E0053] } fn main() {} diff --git a/tests/ui/async-await/in-trait/generics-mismatch.stderr b/tests/ui/async-await/in-trait/generics-mismatch.stderr index c0357dc7f3e64..cb0f95e8d098b 100644 --- a/tests/ui/async-await/in-trait/generics-mismatch.stderr +++ b/tests/ui/async-await/in-trait/generics-mismatch.stderr @@ -1,4 +1,4 @@ -error[E0053]: method `foo` has an incompatible generic parameter for trait `Foo` +error[E0053]: associated function `foo` has an incompatible generic parameter for trait `Foo` --> $DIR/generics-mismatch.rs:8:18 | LL | trait Foo { diff --git a/tests/ui/const-generics/defaults/mismatched_ty_const_in_trait_impl.rs b/tests/ui/const-generics/defaults/mismatched_ty_const_in_trait_impl.rs index 5c9323261a973..9eb33acbb24d1 100644 --- a/tests/ui/const-generics/defaults/mismatched_ty_const_in_trait_impl.rs +++ b/tests/ui/const-generics/defaults/mismatched_ty_const_in_trait_impl.rs @@ -3,7 +3,7 @@ trait Trait { } impl Trait for () { fn foo() {} - //~^ error: method `foo` has an incompatible generic parameter for trait + //~^ error: associated function `foo` has an incompatible generic parameter for trait } trait Other { @@ -11,7 +11,7 @@ trait Other { } impl Other for () { fn bar() {} - //~^ error: method `bar` has an incompatible generic parameter for trait + //~^ error: associated function `bar` has an incompatible generic parameter for trait } trait Uwu { @@ -19,7 +19,7 @@ trait Uwu { } impl Uwu for () { fn baz() {} - //~^ error: method `baz` has an incompatible generic parameter for trait + //~^ error: associated function `baz` has an incompatible generic parameter for trait } trait Aaaaaa { @@ -27,7 +27,7 @@ trait Aaaaaa { } impl Aaaaaa for () { fn bbbb() {} - //~^ error: method `bbbb` has an incompatible generic parameter for trait + //~^ error: associated function `bbbb` has an incompatible generic parameter for trait } trait Names { @@ -35,7 +35,7 @@ trait Names { } impl Names for () { fn abcd() {} - //~^ error: method `abcd` has an incompatible generic parameter for trait + //~^ error: associated function `abcd` has an incompatible generic parameter for trait } fn main() {} diff --git a/tests/ui/const-generics/defaults/mismatched_ty_const_in_trait_impl.stderr b/tests/ui/const-generics/defaults/mismatched_ty_const_in_trait_impl.stderr index 3455f2c8ea97b..7ec162e1c295c 100644 --- a/tests/ui/const-generics/defaults/mismatched_ty_const_in_trait_impl.stderr +++ b/tests/ui/const-generics/defaults/mismatched_ty_const_in_trait_impl.stderr @@ -1,4 +1,4 @@ -error[E0053]: method `foo` has an incompatible generic parameter for trait `Trait` +error[E0053]: associated function `foo` has an incompatible generic parameter for trait `Trait` --> $DIR/mismatched_ty_const_in_trait_impl.rs:5:12 | LL | trait Trait { @@ -11,7 +11,7 @@ LL | impl Trait for () { LL | fn foo() {} | ^^^^^^^^^^^^ found const parameter of type `u64` -error[E0053]: method `bar` has an incompatible generic parameter for trait `Other` +error[E0053]: associated function `bar` has an incompatible generic parameter for trait `Other` --> $DIR/mismatched_ty_const_in_trait_impl.rs:13:12 | LL | trait Other { @@ -24,7 +24,7 @@ LL | impl Other for () { LL | fn bar() {} | ^ found type parameter -error[E0053]: method `baz` has an incompatible generic parameter for trait `Uwu` +error[E0053]: associated function `baz` has an incompatible generic parameter for trait `Uwu` --> $DIR/mismatched_ty_const_in_trait_impl.rs:21:12 | LL | trait Uwu { @@ -37,7 +37,7 @@ LL | impl Uwu for () { LL | fn baz() {} | ^^^^^^^^^^^^ found const parameter of type `i32` -error[E0053]: method `bbbb` has an incompatible generic parameter for trait `Aaaaaa` +error[E0053]: associated function `bbbb` has an incompatible generic parameter for trait `Aaaaaa` --> $DIR/mismatched_ty_const_in_trait_impl.rs:29:13 | LL | trait Aaaaaa { @@ -50,7 +50,7 @@ LL | impl Aaaaaa for () { LL | fn bbbb() {} | ^ found type parameter -error[E0053]: method `abcd` has an incompatible generic parameter for trait `Names` +error[E0053]: associated function `abcd` has an incompatible generic parameter for trait `Names` --> $DIR/mismatched_ty_const_in_trait_impl.rs:37:13 | LL | trait Names { diff --git a/tests/ui/delegation/not-supported.rs b/tests/ui/delegation/not-supported.rs index 5701cc6055db7..d5ac68ecf1b8a 100644 --- a/tests/ui/delegation/not-supported.rs +++ b/tests/ui/delegation/not-supported.rs @@ -53,7 +53,7 @@ mod generics { //~| ERROR method `foo2` has 0 type parameters but its trait declaration has 1 type parameter reuse ::foo3; //~^ ERROR early bound generics are not supported for associated delegation items - //~| ERROR lifetime parameters or bounds on method `foo3` do not match the trait declaration + //~| ERROR lifetime parameters or bounds on associated function `foo3` do not match the trait declaration } struct GenericS(T); diff --git a/tests/ui/delegation/not-supported.stderr b/tests/ui/delegation/not-supported.stderr index e7ba9fc6719a0..14d6b374bd2c4 100644 --- a/tests/ui/delegation/not-supported.stderr +++ b/tests/ui/delegation/not-supported.stderr @@ -84,14 +84,14 @@ LL | fn foo3<'a: 'a>(_: &'a u32) {} LL | reuse ::foo3; | ^^^^ -error[E0195]: lifetime parameters or bounds on method `foo3` do not match the trait declaration +error[E0195]: lifetime parameters or bounds on associated function `foo3` do not match the trait declaration --> $DIR/not-supported.rs:54:29 | LL | fn foo3<'a: 'a>(_: &'a u32) {} - | -------- lifetimes in impl do not match this method in trait + | -------- lifetimes in impl do not match this associated function in trait ... LL | reuse ::foo3; - | ^^^^ lifetimes do not match method in trait + | ^^^^ lifetimes do not match associated function in trait error: delegation to a function with effect parameter is not supported yet --> $DIR/not-supported.rs:112:18 diff --git a/tests/ui/error-codes/E0049.stderr b/tests/ui/error-codes/E0049.stderr index c0cd31faa90d6..83a30aa0a7874 100644 --- a/tests/ui/error-codes/E0049.stderr +++ b/tests/ui/error-codes/E0049.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `foo` has 0 type parameters but its trait declaration has 1 type parameter +error[E0049]: associated function `foo` has 0 type parameters but its trait declaration has 1 type parameter --> $DIR/E0049.rs:8:11 | LL | fn foo(x: T) -> Self; @@ -7,7 +7,7 @@ LL | fn foo(x: T) -> Self; LL | fn foo(x: bool) -> Self { Bar } | ^ found 0 type parameters -error[E0049]: method `fuzz` has 0 type parameters but its trait declaration has 2 type parameters +error[E0049]: associated function `fuzz` has 0 type parameters but its trait declaration has 2 type parameters --> $DIR/E0049.rs:18:12 | LL | fn fuzz(x: A, y: B) -> Self; diff --git a/tests/ui/error-codes/E0195.rs b/tests/ui/error-codes/E0195.rs index f712ee42b8c5b..a7e51dff2f3fd 100644 --- a/tests/ui/error-codes/E0195.rs +++ b/tests/ui/error-codes/E0195.rs @@ -1,13 +1,13 @@ trait Trait { fn bar<'a,'b:'a>(x: &'a str, y: &'b str); - //~^ NOTE lifetimes in impl do not match this method in trait + //~^ NOTE lifetimes in impl do not match this associated function in trait } struct Foo; impl Trait for Foo { fn bar<'a,'b>(x: &'a str, y: &'b str) { //~ ERROR E0195 - //~^ NOTE lifetimes do not match method in trait + //~^ NOTE lifetimes do not match associated function in trait } } diff --git a/tests/ui/error-codes/E0195.stderr b/tests/ui/error-codes/E0195.stderr index b88064b7f90b2..9767dee9aecd1 100644 --- a/tests/ui/error-codes/E0195.stderr +++ b/tests/ui/error-codes/E0195.stderr @@ -1,11 +1,11 @@ -error[E0195]: lifetime parameters or bounds on method `bar` do not match the trait declaration +error[E0195]: lifetime parameters or bounds on associated function `bar` do not match the trait declaration --> $DIR/E0195.rs:9:11 | LL | fn bar<'a,'b:'a>(x: &'a str, y: &'b str); - | ---------- lifetimes in impl do not match this method in trait + | ---------- lifetimes in impl do not match this associated function in trait ... LL | fn bar<'a,'b>(x: &'a str, y: &'b str) { - | ^^^^^^^ lifetimes do not match method in trait + | ^^^^^^^ lifetimes do not match associated function in trait error: aborting due to 1 previous error diff --git a/tests/ui/generic-associated-types/gat-trait-path-missing-lifetime.rs b/tests/ui/generic-associated-types/gat-trait-path-missing-lifetime.rs index 285493132b64d..946cb1a62b7da 100644 --- a/tests/ui/generic-associated-types/gat-trait-path-missing-lifetime.rs +++ b/tests/ui/generic-associated-types/gat-trait-path-missing-lifetime.rs @@ -8,7 +8,7 @@ impl X for T { //~ ERROR: not all trait items implemented fn foo<'a, T1: X>(t : T1) -> T1::Y<'a> { //~^ ERROR missing generics for associated type //~^^ ERROR missing generics for associated type - //~| ERROR method `foo` has 1 type parameter but its trait declaration has 0 type parameters + //~| ERROR associated function `foo` has 1 type parameter but its trait declaration has 0 type parameters t } } diff --git a/tests/ui/generic-associated-types/gat-trait-path-missing-lifetime.stderr b/tests/ui/generic-associated-types/gat-trait-path-missing-lifetime.stderr index 6a600aee11f2f..72c2bdc72a2f4 100644 --- a/tests/ui/generic-associated-types/gat-trait-path-missing-lifetime.stderr +++ b/tests/ui/generic-associated-types/gat-trait-path-missing-lifetime.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `foo` has 1 type parameter but its trait declaration has 0 type parameters +error[E0049]: associated function `foo` has 1 type parameter but its trait declaration has 0 type parameters --> $DIR/gat-trait-path-missing-lifetime.rs:8:10 | LL | fn foo<'a>(t : Self::Y<'a>) -> Self::Y<'a> { t } diff --git a/tests/ui/impl-trait/in-trait/trait-more-generics-than-impl.rs b/tests/ui/impl-trait/in-trait/trait-more-generics-than-impl.rs index d9fac0238e16d..0477de3315a2b 100644 --- a/tests/ui/impl-trait/in-trait/trait-more-generics-than-impl.rs +++ b/tests/ui/impl-trait/in-trait/trait-more-generics-than-impl.rs @@ -6,7 +6,7 @@ trait Foo { impl Foo for S { fn bar() -> impl Sized {} - //~^ ERROR method `bar` has 0 type parameters but its trait declaration has 1 type parameter + //~^ ERROR associated function `bar` has 0 type parameters but its trait declaration has 1 type parameter } fn main() { diff --git a/tests/ui/impl-trait/in-trait/trait-more-generics-than-impl.stderr b/tests/ui/impl-trait/in-trait/trait-more-generics-than-impl.stderr index eed4c07710e06..ca67db403627e 100644 --- a/tests/ui/impl-trait/in-trait/trait-more-generics-than-impl.stderr +++ b/tests/ui/impl-trait/in-trait/trait-more-generics-than-impl.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `bar` has 0 type parameters but its trait declaration has 1 type parameter +error[E0049]: associated function `bar` has 0 type parameters but its trait declaration has 1 type parameter --> $DIR/trait-more-generics-than-impl.rs:8:11 | LL | fn bar() -> impl Sized; diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/const-drop.precise.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/const-drop.precise.stderr index 9d1ca5dbf2c03..b451555ec3ccd 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/const-drop.precise.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/const-drop.precise.stderr @@ -40,7 +40,7 @@ error: `~const` can only be applied to `#[const_trait]` traits LL | const fn a(_: T) {} | ^^^^^^^^ -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-drop.rs:54:5 | LL | #[const_trait] @@ -49,7 +49,7 @@ LL | pub trait SomeTrait { LL | fn foo(); | - expected 0 const parameters -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-drop.rs:54:5 | LL | #[const_trait] diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/const-drop.stock.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/const-drop.stock.stderr index 2f93f9a6743b8..296614f7fd076 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/const-drop.stock.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/const-drop.stock.stderr @@ -40,7 +40,7 @@ error: `~const` can only be applied to `#[const_trait]` traits LL | const fn a(_: T) {} | ^^^^^^^^ -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-drop.rs:54:5 | LL | #[const_trait] @@ -49,7 +49,7 @@ LL | pub trait SomeTrait { LL | fn foo(); | - expected 0 const parameters -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-drop.rs:54:5 | LL | #[const_trait] diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr index b4c4cf0a89059..7643697874f25 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `bar` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `bar` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-default-bound-non-const-specialized-bound.rs:16:1 | LL | #[const_trait] @@ -16,7 +16,7 @@ LL | | T: Foo, //FIXME ~ ERROR missing `~const` qualifier LL | | T: Specialize, | |__________________^ -error[E0049]: method `baz` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `baz` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-default-bound-non-const-specialized-bound.rs:36:1 | LL | #[const_trait] @@ -25,7 +25,7 @@ LL | trait Baz { LL | fn baz(); | - expected 0 const parameters -error[E0049]: method `baz` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `baz` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-default-bound-non-const-specialized-bound.rs:36:1 | LL | #[const_trait] diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.stderr index cabf201405f55..9b2ae8d739c88 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `value` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `value` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-default-const-specialized.rs:10:1 | LL | #[const_trait] @@ -7,7 +7,7 @@ LL | trait Value { LL | fn value() -> u32; | - expected 0 const parameters -error[E0049]: method `value` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `value` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const-default-const-specialized.rs:10:1 | LL | #[const_trait] diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/default-keyword.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/default-keyword.stderr index 52c8708f2c88d..18a25045f4b08 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/default-keyword.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/default-keyword.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/default-keyword.rs:7:1 | LL | #[const_trait] diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.stderr index 1aa34637ca482..ecdc7b930e609 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/issue-95186-specialize-on-tilde-const.rs:14:1 | LL | #[const_trait] @@ -7,7 +7,7 @@ LL | trait Foo { LL | fn foo(); | - expected 0 const parameters -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/issue-95186-specialize-on-tilde-const.rs:14:1 | LL | #[const_trait] @@ -18,7 +18,7 @@ LL | fn foo(); | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error[E0049]: method `bar` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `bar` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/issue-95186-specialize-on-tilde-const.rs:30:1 | LL | #[const_trait] @@ -27,7 +27,7 @@ LL | trait Bar { LL | fn bar() {} | - expected 0 const parameters -error[E0049]: method `bar` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `bar` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/issue-95186-specialize-on-tilde-const.rs:30:1 | LL | #[const_trait] diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.stderr index 0e0f391b0865e..6679bb4653788 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `bar` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `bar` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/issue-95187-same-trait-bound-different-constness.rs:18:1 | LL | #[const_trait] @@ -7,7 +7,7 @@ LL | trait Bar { LL | fn bar(); | - expected 0 const parameters -error[E0049]: method `bar` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `bar` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/issue-95187-same-trait-bound-different-constness.rs:18:1 | LL | #[const_trait] @@ -18,7 +18,7 @@ LL | fn bar(); | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error[E0049]: method `baz` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `baz` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/issue-95187-same-trait-bound-different-constness.rs:38:1 | LL | #[const_trait] @@ -27,7 +27,7 @@ LL | trait Baz { LL | fn baz(); | - expected 0 const parameters -error[E0049]: method `baz` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `baz` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/issue-95187-same-trait-bound-different-constness.rs:38:1 | LL | #[const_trait] diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr index d49beb932591c..7f363922947fe 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `value` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `value` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/non-const-default-const-specialized.rs:9:1 | LL | #[const_trait] @@ -7,7 +7,7 @@ LL | trait Value { LL | fn value() -> u32; | - expected 0 const parameters -error[E0049]: method `value` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `value` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/non-const-default-const-specialized.rs:9:1 | LL | #[const_trait] diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/specializing-constness-2.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/specializing-constness-2.stderr index d082cd6de6097..bf273f349b4bc 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/specializing-constness-2.stderr +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/specializing-constness-2.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `a` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `a` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/specializing-constness-2.rs:9:1 | LL | #[const_trait] @@ -7,7 +7,7 @@ LL | pub trait A { LL | fn a() -> u32; | - expected 0 const parameters -error[E0049]: method `a` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `a` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/specializing-constness-2.rs:9:1 | LL | #[const_trait] diff --git a/tests/ui/specialization/const_trait_impl.stderr b/tests/ui/specialization/const_trait_impl.stderr index e39138983c6e6..643f1de3e8d79 100644 --- a/tests/ui/specialization/const_trait_impl.stderr +++ b/tests/ui/specialization/const_trait_impl.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const_trait_impl.rs:6:1 | LL | #[const_trait] @@ -7,7 +7,7 @@ LL | pub unsafe trait Sup { LL | fn foo() -> u32; | - expected 0 const parameters -error[E0049]: method `foo` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `foo` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const_trait_impl.rs:6:1 | LL | #[const_trait] @@ -36,7 +36,7 @@ error: `~const` can only be applied to `#[const_trait]` traits LL | impl const A for T { | ^^^^^^^ -error[E0049]: method `a` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `a` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const_trait_impl.rs:29:1 | LL | #[const_trait] @@ -45,7 +45,7 @@ LL | pub trait A { LL | fn a() -> u32; | - expected 0 const parameters -error[E0049]: method `a` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `a` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const_trait_impl.rs:29:1 | LL | #[const_trait] @@ -56,7 +56,7 @@ LL | fn a() -> u32; | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error[E0049]: method `a` has 1 const parameter but its trait declaration has 0 const parameters +error[E0049]: associated function `a` has 1 const parameter but its trait declaration has 0 const parameters --> $DIR/const_trait_impl.rs:29:1 | LL | #[const_trait] diff --git a/tests/ui/typeck/issue-36708.stderr b/tests/ui/typeck/issue-36708.stderr index 3589796b6aac6..0aca575320f22 100644 --- a/tests/ui/typeck/issue-36708.stderr +++ b/tests/ui/typeck/issue-36708.stderr @@ -1,4 +1,4 @@ -error[E0049]: method `foo` has 1 type parameter but its trait declaration has 0 type parameters +error[E0049]: associated function `foo` has 1 type parameter but its trait declaration has 0 type parameters --> $DIR/issue-36708.rs:8:12 | LL | fn foo() {} From 6a90be31b13753b66e423dd8dc567fd301043b0f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 10 Aug 2024 15:03:24 +0200 Subject: [PATCH 14/15] Stop showing impl items for negative impls --- src/librustdoc/clean/types.rs | 4 ++++ src/librustdoc/html/format.rs | 5 ++--- src/librustdoc/html/render/mod.rs | 37 ++++++++++++++++++------------- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 4850500a1bfae..ff5c16f2b3e9a 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -2446,6 +2446,10 @@ impl Impl { .map(|did| tcx.provided_trait_methods(did).map(|meth| meth.name).collect()) .unwrap_or_default() } + + pub(crate) fn is_negative_trait_impl(&self) -> bool { + matches!(self.polarity, ty::ImplPolarity::Negative) + } } #[derive(Clone, Debug)] diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index b5ab6a35fdb9d..6357cfee141fd 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1285,9 +1285,8 @@ impl clean::Impl { f.write_str(" ")?; if let Some(ref ty) = self.trait_ { - match self.polarity { - ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => {} - ty::ImplPolarity::Negative => write!(f, "!")?, + if self.is_negative_trait_impl() { + write!(f, "!")?; } ty.print(cx).fmt(f)?; write!(f, " for ")?; diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 9074e40a53614..7ce637d3ab42d 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1780,20 +1780,23 @@ fn render_impl( let mut impl_items = Buffer::empty_from(w); let mut default_impl_items = Buffer::empty_from(w); + let impl_ = i.inner_impl(); - for trait_item in &i.inner_impl().items { - doc_impl_item( - &mut default_impl_items, - &mut impl_items, - cx, - trait_item, - if trait_.is_some() { &i.impl_item } else { parent }, - link, - render_mode, - false, - trait_, - rendering_params, - ); + if !impl_.is_negative_trait_impl() { + for trait_item in &impl_.items { + doc_impl_item( + &mut default_impl_items, + &mut impl_items, + cx, + trait_item, + if trait_.is_some() { &i.impl_item } else { parent }, + link, + render_mode, + false, + trait_, + rendering_params, + ); + } } fn render_default_items( @@ -1844,13 +1847,15 @@ fn render_impl( // We don't emit documentation for default items if they appear in the // Implementations on Foreign Types or Implementors sections. if rendering_params.show_default_items { - if let Some(t) = trait_ { + if let Some(t) = trait_ + && !impl_.is_negative_trait_impl() + { render_default_items( &mut default_impl_items, &mut impl_items, cx, t, - i.inner_impl(), + impl_, &i.impl_item, render_mode, rendering_params, @@ -1882,7 +1887,7 @@ fn render_impl( } if let Some(ref dox) = i.impl_item.opt_doc_value() { - if trait_.is_none() && i.inner_impl().items.is_empty() { + if trait_.is_none() && impl_.items.is_empty() { w.write_str( "
\
This impl block contains no items.
\ From 5baf7c21cfb872a82714d0f59a78bd37b6db5236 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 10 Aug 2024 15:03:43 +0200 Subject: [PATCH 15/15] Add regression tests for negative impls not showing their items --- tests/rustdoc/negative-impl-no-items.rs | 26 +++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/rustdoc/negative-impl-no-items.rs diff --git a/tests/rustdoc/negative-impl-no-items.rs b/tests/rustdoc/negative-impl-no-items.rs new file mode 100644 index 0000000000000..c628e54203368 --- /dev/null +++ b/tests/rustdoc/negative-impl-no-items.rs @@ -0,0 +1,26 @@ +// This test ensures that negative impls don't have items listed inside them. + +#![feature(negative_impls)] +#![crate_name = "foo"] + +pub struct Thing; + +//@ has 'foo/struct.Thing.html' +// We check the full path to ensure there is no `
` element. +//@ has - '//div[@id="trait-implementations-list"]/section[@id="impl-Iterator-for-Thing"]/h3' \ +// 'impl !Iterator for Thing' +impl !Iterator for Thing {} + +// This struct will allow us to compare both paths. +pub struct Witness; + +//@ has 'foo/struct.Witness.html' +//@ has - '//div[@id="trait-implementations-list"]/details//section[@id="impl-Iterator-for-Witness"]/h3' \ +// 'impl Iterator for Witness' +impl Iterator for Witness { + type Item = u8; + + fn next(&mut self) -> Option { + None + } +}