From 249d686c7005ebb4d05234e9db6d29c2923296e2 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 31 Jul 2024 15:25:13 -0700 Subject: [PATCH 1/6] rustdoc: Rename `SelfTy` to `ReceiverTy` `SelfTy` makes it sound like it is literally the `Self` type, whereas in fact it may be `&Self` or other types. Plus, I want to use the name `SelfTy` for a new variant of `clean::Type`. Having both causes resolution conflicts or at least confusion. --- src/librustdoc/clean/types.rs | 10 +++++----- src/librustdoc/html/format.rs | 2 +- src/librustdoc/html/render/mod.rs | 14 +++++++------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 7d5a16bc7e3..32e795e565e 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -34,7 +34,7 @@ use {rustc_ast as ast, rustc_hir as hir}; pub(crate) use self::ItemKind::*; -pub(crate) use self::SelfTy::*; +pub(crate) use self::ReceiverTy::*; pub(crate) use self::Type::{ Array, BareFunction, BorrowedRef, DynTrait, Generic, ImplTrait, Infer, Primitive, QPath, RawPointer, Slice, Tuple, @@ -1384,8 +1384,8 @@ pub(crate) struct FnDecl { } impl FnDecl { - pub(crate) fn self_type(&self) -> Option { - self.inputs.values.get(0).and_then(|v| v.to_self()) + pub(crate) fn receiver_type(&self) -> Option { + self.inputs.values.get(0).and_then(|v| v.to_receiver()) } } @@ -1404,14 +1404,14 @@ pub(crate) struct Argument { } #[derive(Clone, PartialEq, Debug)] -pub(crate) enum SelfTy { +pub(crate) enum ReceiverTy { SelfValue, SelfBorrowed(Option, Mutability), SelfExplicit(Type), } impl Argument { - pub(crate) fn to_self(&self) -> Option { + pub(crate) fn to_receiver(&self) -> Option { if self.name != kw::SelfLower { return None; } diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index bb5ac303ffd..6476c8bd7c0 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1452,7 +1452,7 @@ fn inner_full_print( let last_input_index = self.inputs.values.len().checked_sub(1); for (i, input) in self.inputs.values.iter().enumerate() { - if let Some(selfty) = input.to_self() { + if let Some(selfty) = input.to_receiver() { match selfty { clean::SelfValue => { write!(f, "self")?; diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 9617f0d9a1f..b7bcda2a604 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -58,7 +58,7 @@ pub(crate) use self::context::*; pub(crate) use self::span_map::{collect_spans_and_sources, LinkFromSrc}; -use crate::clean::{self, ItemId, RenderedLink, SelfTy}; +use crate::clean::{self, ItemId, ReceiverTy, RenderedLink}; use crate::error::Error; use crate::formats::cache::Cache; use crate::formats::item_type::ItemType; @@ -1372,21 +1372,21 @@ fn render_deref_methods( fn should_render_item(item: &clean::Item, deref_mut_: bool, tcx: TyCtxt<'_>) -> bool { let self_type_opt = match *item.kind { - clean::MethodItem(ref method, _) => method.decl.self_type(), - clean::TyMethodItem(ref method) => method.decl.self_type(), + clean::MethodItem(ref method, _) => method.decl.receiver_type(), + clean::TyMethodItem(ref method) => method.decl.receiver_type(), _ => None, }; if let Some(self_ty) = self_type_opt { let (by_mut_ref, by_box, by_value) = match self_ty { - SelfTy::SelfBorrowed(_, mutability) - | SelfTy::SelfExplicit(clean::BorrowedRef { mutability, .. }) => { + ReceiverTy::SelfBorrowed(_, mutability) + | ReceiverTy::SelfExplicit(clean::BorrowedRef { mutability, .. }) => { (mutability == Mutability::Mut, false, false) } - SelfTy::SelfExplicit(clean::Type::Path { path }) => { + ReceiverTy::SelfExplicit(clean::Type::Path { path }) => { (false, Some(path.def_id()) == tcx.lang_items().owned_box(), false) } - SelfTy::SelfValue => (false, false, true), + ReceiverTy::SelfValue => (false, false, true), _ => (false, false, false), }; From 664b3ffbe932d93677ac7dff52252e02aee60ef9 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 31 Jul 2024 16:45:05 -0700 Subject: [PATCH 2/6] rustdoc: Create `SelfTy` to replace `Generic(kw::SelfUpper)` Rustdoc often has to special-case `Self` because it is, well, a special type of generic parameter (although it also behaves as an alias in concrete impls). Instead of spreading this special-casing throughout the code base, create a new variant of the `clean::Type` enum that is for `Self` types. This is a refactoring that has almost no impact on rustdoc's behavior, except that `&Self`, `(Self,)`, `&[Self]`, and other similar occurrences of `Self` no longer link to the wrapping type (reference primitive, tuple primitive, etc.) as regular generics do. I felt this made more sense since users would expect `Self` to link to the containing trait or aliased type (though those are usually expanded), not the primitive that is wrapping it. For an example of the change, see the docs for `std::alloc::Allocator::by_ref`. --- src/librustdoc/clean/inline.rs | 16 +++++----------- src/librustdoc/clean/mod.rs | 11 ++++++----- src/librustdoc/clean/simplify.rs | 1 - src/librustdoc/clean/types.rs | 10 +++++++--- src/librustdoc/clean/utils.rs | 2 +- src/librustdoc/html/format.rs | 1 + src/librustdoc/html/render/search_index.rs | 15 ++++++++++++--- src/librustdoc/json/conversions.rs | 4 +++- 8 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index d92bc845664..f8953f0ebcf 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -12,7 +12,7 @@ use rustc_middle::ty::{self, TyCtxt}; use rustc_span::def_id::LOCAL_CRATE; use rustc_span::hygiene::MacroKind; -use rustc_span::symbol::{kw, sym, Symbol}; +use rustc_span::symbol::{sym, Symbol}; use thin_vec::{thin_vec, ThinVec}; use {rustc_ast as ast, rustc_hir as hir}; @@ -792,11 +792,7 @@ fn build_macro( fn filter_non_trait_generics(trait_did: DefId, mut g: clean::Generics) -> clean::Generics { for pred in &mut g.where_predicates { match *pred { - clean::WherePredicate::BoundPredicate { - ty: clean::Generic(ref s), - ref mut bounds, - .. - } if *s == kw::SelfUpper => { + clean::WherePredicate::BoundPredicate { ty: clean::SelfTy, ref mut bounds, .. } => { bounds.retain(|bound| match bound { clean::GenericBound::TraitBound(clean::PolyTrait { trait_, .. }, _) => { trait_.def_id() != trait_did @@ -812,13 +808,13 @@ fn filter_non_trait_generics(trait_did: DefId, mut g: clean::Generics) -> clean: clean::WherePredicate::BoundPredicate { ty: clean::QPath(box clean::QPathData { - self_type: clean::Generic(ref s), + self_type: clean::Generic(_), trait_: Some(trait_), .. }), bounds, .. - } => !(bounds.is_empty() || *s == kw::SelfUpper && trait_.def_id() == trait_did), + } => !bounds.is_empty() && trait_.def_id() != trait_did, _ => true, }); g @@ -832,9 +828,7 @@ fn separate_supertrait_bounds( ) -> (clean::Generics, Vec) { let mut ty_bounds = Vec::new(); g.where_predicates.retain(|pred| match *pred { - clean::WherePredicate::BoundPredicate { ty: clean::Generic(ref s), ref bounds, .. } - if *s == kw::SelfUpper => - { + clean::WherePredicate::BoundPredicate { ty: clean::SelfTy, ref bounds, .. } => { ty_bounds.extend(bounds.iter().cloned()); false } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 324b633e8ea..cffadc7c10a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1351,11 +1351,11 @@ pub(crate) fn clean_middle_assoc_item<'tcx>( let self_arg_ty = tcx.fn_sig(assoc_item.def_id).instantiate_identity().input(0).skip_binder(); if self_arg_ty == self_ty { - item.decl.inputs.values[0].type_ = Generic(kw::SelfUpper); + item.decl.inputs.values[0].type_ = SelfTy; } else if let ty::Ref(_, ty, _) = *self_arg_ty.kind() { if ty == self_ty { match item.decl.inputs.values[0].type_ { - BorrowedRef { ref mut type_, .. } => **type_ = Generic(kw::SelfUpper), + BorrowedRef { ref mut type_, .. } => **type_ = SelfTy, _ => unreachable!(), } } @@ -1439,9 +1439,8 @@ fn param_eq_arg(param: &GenericParamDef, arg: &GenericArg) -> bool { if trait_.def_id() != assoc_item.container_id(tcx) { return true; } - match *self_type { - Generic(ref s) if *s == kw::SelfUpper => {} - _ => return true, + if *self_type != SelfTy { + return true; } match &assoc.args { GenericArgs::AngleBracketed { args, constraints } => { @@ -2228,6 +2227,8 @@ pub(crate) fn clean_middle_ty<'tcx>( ty::Param(ref p) => { if let Some(bounds) = cx.impl_trait_bounds.remove(&p.index.into()) { ImplTrait(bounds) + } else if p.name == kw::SelfUpper { + SelfTy } else { Generic(p.name) } diff --git a/src/librustdoc/clean/simplify.rs b/src/librustdoc/clean/simplify.rs index 739f6eb8cc8..1d81ae3eb8b 100644 --- a/src/librustdoc/clean/simplify.rs +++ b/src/librustdoc/clean/simplify.rs @@ -145,7 +145,6 @@ pub(crate) fn sized_bounds(cx: &mut DocContext<'_>, generics: &mut clean::Generi // should be handled when cleaning associated types. generics.where_predicates.retain(|pred| { if let WP::BoundPredicate { ty: clean::Generic(param), bounds, .. } = pred - && *param != rustc_span::symbol::kw::SelfUpper && bounds.iter().any(|b| b.is_sized_bound(cx)) { sized_params.insert(*param); diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 32e795e565e..82f1312364a 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -37,7 +37,7 @@ pub(crate) use self::ReceiverTy::*; pub(crate) use self::Type::{ Array, BareFunction, BorrowedRef, DynTrait, Generic, ImplTrait, Infer, Primitive, QPath, - RawPointer, Slice, Tuple, + RawPointer, SelfTy, Slice, Tuple, }; use crate::clean::cfg::Cfg; use crate::clean::clean_middle_path; @@ -1477,6 +1477,8 @@ pub(crate) enum Type { DynTrait(Vec, Option), /// A type parameter. Generic(Symbol), + /// The `Self` type. + SelfTy, /// A primitive (aka, builtin) type. Primitive(PrimitiveType), /// A function pointer: `extern "ABI" fn(...) -> ...` @@ -1571,6 +1573,8 @@ pub(crate) fn is_doc_subtype_of(&self, other: &Self, cache: &Cache) -> bool { // If both sides are generic, this returns true. (_, Type::Generic(_)) => true, (Type::Generic(_), _) => false, + // `Self` only matches itself. + (Type::SelfTy, Type::SelfTy) => true, // Paths account for both the path itself and its generics. (Type::Path { path: a }, Type::Path { path: b }) => { a.def_id() == b.def_id() @@ -1642,7 +1646,7 @@ pub(crate) fn is_assoc_ty(&self) -> bool { pub(crate) fn is_self_type(&self) -> bool { match *self { - Generic(name) => name == kw::SelfUpper, + SelfTy => true, _ => false, } } @@ -1700,7 +1704,7 @@ pub(crate) fn def_id(&self, cache: &Cache) -> Option { Type::Pat(..) => PrimitiveType::Pat, RawPointer(..) => PrimitiveType::RawPointer, QPath(box QPathData { ref self_type, .. }) => return self_type.def_id(cache), - Generic(_) | Infer | ImplTrait(_) => return None, + Generic(_) | SelfTy | Infer | ImplTrait(_) => return None, }; Primitive(t).def_id(cache) } diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 2dd3041ab4c..68266f3506a 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -468,7 +468,7 @@ pub(crate) fn resolve_type(cx: &mut DocContext<'_>, path: Path) -> Type { match path.res { Res::PrimTy(p) => Primitive(PrimitiveType::from(p)), Res::SelfTyParam { .. } | Res::SelfTyAlias { .. } if path.segments.len() == 1 => { - Generic(kw::SelfUpper) + Type::SelfTy } Res::Def(DefKind::TyParam, _) if path.segments.len() == 1 => Generic(path.segments[0].name), _ => { diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 6476c8bd7c0..0fe535ade9b 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1006,6 +1006,7 @@ fn fmt_type<'cx>( match *t { clean::Generic(name) => f.write_str(name.as_str()), + clean::SelfTy => f.write_str("Self"), clean::Type::Path { ref path } => { // Paths like `T::Output` and `Self::Output` should be rendered with all segments. let did = path.def_id(); diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index b3f4d82e054..fc3d6c99b80 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -797,7 +797,11 @@ fn get_index_type_id( } } // Not supported yet - clean::Type::Pat(..) | clean::Generic(_) | clean::ImplTrait(_) | clean::Infer => None, + clean::Type::Pat(..) + | clean::Generic(_) + | clean::SelfTy + | clean::ImplTrait(_) + | clean::Infer => None, } } @@ -848,13 +852,18 @@ fn simplify_fn_type<'tcx, 'a>( (false, arg) }; + let as_arg_s = |t: &Type| match *t { + Type::Generic(arg_s) => Some(arg_s), + Type::SelfTy => Some(kw::SelfUpper), + _ => None, + }; // If this argument is a type parameter and not a trait bound or a type, we need to look // for its bounds. - if let Type::Generic(arg_s) = *arg { + if let Some(arg_s) = as_arg_s(arg) { // First we check if the bounds are in a `where` predicate... let mut type_bounds = Vec::new(); for where_pred in generics.where_predicates.iter().filter(|g| match g { - WherePredicate::BoundPredicate { ty: Type::Generic(ty_s), .. } => *ty_s == arg_s, + WherePredicate::BoundPredicate { ty, .. } => *ty == *arg, _ => false, }) { let bounds = where_pred.get_bounds().unwrap_or_else(|| &[]); diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index b56244f2d3b..b97d710c007 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -578,7 +578,7 @@ impl FromWithTcx for Type { fn from_tcx(ty: clean::Type, tcx: TyCtxt<'_>) -> Self { use clean::Type::{ Array, BareFunction, BorrowedRef, Generic, ImplTrait, Infer, Primitive, QPath, - RawPointer, Slice, Tuple, + RawPointer, SelfTy, Slice, Tuple, }; match ty { @@ -588,6 +588,8 @@ fn from_tcx(ty: clean::Type, tcx: TyCtxt<'_>) -> Self { traits: bounds.into_tcx(tcx), }), Generic(s) => Type::Generic(s.to_string()), + // FIXME: add dedicated variant to json Type? + SelfTy => Type::Generic("Self".to_owned()), Primitive(p) => Type::Primitive(p.as_sym().to_string()), BareFunction(f) => Type::FunctionPointer(Box::new((*f).into_tcx(tcx))), Tuple(t) => Type::Tuple(t.into_tcx(tcx)), From 4e348fa803723a38553138d9795cb2c6b7fb964a Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 31 Jul 2024 17:06:32 -0700 Subject: [PATCH 3/6] rustdoc: Stop treating `Self` as a generic in search index We already have special-cased code to handle inlining `Self` as the type or trait it refers to, and this was just causing glitches like the search `A -> B` yielding blanket `Into` impls. --- src/librustdoc/html/render/search_index.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index fc3d6c99b80..e4d948b8c9c 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -852,14 +852,9 @@ fn simplify_fn_type<'tcx, 'a>( (false, arg) }; - let as_arg_s = |t: &Type| match *t { - Type::Generic(arg_s) => Some(arg_s), - Type::SelfTy => Some(kw::SelfUpper), - _ => None, - }; // If this argument is a type parameter and not a trait bound or a type, we need to look // for its bounds. - if let Some(arg_s) = as_arg_s(arg) { + if let Type::Generic(arg_s) = *arg { // First we check if the bounds are in a `where` predicate... let mut type_bounds = Vec::new(); for where_pred in generics.where_predicates.iter().filter(|g| match g { From e452e3def69deca6a671b6067b1faf14b4cf83f2 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 31 Jul 2024 17:17:14 -0700 Subject: [PATCH 4/6] Use `match` instead of sequence of `if let`s This is much more readable and idiomatic, and also may help performance since `match`es usually use switches while `if`s may not. I also fixed an incorrect comment. --- src/librustdoc/html/render/search_index.rs | 489 +++++++++++---------- 1 file changed, 249 insertions(+), 240 deletions(-) diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index e4d948b8c9c..8a2f31f7413 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -854,15 +854,70 @@ fn simplify_fn_type<'tcx, 'a>( // If this argument is a type parameter and not a trait bound or a type, we need to look // for its bounds. - if let Type::Generic(arg_s) = *arg { - // First we check if the bounds are in a `where` predicate... - let mut type_bounds = Vec::new(); - for where_pred in generics.where_predicates.iter().filter(|g| match g { - WherePredicate::BoundPredicate { ty, .. } => *ty == *arg, - _ => false, - }) { - let bounds = where_pred.get_bounds().unwrap_or_else(|| &[]); - for bound in bounds.iter() { + match *arg { + Type::Generic(arg_s) => { + // First we check if the bounds are in a `where` predicate... + let mut type_bounds = Vec::new(); + for where_pred in generics.where_predicates.iter().filter(|g| match g { + WherePredicate::BoundPredicate { ty, .. } => *ty == *arg, + _ => false, + }) { + let bounds = where_pred.get_bounds().unwrap_or_else(|| &[]); + for bound in bounds.iter() { + if let Some(path) = bound.get_trait_path() { + let ty = Type::Path { path }; + simplify_fn_type( + self_, + generics, + &ty, + tcx, + recurse + 1, + &mut type_bounds, + rgen, + is_return, + cache, + ); + } + } + } + // Otherwise we check if the trait bounds are "inlined" like `T: Option`... + if let Some(bound) = generics.params.iter().find(|g| g.is_type() && g.name == arg_s) { + for bound in bound.get_bounds().unwrap_or(&[]) { + if let Some(path) = bound.get_trait_path() { + let ty = Type::Path { path }; + simplify_fn_type( + self_, + generics, + &ty, + tcx, + recurse + 1, + &mut type_bounds, + rgen, + is_return, + cache, + ); + } + } + } + if let Some((idx, _)) = rgen.get(&SimplifiedParam::Symbol(arg_s)) { + res.push(RenderType { + id: Some(RenderTypeId::Index(*idx)), + generics: None, + bindings: None, + }); + } else { + let idx = -isize::try_from(rgen.len() + 1).unwrap(); + rgen.insert(SimplifiedParam::Symbol(arg_s), (idx, type_bounds)); + res.push(RenderType { + id: Some(RenderTypeId::Index(idx)), + generics: None, + bindings: None, + }); + } + } + Type::ImplTrait(ref bounds) => { + let mut type_bounds = Vec::new(); + for bound in bounds { if let Some(path) = bound.get_trait_path() { let ty = Type::Path { path }; simplify_fn_type( @@ -878,103 +933,22 @@ fn simplify_fn_type<'tcx, 'a>( ); } } - } - // Otherwise we check if the trait bounds are "inlined" like `T: Option`... - if let Some(bound) = generics.params.iter().find(|g| g.is_type() && g.name == arg_s) { - for bound in bound.get_bounds().unwrap_or(&[]) { - if let Some(path) = bound.get_trait_path() { - let ty = Type::Path { path }; - simplify_fn_type( - self_, - generics, - &ty, - tcx, - recurse + 1, - &mut type_bounds, - rgen, - is_return, - cache, - ); - } + if is_return && !type_bounds.is_empty() { + // In return position, `impl Trait` is a unique thing. + res.push(RenderType { id: None, generics: Some(type_bounds), bindings: None }); + } else { + // In parameter position, `impl Trait` is the same as an unnamed generic parameter. + let idx = -isize::try_from(rgen.len() + 1).unwrap(); + rgen.insert(SimplifiedParam::Anonymous(idx), (idx, type_bounds)); + res.push(RenderType { + id: Some(RenderTypeId::Index(idx)), + generics: None, + bindings: None, + }); } } - if let Some((idx, _)) = rgen.get(&SimplifiedParam::Symbol(arg_s)) { - res.push(RenderType { - id: Some(RenderTypeId::Index(*idx)), - generics: None, - bindings: None, - }); - } else { - let idx = -isize::try_from(rgen.len() + 1).unwrap(); - rgen.insert(SimplifiedParam::Symbol(arg_s), (idx, type_bounds)); - res.push(RenderType { - id: Some(RenderTypeId::Index(idx)), - generics: None, - bindings: None, - }); - } - } else if let Type::ImplTrait(ref bounds) = *arg { - let mut type_bounds = Vec::new(); - for bound in bounds { - if let Some(path) = bound.get_trait_path() { - let ty = Type::Path { path }; - simplify_fn_type( - self_, - generics, - &ty, - tcx, - recurse + 1, - &mut type_bounds, - rgen, - is_return, - cache, - ); - } - } - if is_return && !type_bounds.is_empty() { - // In parameter position, `impl Trait` is a unique thing. - res.push(RenderType { id: None, generics: Some(type_bounds), bindings: None }); - } else { - // In parameter position, `impl Trait` is the same as an unnamed generic parameter. - let idx = -isize::try_from(rgen.len() + 1).unwrap(); - rgen.insert(SimplifiedParam::Anonymous(idx), (idx, type_bounds)); - res.push(RenderType { - id: Some(RenderTypeId::Index(idx)), - generics: None, - bindings: None, - }); - } - } else if let Type::Slice(ref ty) = *arg { - let mut ty_generics = Vec::new(); - simplify_fn_type( - self_, - generics, - &ty, - tcx, - recurse + 1, - &mut ty_generics, - rgen, - is_return, - cache, - ); - res.push(get_index_type(arg, ty_generics, rgen)); - } else if let Type::Array(ref ty, _) = *arg { - let mut ty_generics = Vec::new(); - simplify_fn_type( - self_, - generics, - &ty, - tcx, - recurse + 1, - &mut ty_generics, - rgen, - is_return, - cache, - ); - res.push(get_index_type(arg, ty_generics, rgen)); - } else if let Type::Tuple(ref tys) = *arg { - let mut ty_generics = Vec::new(); - for ty in tys { + Type::Slice(ref ty) => { + let mut ty_generics = Vec::new(); simplify_fn_type( self_, generics, @@ -986,15 +960,14 @@ fn simplify_fn_type<'tcx, 'a>( is_return, cache, ); + res.push(get_index_type(arg, ty_generics, rgen)); } - res.push(get_index_type(arg, ty_generics, rgen)); - } else if let Type::BareFunction(ref bf) = *arg { - let mut ty_generics = Vec::new(); - for ty in bf.decl.inputs.values.iter().map(|arg| &arg.type_) { + Type::Array(ref ty, _) => { + let mut ty_generics = Vec::new(); simplify_fn_type( self_, generics, - ty, + &ty, tcx, recurse + 1, &mut ty_generics, @@ -1002,62 +975,11 @@ fn simplify_fn_type<'tcx, 'a>( is_return, cache, ); + res.push(get_index_type(arg, ty_generics, rgen)); } - // The search index, for simplicity's sake, represents fn pointers and closures - // the same way: as a tuple for the parameters, and an associated type for the - // return type. - let mut ty_output = Vec::new(); - simplify_fn_type( - self_, - generics, - &bf.decl.output, - tcx, - recurse + 1, - &mut ty_output, - rgen, - is_return, - cache, - ); - let ty_bindings = vec![(RenderTypeId::AssociatedType(sym::Output), ty_output)]; - res.push(RenderType { - id: get_index_type_id(&arg, rgen), - bindings: Some(ty_bindings), - generics: Some(ty_generics), - }); - } else if let Type::BorrowedRef { lifetime: _, mutability, ref type_ } = *arg { - let mut ty_generics = Vec::new(); - if mutability.is_mut() { - ty_generics.push(RenderType { - id: Some(RenderTypeId::Mut), - generics: None, - bindings: None, - }); - } - simplify_fn_type( - self_, - generics, - &type_, - tcx, - recurse + 1, - &mut ty_generics, - rgen, - is_return, - cache, - ); - res.push(get_index_type(arg, ty_generics, rgen)); - } else { - // This is not a type parameter. So for example if we have `T, U: Option`, and we're - // looking at `Option`, we enter this "else" condition, otherwise if it's `T`, we don't. - // - // So in here, we can add it directly and look for its own type parameters (so for `Option`, - // we will look for them but not for `T`). - let mut ty_generics = Vec::new(); - let mut ty_constraints = Vec::new(); - if let Some(arg_generics) = arg.generic_args() { - for ty in arg_generics.into_iter().filter_map(|param| match param { - clean::GenericArg::Type(ty) => Some(ty), - _ => None, - }) { + Type::Tuple(ref tys) => { + let mut ty_generics = Vec::new(); + for ty in tys { simplify_fn_type( self_, generics, @@ -1070,94 +992,181 @@ fn simplify_fn_type<'tcx, 'a>( cache, ); } - for constraint in arg_generics.constraints() { - simplify_fn_constraint( + res.push(get_index_type(arg, ty_generics, rgen)); + } + Type::BareFunction(ref bf) => { + let mut ty_generics = Vec::new(); + for ty in bf.decl.inputs.values.iter().map(|arg| &arg.type_) { + simplify_fn_type( self_, generics, - &constraint, + ty, tcx, recurse + 1, - &mut ty_constraints, + &mut ty_generics, rgen, is_return, cache, ); } + // The search index, for simplicity's sake, represents fn pointers and closures + // the same way: as a tuple for the parameters, and an associated type for the + // return type. + let mut ty_output = Vec::new(); + simplify_fn_type( + self_, + generics, + &bf.decl.output, + tcx, + recurse + 1, + &mut ty_output, + rgen, + is_return, + cache, + ); + let ty_bindings = vec![(RenderTypeId::AssociatedType(sym::Output), ty_output)]; + res.push(RenderType { + id: get_index_type_id(&arg, rgen), + bindings: Some(ty_bindings), + generics: Some(ty_generics), + }); } - // Every trait associated type on self gets assigned to a type parameter index - // this same one is used later for any appearances of these types - // - // for example, Iterator::next is: - // - // trait Iterator { - // fn next(&mut self) -> Option - // } - // - // Self is technically just Iterator, but we want to pretend it's more like this: - // - // fn next(self: Iterator) -> Option - if is_self - && let Type::Path { path } = arg - && let def_id = path.def_id() - && let Some(trait_) = cache.traits.get(&def_id) - && trait_.items.iter().any(|at| at.is_ty_associated_type()) - { - for assoc_ty in &trait_.items { - if let clean::ItemKind::TyAssocTypeItem(_generics, bounds) = &*assoc_ty.kind - && let Some(name) = assoc_ty.name - { - let idx = -isize::try_from(rgen.len() + 1).unwrap(); - let (idx, stored_bounds) = rgen - .entry(SimplifiedParam::AssociatedType(def_id, name)) - .or_insert_with(|| (idx, Vec::new())); - let idx = *idx; - if stored_bounds.is_empty() { - // Can't just pass stored_bounds to simplify_fn_type, - // because it also accepts rgen as a parameter. - // Instead, have it fill in this local, then copy it into the map afterward. - let mut type_bounds = Vec::new(); - for bound in bounds { - if let Some(path) = bound.get_trait_path() { - let ty = Type::Path { path }; - simplify_fn_type( - self_, - generics, - &ty, - tcx, - recurse + 1, - &mut type_bounds, - rgen, - is_return, - cache, - ); - } - } - let stored_bounds = &mut rgen - .get_mut(&SimplifiedParam::AssociatedType(def_id, name)) - .unwrap() - .1; - if stored_bounds.is_empty() { - *stored_bounds = type_bounds; - } - } - ty_constraints.push(( - RenderTypeId::AssociatedType(name), - vec![RenderType { - id: Some(RenderTypeId::Index(idx)), - generics: None, - bindings: None, - }], - )) + Type::BorrowedRef { lifetime: _, mutability, ref type_ } => { + let mut ty_generics = Vec::new(); + if mutability.is_mut() { + ty_generics.push(RenderType { + id: Some(RenderTypeId::Mut), + generics: None, + bindings: None, + }); + } + simplify_fn_type( + self_, + generics, + &type_, + tcx, + recurse + 1, + &mut ty_generics, + rgen, + is_return, + cache, + ); + res.push(get_index_type(arg, ty_generics, rgen)); + } + _ => { + // This is not a type parameter. So for example if we have `T, U: Option`, and we're + // looking at `Option`, we enter this "else" condition, otherwise if it's `T`, we don't. + // + // So in here, we can add it directly and look for its own type parameters (so for `Option`, + // we will look for them but not for `T`). + let mut ty_generics = Vec::new(); + let mut ty_constraints = Vec::new(); + if let Some(arg_generics) = arg.generic_args() { + for ty in arg_generics.into_iter().filter_map(|param| match param { + clean::GenericArg::Type(ty) => Some(ty), + _ => None, + }) { + simplify_fn_type( + self_, + generics, + &ty, + tcx, + recurse + 1, + &mut ty_generics, + rgen, + is_return, + cache, + ); + } + for constraint in arg_generics.constraints() { + simplify_fn_constraint( + self_, + generics, + &constraint, + tcx, + recurse + 1, + &mut ty_constraints, + rgen, + is_return, + cache, + ); } } - } - let id = get_index_type_id(&arg, rgen); - if id.is_some() || !ty_generics.is_empty() { - res.push(RenderType { - id, - bindings: if ty_constraints.is_empty() { None } else { Some(ty_constraints) }, - generics: if ty_generics.is_empty() { None } else { Some(ty_generics) }, - }); + // Every trait associated type on self gets assigned to a type parameter index + // this same one is used later for any appearances of these types + // + // for example, Iterator::next is: + // + // trait Iterator { + // fn next(&mut self) -> Option + // } + // + // Self is technically just Iterator, but we want to pretend it's more like this: + // + // fn next(self: Iterator) -> Option + if is_self + && let Type::Path { path } = arg + && let def_id = path.def_id() + && let Some(trait_) = cache.traits.get(&def_id) + && trait_.items.iter().any(|at| at.is_ty_associated_type()) + { + for assoc_ty in &trait_.items { + if let clean::ItemKind::TyAssocTypeItem(_generics, bounds) = &*assoc_ty.kind + && let Some(name) = assoc_ty.name + { + let idx = -isize::try_from(rgen.len() + 1).unwrap(); + let (idx, stored_bounds) = rgen + .entry(SimplifiedParam::AssociatedType(def_id, name)) + .or_insert_with(|| (idx, Vec::new())); + let idx = *idx; + if stored_bounds.is_empty() { + // Can't just pass stored_bounds to simplify_fn_type, + // because it also accepts rgen as a parameter. + // Instead, have it fill in this local, then copy it into the map afterward. + let mut type_bounds = Vec::new(); + for bound in bounds { + if let Some(path) = bound.get_trait_path() { + let ty = Type::Path { path }; + simplify_fn_type( + self_, + generics, + &ty, + tcx, + recurse + 1, + &mut type_bounds, + rgen, + is_return, + cache, + ); + } + } + let stored_bounds = &mut rgen + .get_mut(&SimplifiedParam::AssociatedType(def_id, name)) + .unwrap() + .1; + if stored_bounds.is_empty() { + *stored_bounds = type_bounds; + } + } + ty_constraints.push(( + RenderTypeId::AssociatedType(name), + vec![RenderType { + id: Some(RenderTypeId::Index(idx)), + generics: None, + bindings: None, + }], + )) + } + } + } + let id = get_index_type_id(&arg, rgen); + if id.is_some() || !ty_generics.is_empty() { + res.push(RenderType { + id, + bindings: if ty_constraints.is_empty() { None } else { Some(ty_constraints) }, + generics: if ty_generics.is_empty() { None } else { Some(ty_generics) }, + }); + } } } } From b4f77df9f8540f48744794c641e12ef66e0e57ba Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 31 Jul 2024 17:33:13 -0700 Subject: [PATCH 5/6] rustdoc: Delete `ReceiverTy` (formerly known as `SelfTy`) It was barely used, and the places that used it are actually clearer without it since they were often undoing some of its work. This also avoids an unnecessary clone of the receiver type and removes a layer of logical indirection in the code. --- src/librustdoc/clean/types.rs | 25 +++---------------------- src/librustdoc/html/format.rs | 27 ++++++++++----------------- src/librustdoc/html/render/mod.rs | 11 +++++------ 3 files changed, 18 insertions(+), 45 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 82f1312364a..4850500a1bf 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -34,7 +34,6 @@ use {rustc_ast as ast, rustc_hir as hir}; pub(crate) use self::ItemKind::*; -pub(crate) use self::ReceiverTy::*; pub(crate) use self::Type::{ Array, BareFunction, BorrowedRef, DynTrait, Generic, ImplTrait, Infer, Primitive, QPath, RawPointer, SelfTy, Slice, Tuple, @@ -1384,7 +1383,7 @@ pub(crate) struct FnDecl { } impl FnDecl { - pub(crate) fn receiver_type(&self) -> Option { + pub(crate) fn receiver_type(&self) -> Option<&Type> { self.inputs.values.get(0).and_then(|v| v.to_receiver()) } } @@ -1403,27 +1402,9 @@ pub(crate) struct Argument { pub(crate) is_const: bool, } -#[derive(Clone, PartialEq, Debug)] -pub(crate) enum ReceiverTy { - SelfValue, - SelfBorrowed(Option, Mutability), - SelfExplicit(Type), -} - impl Argument { - pub(crate) fn to_receiver(&self) -> Option { - if self.name != kw::SelfLower { - return None; - } - if self.type_.is_self_type() { - return Some(SelfValue); - } - match self.type_ { - BorrowedRef { ref lifetime, mutability, ref type_ } if type_.is_self_type() => { - Some(SelfBorrowed(lifetime.clone(), mutability)) - } - _ => Some(SelfExplicit(self.type_.clone())), - } + pub(crate) fn to_receiver(&self) -> Option<&Type> { + if self.name == kw::SelfLower { Some(&self.type_) } else { None } } } diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 0fe535ade9b..b5ab6a35fdb 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1455,27 +1455,20 @@ fn inner_full_print( for (i, input) in self.inputs.values.iter().enumerate() { if let Some(selfty) = input.to_receiver() { match selfty { - clean::SelfValue => { + clean::SelfTy => { write!(f, "self")?; } - clean::SelfBorrowed(Some(ref lt), mutability) => { - write!( - f, - "{amp}{lifetime} {mutability}self", - lifetime = lt.print(), - mutability = mutability.print_with_space(), - )?; + clean::BorrowedRef { lifetime, mutability, type_: box clean::SelfTy } => { + write!(f, "{amp}")?; + match lifetime { + Some(lt) => write!(f, "{lt} ", lt = lt.print())?, + None => {} + } + write!(f, "{mutability}self", mutability = mutability.print_with_space())?; } - clean::SelfBorrowed(None, mutability) => { - write!( - f, - "{amp}{mutability}self", - mutability = mutability.print_with_space(), - )?; - } - clean::SelfExplicit(ref typ) => { + _ => { write!(f, "self: ")?; - typ.print(cx).fmt(f)?; + selfty.print(cx).fmt(f)?; } } } else { diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index b7bcda2a604..9074e40a536 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -58,7 +58,7 @@ pub(crate) use self::context::*; pub(crate) use self::span_map::{collect_spans_and_sources, LinkFromSrc}; -use crate::clean::{self, ItemId, ReceiverTy, RenderedLink}; +use crate::clean::{self, ItemId, RenderedLink}; use crate::error::Error; use crate::formats::cache::Cache; use crate::formats::item_type::ItemType; @@ -1378,15 +1378,14 @@ fn should_render_item(item: &clean::Item, deref_mut_: bool, tcx: TyCtxt<'_>) -> }; if let Some(self_ty) = self_type_opt { - let (by_mut_ref, by_box, by_value) = match self_ty { - ReceiverTy::SelfBorrowed(_, mutability) - | ReceiverTy::SelfExplicit(clean::BorrowedRef { mutability, .. }) => { + let (by_mut_ref, by_box, by_value) = match *self_ty { + clean::Type::BorrowedRef { mutability, .. } => { (mutability == Mutability::Mut, false, false) } - ReceiverTy::SelfExplicit(clean::Type::Path { path }) => { + clean::Type::Path { ref path } => { (false, Some(path.def_id()) == tcx.lang_items().owned_box(), false) } - ReceiverTy::SelfValue => (false, false, true), + clean::Type::SelfTy => (false, false, true), _ => (false, false, false), }; From dac7f20e1308ea17655d17f43dffaca813857300 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 31 Jul 2024 20:13:17 -0700 Subject: [PATCH 6/6] Add test for `Self` not being a generic in search index --- tests/rustdoc-js/self-is-not-generic.js | 22 ++++++++++++++++++++++ tests/rustdoc-js/self-is-not-generic.rs | 11 +++++++++++ 2 files changed, 33 insertions(+) create mode 100644 tests/rustdoc-js/self-is-not-generic.js create mode 100644 tests/rustdoc-js/self-is-not-generic.rs diff --git a/tests/rustdoc-js/self-is-not-generic.js b/tests/rustdoc-js/self-is-not-generic.js new file mode 100644 index 00000000000..0fdf5b4117d --- /dev/null +++ b/tests/rustdoc-js/self-is-not-generic.js @@ -0,0 +1,22 @@ +// exact-check + +const EXPECTED = [ + { + 'query': 'A -> A', + 'others': [ + { 'path': 'self_is_not_generic::Thing', 'name': 'from' } + ], + }, + { + 'query': 'A -> B', + 'others': [ + { 'path': 'self_is_not_generic::Thing', 'name': 'try_from' } + ], + }, + { + 'query': 'Combine -> Combine', + 'others': [ + { 'path': 'self_is_not_generic::Combine', 'name': 'combine' } + ], + } +]; diff --git a/tests/rustdoc-js/self-is-not-generic.rs b/tests/rustdoc-js/self-is-not-generic.rs new file mode 100644 index 00000000000..d6a96acb73c --- /dev/null +++ b/tests/rustdoc-js/self-is-not-generic.rs @@ -0,0 +1,11 @@ +pub trait Combine { + fn combine(&self, other: &Self) -> Self; +} + +pub struct Thing; + +impl Combine for Thing { + fn combine(&self, other: &Self) -> Self { + Self + } +}