From c6015851f7e54d1e1e267afb315a2b4b23096d0d Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 9 Mar 2023 13:18:07 -0700 Subject: [PATCH 1/5] rustdoc: move notable trait tests into their own directory --- .../doc-notable_trait-mut_t_is_not_an_iterator.rs | 0 .../{ => notable-trait}/doc-notable_trait-mut_t_is_not_ref_t.rs | 0 .../doc-notable_trait-slice.bare_fn_matches.html | 0 tests/rustdoc/{ => notable-trait}/doc-notable_trait-slice.rs | 0 tests/rustdoc/{ => notable-trait}/doc-notable_trait.bare-fn.html | 0 tests/rustdoc/{ => notable-trait}/doc-notable_trait.rs | 0 .../{ => notable-trait}/doc-notable_trait.some-struct-new.html | 0 tests/rustdoc/{ => notable-trait}/doc-notable_trait.wrap-me.html | 0 .../doc-notable_trait_box_is_not_an_iterator.rs | 0 .../{ => notable-trait}/spotlight-from-dependency.odd.html | 0 tests/rustdoc/{ => notable-trait}/spotlight-from-dependency.rs | 0 11 files changed, 0 insertions(+), 0 deletions(-) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait-mut_t_is_not_an_iterator.rs (100%) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait-mut_t_is_not_ref_t.rs (100%) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait-slice.bare_fn_matches.html (100%) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait-slice.rs (100%) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait.bare-fn.html (100%) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait.rs (100%) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait.some-struct-new.html (100%) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait.wrap-me.html (100%) rename tests/rustdoc/{ => notable-trait}/doc-notable_trait_box_is_not_an_iterator.rs (100%) rename tests/rustdoc/{ => notable-trait}/spotlight-from-dependency.odd.html (100%) rename tests/rustdoc/{ => notable-trait}/spotlight-from-dependency.rs (100%) diff --git a/tests/rustdoc/doc-notable_trait-mut_t_is_not_an_iterator.rs b/tests/rustdoc/notable-trait/doc-notable_trait-mut_t_is_not_an_iterator.rs similarity index 100% rename from tests/rustdoc/doc-notable_trait-mut_t_is_not_an_iterator.rs rename to tests/rustdoc/notable-trait/doc-notable_trait-mut_t_is_not_an_iterator.rs diff --git a/tests/rustdoc/doc-notable_trait-mut_t_is_not_ref_t.rs b/tests/rustdoc/notable-trait/doc-notable_trait-mut_t_is_not_ref_t.rs similarity index 100% rename from tests/rustdoc/doc-notable_trait-mut_t_is_not_ref_t.rs rename to tests/rustdoc/notable-trait/doc-notable_trait-mut_t_is_not_ref_t.rs diff --git a/tests/rustdoc/doc-notable_trait-slice.bare_fn_matches.html b/tests/rustdoc/notable-trait/doc-notable_trait-slice.bare_fn_matches.html similarity index 100% rename from tests/rustdoc/doc-notable_trait-slice.bare_fn_matches.html rename to tests/rustdoc/notable-trait/doc-notable_trait-slice.bare_fn_matches.html diff --git a/tests/rustdoc/doc-notable_trait-slice.rs b/tests/rustdoc/notable-trait/doc-notable_trait-slice.rs similarity index 100% rename from tests/rustdoc/doc-notable_trait-slice.rs rename to tests/rustdoc/notable-trait/doc-notable_trait-slice.rs diff --git a/tests/rustdoc/doc-notable_trait.bare-fn.html b/tests/rustdoc/notable-trait/doc-notable_trait.bare-fn.html similarity index 100% rename from tests/rustdoc/doc-notable_trait.bare-fn.html rename to tests/rustdoc/notable-trait/doc-notable_trait.bare-fn.html diff --git a/tests/rustdoc/doc-notable_trait.rs b/tests/rustdoc/notable-trait/doc-notable_trait.rs similarity index 100% rename from tests/rustdoc/doc-notable_trait.rs rename to tests/rustdoc/notable-trait/doc-notable_trait.rs diff --git a/tests/rustdoc/doc-notable_trait.some-struct-new.html b/tests/rustdoc/notable-trait/doc-notable_trait.some-struct-new.html similarity index 100% rename from tests/rustdoc/doc-notable_trait.some-struct-new.html rename to tests/rustdoc/notable-trait/doc-notable_trait.some-struct-new.html diff --git a/tests/rustdoc/doc-notable_trait.wrap-me.html b/tests/rustdoc/notable-trait/doc-notable_trait.wrap-me.html similarity index 100% rename from tests/rustdoc/doc-notable_trait.wrap-me.html rename to tests/rustdoc/notable-trait/doc-notable_trait.wrap-me.html diff --git a/tests/rustdoc/doc-notable_trait_box_is_not_an_iterator.rs b/tests/rustdoc/notable-trait/doc-notable_trait_box_is_not_an_iterator.rs similarity index 100% rename from tests/rustdoc/doc-notable_trait_box_is_not_an_iterator.rs rename to tests/rustdoc/notable-trait/doc-notable_trait_box_is_not_an_iterator.rs diff --git a/tests/rustdoc/spotlight-from-dependency.odd.html b/tests/rustdoc/notable-trait/spotlight-from-dependency.odd.html similarity index 100% rename from tests/rustdoc/spotlight-from-dependency.odd.html rename to tests/rustdoc/notable-trait/spotlight-from-dependency.odd.html diff --git a/tests/rustdoc/spotlight-from-dependency.rs b/tests/rustdoc/notable-trait/spotlight-from-dependency.rs similarity index 100% rename from tests/rustdoc/spotlight-from-dependency.rs rename to tests/rustdoc/notable-trait/spotlight-from-dependency.rs From ee6b228b6a8b874ca8b6e88cd58d4618cf20fb20 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 9 Mar 2023 13:41:13 -0700 Subject: [PATCH 2/5] rustdoc: handle generics better when matching notable traits This commit makes the `clean::Type::is_same` non-commutative, so that a generic `impl` matches a concrete return, but a generic return does not match a concrete `impl`. It makes slice and vector Write for `u8` not match on every generic return value. --- src/librustdoc/clean/types.rs | 46 +++++++++++++++++-- src/librustdoc/clean/types/tests.rs | 11 +++++ src/librustdoc/html/render/mod.rs | 4 +- .../notable-trait/doc-notable_trait-slice.rs | 6 +++ .../notable-trait/notable-trait-generics.rs | 35 ++++++++++++++ 5 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 tests/rustdoc/notable-trait/notable-trait-generics.rs diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 6d8380c5fcc18..5217de7aa8a4f 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1471,11 +1471,37 @@ impl Type { result } - /// Check if two types are "potentially the same". + pub(crate) fn is_borrowed_ref(&self) -> bool { + matches!(self, Type::BorrowedRef { .. }) + } + + /// Check if two types are "the same" for documentation purposes. + /// /// This is different from `Eq`, because it knows that things like /// `Placeholder` are possible matches for everything. + /// + /// This relation is not commutative when generics are involved: + /// + /// ```ignore(private) + /// # // see types/tests.rs:is_same_generic for the real test + /// use rustdoc::format::cache::Cache; + /// use rustdoc::clean::types::{Type, PrimitiveType}; + /// let cache = Cache::new(false); + /// let generic = Type::Generic(rustc_span::symbol::sym::Any); + /// let unit = Type::Primitive(PrimitiveType::Unit); + /// assert!(!generic.is_same(&unit, &cache)); + /// assert!(unit.is_same(&generic, &cache)); + /// ``` + /// + /// An owned type is also the same as its borrowed variants (this is commutative), + /// but `&T` is not the same as `&mut T`. pub(crate) fn is_same(&self, other: &Self, cache: &Cache) -> bool { - match (self, other) { + let (self_cleared, other_cleared) = if !self.is_borrowed_ref() || !other.is_borrowed_ref() { + (self.without_borrowed_ref(), other.without_borrowed_ref()) + } else { + (self, other) + }; + match (self_cleared, other_cleared) { // Recursive cases. (Type::Tuple(a), Type::Tuple(b)) => { a.len() == b.len() && a.iter().zip(b).all(|(a, b)| a.is_same(b, cache)) @@ -1489,9 +1515,21 @@ impl Type { Type::BorrowedRef { mutability, type_, .. }, Type::BorrowedRef { mutability: b_mutability, type_: b_type_, .. }, ) => mutability == b_mutability && type_.is_same(b_type_, cache), - // Placeholders and generics are equal to all other types. + // Placeholders are equal to all other types. (Type::Infer, _) | (_, Type::Infer) => true, - (Type::Generic(_), _) | (_, Type::Generic(_)) => true, + // Generics match everything on the right, but not on the left. + (_, Type::Generic(_)) => true, + (Type::Generic(_), _) => false, + // Paths account for both the path itself and its generics. + (Type::Path { path: a }, Type::Path { path: b }) => { + a.def_id() == b.def_id() + && a.generics() + .zip(b.generics()) + .map(|(ag, bg)| { + ag.iter().zip(bg.iter()).all(|(at, bt)| at.is_same(bt, cache)) + }) + .unwrap_or(true) + } // Other cases, such as primitives, just use recursion. (a, b) => a .def_id(cache) diff --git a/src/librustdoc/clean/types/tests.rs b/src/librustdoc/clean/types/tests.rs index 20627c2cfc164..7df87a9804aa2 100644 --- a/src/librustdoc/clean/types/tests.rs +++ b/src/librustdoc/clean/types/tests.rs @@ -69,3 +69,14 @@ fn should_not_trim() { run_test("\t line1 \n\t line2", "line1 \nline2"); run_test(" \tline1 \n \tline2", "line1 \nline2"); } + +#[test] +fn is_same_generic() { + use crate::clean::types::{PrimitiveType, Type}; + use crate::formats::cache::Cache; + let cache = Cache::new(false); + let generic = Type::Generic(rustc_span::symbol::sym::Any); + let unit = Type::Primitive(PrimitiveType::Unit); + assert!(!generic.is_same(&unit, &cache)); + assert!(unit.is_same(&generic, &cache)); +} diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index e6a040d02e565..f8c26dc4706b1 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1291,7 +1291,7 @@ pub(crate) fn notable_traits_button(ty: &clean::Type, cx: &mut Context<'_>) -> O if let Some(impls) = cx.cache().impls.get(&did) { for i in impls { let impl_ = i.inner_impl(); - if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) { + if !ty.is_same(&impl_.for_, cx.cache()) { // Two different types might have the same did, // without actually being the same. continue; @@ -1327,7 +1327,7 @@ fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> (String, String) { for i in impls { let impl_ = i.inner_impl(); - if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) { + if !ty.is_same(&impl_.for_, cx.cache()) { // Two different types might have the same did, // without actually being the same. continue; diff --git a/tests/rustdoc/notable-trait/doc-notable_trait-slice.rs b/tests/rustdoc/notable-trait/doc-notable_trait-slice.rs index 2411da8cd4549..ef206710b4b08 100644 --- a/tests/rustdoc/notable-trait/doc-notable_trait-slice.rs +++ b/tests/rustdoc/notable-trait/doc-notable_trait-slice.rs @@ -18,3 +18,9 @@ pub fn bare_fn_matches() -> &'static [SomeStruct] { pub fn bare_fn_no_matches() -> &'static [OtherStruct] { &[] } + +// @has doc_notable_trait_slice/fn.bare_fn_mut_no_matches.html +// @count - '//script[@id="notable-traits-data"]' 0 +pub fn bare_fn_mut_no_matches() -> &'static mut [SomeStruct] { + &mut [] +} diff --git a/tests/rustdoc/notable-trait/notable-trait-generics.rs b/tests/rustdoc/notable-trait/notable-trait-generics.rs new file mode 100644 index 0000000000000..7bfe9d43ea986 --- /dev/null +++ b/tests/rustdoc/notable-trait/notable-trait-generics.rs @@ -0,0 +1,35 @@ +#![feature(doc_notable_trait)] + +// Notable traits SHOULD be shown when the `impl` has a generic type and the +// return type has a concrete type. +pub mod generic_return { + pub struct Wrapper(T); + + #[doc(notable_trait)] + pub trait NotableTrait {} + + impl NotableTrait for Wrapper {} + + // @has notable_trait_generics/generic_return/fn.returning.html + // @!has - '//a[@class="tooltip"]/@data-notable-ty' 'Wrapper' + pub fn returning() -> Wrapper { + loop {} + } +} + +// Notable traits SHOULD NOT be shown when the `impl` has a concrete type and +// the return type has a generic type. +pub mod generic_impl { + pub struct Wrapper(T); + + #[doc(notable_trait)] + pub trait NotableTrait {} + + impl NotableTrait for Wrapper {} + + // @has notable_trait_generics/generic_impl/fn.returning.html + // @has - '//a[@class="tooltip"]/@data-notable-ty' 'Wrapper' + pub fn returning() -> Wrapper { + loop {} + } +} From 86179c4549e74acf22fc4da02b64f0bb2ce1f4aa Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sun, 12 Mar 2023 17:45:15 -0700 Subject: [PATCH 3/5] rustdoc: rename `Type::is_same` to `is_doc_subtype_of` --- src/librustdoc/clean/types.rs | 14 +++++++------- src/librustdoc/clean/types/tests.rs | 4 ++-- src/librustdoc/html/render/mod.rs | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 5217de7aa8a4f..1b8478ee21c68 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1495,7 +1495,7 @@ impl Type { /// /// An owned type is also the same as its borrowed variants (this is commutative), /// but `&T` is not the same as `&mut T`. - pub(crate) fn is_same(&self, other: &Self, cache: &Cache) -> bool { + pub(crate) fn is_doc_subtype_of(&self, other: &Self, cache: &Cache) -> bool { let (self_cleared, other_cleared) = if !self.is_borrowed_ref() || !other.is_borrowed_ref() { (self.without_borrowed_ref(), other.without_borrowed_ref()) } else { @@ -1504,17 +1504,17 @@ impl Type { match (self_cleared, other_cleared) { // Recursive cases. (Type::Tuple(a), Type::Tuple(b)) => { - a.len() == b.len() && a.iter().zip(b).all(|(a, b)| a.is_same(b, cache)) + a.len() == b.len() && a.iter().zip(b).all(|(a, b)| a.is_doc_subtype_of(b, cache)) } - (Type::Slice(a), Type::Slice(b)) => a.is_same(b, cache), - (Type::Array(a, al), Type::Array(b, bl)) => al == bl && a.is_same(b, cache), + (Type::Slice(a), Type::Slice(b)) => a.is_doc_subtype_of(b, cache), + (Type::Array(a, al), Type::Array(b, bl)) => al == bl && a.is_doc_subtype_of(b, cache), (Type::RawPointer(mutability, type_), Type::RawPointer(b_mutability, b_type_)) => { - mutability == b_mutability && type_.is_same(b_type_, cache) + mutability == b_mutability && type_.is_doc_subtype_of(b_type_, cache) } ( Type::BorrowedRef { mutability, type_, .. }, Type::BorrowedRef { mutability: b_mutability, type_: b_type_, .. }, - ) => mutability == b_mutability && type_.is_same(b_type_, cache), + ) => mutability == b_mutability && type_.is_doc_subtype_of(b_type_, cache), // Placeholders are equal to all other types. (Type::Infer, _) | (_, Type::Infer) => true, // Generics match everything on the right, but not on the left. @@ -1526,7 +1526,7 @@ impl Type { && a.generics() .zip(b.generics()) .map(|(ag, bg)| { - ag.iter().zip(bg.iter()).all(|(at, bt)| at.is_same(bt, cache)) + ag.iter().zip(bg.iter()).all(|(at, bt)| at.is_doc_subtype_of(bt, cache)) }) .unwrap_or(true) } diff --git a/src/librustdoc/clean/types/tests.rs b/src/librustdoc/clean/types/tests.rs index 7df87a9804aa2..afbee3e5f78ca 100644 --- a/src/librustdoc/clean/types/tests.rs +++ b/src/librustdoc/clean/types/tests.rs @@ -77,6 +77,6 @@ fn is_same_generic() { let cache = Cache::new(false); let generic = Type::Generic(rustc_span::symbol::sym::Any); let unit = Type::Primitive(PrimitiveType::Unit); - assert!(!generic.is_same(&unit, &cache)); - assert!(unit.is_same(&generic, &cache)); + assert!(!generic.is_doc_subtype_of(&unit, &cache)); + assert!(unit.is_doc_subtype_of(&generic, &cache)); } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index f8c26dc4706b1..832c4e7cfe77b 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1291,7 +1291,7 @@ pub(crate) fn notable_traits_button(ty: &clean::Type, cx: &mut Context<'_>) -> O if let Some(impls) = cx.cache().impls.get(&did) { for i in impls { let impl_ = i.inner_impl(); - if !ty.is_same(&impl_.for_, cx.cache()) { + if !ty.is_doc_subtype_of(&impl_.for_, cx.cache()) { // Two different types might have the same did, // without actually being the same. continue; @@ -1327,7 +1327,7 @@ fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> (String, String) { for i in impls { let impl_ = i.inner_impl(); - if !ty.is_same(&impl_.for_, cx.cache()) { + if !ty.is_doc_subtype_of(&impl_.for_, cx.cache()) { // Two different types might have the same did, // without actually being the same. continue; From bfb66eb4bb4ada8a0b4d48d73a93e8f5df0e54be Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 13 Mar 2023 23:03:53 -0700 Subject: [PATCH 4/5] rustdoc: fix comments in test --- tests/rustdoc/notable-trait/notable-trait-generics.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/rustdoc/notable-trait/notable-trait-generics.rs b/tests/rustdoc/notable-trait/notable-trait-generics.rs index 7bfe9d43ea986..611902abad65b 100644 --- a/tests/rustdoc/notable-trait/notable-trait-generics.rs +++ b/tests/rustdoc/notable-trait/notable-trait-generics.rs @@ -1,7 +1,7 @@ #![feature(doc_notable_trait)] -// Notable traits SHOULD be shown when the `impl` has a generic type and the -// return type has a concrete type. +// Notable traits SHOULD NOT be shown when the `impl` has a concrete type and +// the return type has a generic type. pub mod generic_return { pub struct Wrapper(T); @@ -17,8 +17,8 @@ pub mod generic_return { } } -// Notable traits SHOULD NOT be shown when the `impl` has a concrete type and -// the return type has a generic type. +// Notable traits SHOULD be shown when the `impl` has a generic type and the +// return type has a concrete type. pub mod generic_impl { pub struct Wrapper(T); From 7f76084933b69ae856bc872c3eecdc2378a21f4b Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 17 Mar 2023 10:46:38 -0700 Subject: [PATCH 5/5] Add clarifying comments --- src/librustdoc/clean/types.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 1b8478ee21c68..7dbb3f76a0a83 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1496,6 +1496,8 @@ impl Type { /// An owned type is also the same as its borrowed variants (this is commutative), /// but `&T` is not the same as `&mut T`. pub(crate) fn is_doc_subtype_of(&self, other: &Self, cache: &Cache) -> bool { + // Strip the references so that it can compare the actual types, unless both are references. + // If both are references, leave them alone and compare the mutabilities later. let (self_cleared, other_cleared) = if !self.is_borrowed_ref() || !other.is_borrowed_ref() { (self.without_borrowed_ref(), other.without_borrowed_ref()) } else { @@ -1518,6 +1520,7 @@ impl Type { // Placeholders are equal to all other types. (Type::Infer, _) | (_, Type::Infer) => true, // Generics match everything on the right, but not on the left. + // If both sides are generic, this returns true. (_, Type::Generic(_)) => true, (Type::Generic(_), _) => false, // Paths account for both the path itself and its generics.