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 7d5a16bc7e3d5..32e795e565ed2 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -34,7 +34,7 @@ use thin_vec::ThinVec; 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 bb5ac303ffd63..6476c8bd7c0d3 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1452,7 +1452,7 @@ impl clean::FnDecl { 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 9617f0d9a1f25..b7bcda2a604cf 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -58,7 +58,7 @@ use serde::{Serialize, Serializer}; 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 d92bc8456649f..f8953f0ebcfb5 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -12,7 +12,7 @@ use rustc_middle::ty::fast_reject::SimplifiedType; 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 324b633e8ea7e..cffadc7c10a91 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 @@ pub(crate) fn clean_middle_assoc_item<'tcx>( 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 739f6eb8cc89d..1d81ae3eb8ba7 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 32e795e565ed2..82f1312364a5b 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -37,7 +37,7 @@ 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, 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 @@ impl Type { // 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 @@ impl Type { pub(crate) fn is_self_type(&self) -> bool { match *self { - Generic(name) => name == kw::SelfUpper, + SelfTy => true, _ => false, } } @@ -1700,7 +1704,7 @@ impl Type { 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 2dd3041ab4c61..68266f3506a01 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 6476c8bd7c0d3..0fe535ade9b2a 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 b3f4d82e054b8..fc3d6c99b809f 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 b56244f2d3b6d..b97d710c007ce 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 @@ impl FromWithTcx for Type { 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 fc3d6c99b809f..e4d948b8c9c1a 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 | 477 +++++++++++---------- 1 file changed, 243 insertions(+), 234 deletions(-) diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index e4d948b8c9c1a..8a2f31f7413e1 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -854,34 +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() { - 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, - ); + 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, + }); + } } - // 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(&[]) { + 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( @@ -897,84 +933,22 @@ fn simplify_fn_type<'tcx, 'a>( ); } } - } - 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 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 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; + 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, + ); + } + } + // 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() { - *stored_bounds = type_bounds; + // 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, + }], + )) } - 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) }, - }); + 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 82f1312364a5b..4850500a1bfae 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -34,7 +34,6 @@ use thin_vec::ThinVec; 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 0fe535ade9b2a..b5ab6a35fdb9d 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1455,27 +1455,20 @@ impl clean::FnDecl { 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::SelfBorrowed(None, mutability) => { - write!( - f, - "{amp}{mutability}self", - 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::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 b7bcda2a604cf..9074e40a53614 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -58,7 +58,7 @@ use serde::{Serialize, Serializer}; 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 0000000000000..0fdf5b4117d8f --- /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 0000000000000..d6a96acb73caf --- /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 + } +}