From 07d6454a0fe1f34096a5c20d3f59d8c20e8a0823 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Sun, 22 Aug 2021 20:08:48 -0700 Subject: [PATCH 1/3] Remove `Type::ResolvedPath.is_generic` It can be computed on-demand. --- src/librustdoc/clean/auto_trait.rs | 15 ++-------- src/librustdoc/clean/mod.rs | 13 ++++----- src/librustdoc/clean/types.rs | 20 ++++++++----- src/librustdoc/clean/utils.rs | 24 ++++++--------- src/librustdoc/html/format.rs | 37 +++++++++--------------- src/librustdoc/html/render/print_item.rs | 18 ++++++------ src/librustdoc/json/conversions.rs | 2 +- 7 files changed, 52 insertions(+), 77 deletions(-) diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index 11adb56490b..83a4a4157b2 100644 --- a/src/librustdoc/clean/auto_trait.rs +++ b/src/librustdoc/clean/auto_trait.rs @@ -354,7 +354,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { let (poly_trait, output) = (data.0.as_ref().unwrap().clone(), data.1.as_ref().cloned().map(Box::new)); let new_ty = match poly_trait.trait_ { - Type::ResolvedPath { ref path, ref did, ref is_generic } => { + Type::ResolvedPath { ref path, ref did } => { let mut new_path = path.clone(); let last_segment = new_path.segments.pop().expect("segments were empty"); @@ -389,11 +389,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { .segments .push(PathSegment { name: last_segment.name, args: new_params }); - Type::ResolvedPath { - path: new_path, - did: *did, - is_generic: *is_generic, - } + Type::ResolvedPath { path: new_path, did: *did } } _ => panic!("Unexpected data: {:?}, {:?}", ty, data), }; @@ -563,11 +559,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { Type::QPath { name: left_name, ref self_type, ref trait_, .. } => { let ty = &*self_type; match **trait_ { - Type::ResolvedPath { - path: ref trait_path, - ref did, - ref is_generic, - } => { + Type::ResolvedPath { path: ref trait_path, ref did } => { let mut new_trait_path = trait_path.clone(); if self.is_fn_ty(trait_) && left_name == sym::Output { @@ -612,7 +604,6 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { trait_: Type::ResolvedPath { path: new_trait_path, did: *did, - is_generic: *is_generic, }, generic_params: Vec::new(), }, diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index e281bbc59c2..a63fea595be 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -168,7 +168,7 @@ impl Clean for (ty::TraitRef<'_>, &[TypeBinding]) { debug!("ty::TraitRef\n subst: {:?}\n", trait_ref.substs); - ResolvedPath { path, did: trait_ref.def_id, is_generic: false } + ResolvedPath { path, did: trait_ref.def_id } } } @@ -1442,12 +1442,12 @@ impl<'tcx> Clean for Ty<'tcx> { }; inline::record_extern_fqn(cx, did, kind); let path = external_path(cx, did, false, vec![], substs); - ResolvedPath { path, did, is_generic: false } + ResolvedPath { path, did } } ty::Foreign(did) => { inline::record_extern_fqn(cx, did, ItemType::ForeignType); let path = external_path(cx, did, false, vec![], InternalSubsts::empty()); - ResolvedPath { path, did, is_generic: false } + ResolvedPath { path, did } } ty::Dynamic(ref obj, ref reg) => { // HACK: pick the first `did` as the `did` of the trait object. Someone @@ -1473,7 +1473,7 @@ impl<'tcx> Clean for Ty<'tcx> { let path = external_path(cx, did, false, vec![], empty); inline::record_extern_fqn(cx, did, ItemType::Trait); let bound = PolyTrait { - trait_: ResolvedPath { path, did, is_generic: false }, + trait_: ResolvedPath { path, did }, generic_params: Vec::new(), }; bounds.push(bound); @@ -1490,10 +1490,7 @@ impl<'tcx> Clean for Ty<'tcx> { let path = external_path(cx, did, false, bindings, substs); bounds.insert( 0, - PolyTrait { - trait_: ResolvedPath { path, did, is_generic: false }, - generic_params: Vec::new(), - }, + PolyTrait { trait_: ResolvedPath { path, did }, generic_params: Vec::new() }, ); DynTrait(bounds, lifetime) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index b3c320555f9..3033af333df 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1114,10 +1114,7 @@ impl GenericBound { let path = external_path(cx, did, false, vec![], empty); inline::record_extern_fqn(cx, did, ItemType::Trait); GenericBound::TraitBound( - PolyTrait { - trait_: ResolvedPath { path, did, is_generic: false }, - generic_params: Vec::new(), - }, + PolyTrait { trait_: ResolvedPath { path, did }, generic_params: Vec::new() }, hir::TraitBoundModifier::Maybe, ) } @@ -1384,8 +1381,6 @@ crate enum Type { ResolvedPath { path: Path, did: DefId, - /// `true` if is a `T::Name` path for associated types. - is_generic: bool, }, /// `dyn for<'a> Trait<'a> + Send + 'static` DynTrait(Vec, Option), @@ -1504,8 +1499,8 @@ impl Type { } crate fn is_generic(&self) -> bool { - match *self { - ResolvedPath { is_generic, .. } => is_generic, + match self { + ResolvedPath { path, .. } => path.is_generic(), _ => false, } } @@ -1994,6 +1989,15 @@ impl Path { String::from(if self.global { "::" } else { "" }) + &self.segments.iter().map(|s| s.name.to_string()).collect::>().join("::") } + + crate fn is_generic(&self) -> bool { + match self.res { + Res::SelfTy(..) if self.segments.len() != 1 => true, + Res::Def(DefKind::TyParam, _) if self.segments.len() != 1 => true, + Res::Def(DefKind::AssocTy, _) => true, + _ => false, + } + } } #[derive(Clone, PartialEq, Eq, Debug, Hash)] diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index b0021d1234c..33d460d587a 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -159,9 +159,7 @@ pub(super) fn external_path( crate fn strip_type(ty: Type) -> Type { match ty { - Type::ResolvedPath { path, did, is_generic } => { - Type::ResolvedPath { path: strip_path(&path), did, is_generic } - } + Type::ResolvedPath { path, did } => Type::ResolvedPath { path: strip_path(&path), did }, Type::DynTrait(mut bounds, lt) => { let first = bounds.remove(0); let stripped_trait = strip_type(first.trait_); @@ -404,19 +402,15 @@ crate fn print_const_expr(tcx: TyCtxt<'_>, body: hir::BodyId) -> String { crate fn resolve_type(cx: &mut DocContext<'_>, path: Path) -> Type { debug!("resolve_type({:?})", path); - let is_generic = match path.res { - Res::PrimTy(p) => return Primitive(PrimitiveType::from(p)), - Res::SelfTy(..) if path.segments.len() == 1 => { - return Generic(kw::SelfUpper); + match path.res { + Res::PrimTy(p) => Primitive(PrimitiveType::from(p)), + Res::SelfTy(..) if path.segments.len() == 1 => Generic(kw::SelfUpper), + Res::Def(DefKind::TyParam, _) if path.segments.len() == 1 => Generic(path.segments[0].name), + _ => { + let did = register_res(cx, path.res); + ResolvedPath { path, did } } - Res::Def(DefKind::TyParam, _) if path.segments.len() == 1 => { - return Generic(path.segments[0].name); - } - Res::SelfTy(..) | Res::Def(DefKind::TyParam | DefKind::AssocTy, _) => true, - _ => false, - }; - let did = register_res(cx, path.res); - ResolvedPath { path, did, is_generic } + } } crate fn get_auto_trait_and_blanket_impls( diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 2fde0017dc8..722d91ca69b 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -752,9 +752,9 @@ fn fmt_type<'cx>( match *t { clean::Generic(name) => write!(f, "{}", name), - clean::ResolvedPath { did, ref path, is_generic } => { + clean::ResolvedPath { did, ref path } => { // Paths like `T::Output` and `Self::Output` should be rendered with all segments. - resolved_path(f, did, path, is_generic, use_absolute, cx) + resolved_path(f, did, path, path.is_generic(), use_absolute, cx) } clean::DynTrait(ref bounds, ref lt) => { f.write_str("dyn ")?; @@ -825,28 +825,17 @@ fn fmt_type<'cx>( hir::Mutability::Mut => "mut", hir::Mutability::Not => "const", }; - match **t { - clean::Generic(_) | clean::ResolvedPath { is_generic: true, .. } => { - if f.alternate() { - primitive_link( - f, - clean::PrimitiveType::RawPointer, - &format!("*{} {:#}", m, t.print(cx)), - cx, - ) - } else { - primitive_link( - f, - clean::PrimitiveType::RawPointer, - &format!("*{} {}", m, t.print(cx)), - cx, - ) - } - } - _ => { - primitive_link(f, clean::PrimitiveType::RawPointer, &format!("*{} ", m), cx)?; - fmt::Display::fmt(&t.print(cx), f) - } + + if matches!(**t, clean::Generic(_)) || t.is_generic() { + let text = if f.alternate() { + format!("*{} {:#}", m, t.print(cx)) + } else { + format!("*{} {}", m, t.print(cx)) + }; + primitive_link(f, clean::PrimitiveType::RawPointer, &text, cx) + } else { + primitive_link(f, clean::PrimitiveType::RawPointer, &format!("*{} ", m), cx)?; + fmt::Display::fmt(&t.print(cx), f) } } clean::BorrowedRef { lifetime: ref l, mutability, type_: ref ty } => { diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 52505f2d634..90fed020119 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -712,11 +712,10 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra let mut implementor_dups: FxHashMap = FxHashMap::default(); for implementor in implementors { match implementor.inner_impl().for_ { - clean::ResolvedPath { ref path, did, is_generic: false, .. } + clean::ResolvedPath { ref path, did, .. } | clean::BorrowedRef { - type_: box clean::ResolvedPath { ref path, did, is_generic: false, .. }, - .. - } => { + type_: box clean::ResolvedPath { ref path, did, .. }, .. + } if !path.is_generic() => { let &mut (prev_did, ref mut has_duplicates) = implementor_dups.entry(path.last()).or_insert((did, false)); if prev_did != did { @@ -1410,11 +1409,12 @@ fn render_implementor( // If there's already another implementor that has the same abridged name, use the // full path, for example in `std::iter::ExactSizeIterator` let use_absolute = match implementor.inner_impl().for_ { - clean::ResolvedPath { ref path, is_generic: false, .. } - | clean::BorrowedRef { - type_: box clean::ResolvedPath { ref path, is_generic: false, .. }, - .. - } => implementor_dups[&path.last()].1, + clean::ResolvedPath { ref path, .. } + | clean::BorrowedRef { type_: box clean::ResolvedPath { ref path, .. }, .. } + if !path.is_generic() => + { + implementor_dups[&path.last()].1 + } _ => false, }; render_impl( diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index f8ea7a499b2..fda90703057 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -387,7 +387,7 @@ impl FromWithTcx for Type { fn from_tcx(ty: clean::Type, tcx: TyCtxt<'_>) -> Self { use clean::Type::*; match ty { - ResolvedPath { path, did, is_generic: _ } => Type::ResolvedPath { + ResolvedPath { path, did } => Type::ResolvedPath { name: path.whole_name(), id: from_item_id(did.into()), args: path.segments.last().map(|args| Box::new(args.clone().args.into_tcx(tcx))), From 1085dc2148594bc259f9629b7ca4aa9489c8c10d Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 30 Aug 2021 20:53:57 -0700 Subject: [PATCH 2/3] Rename `is_generic()` to `is_assoc_ty()` The new name is more accurate than the previous one. --- src/librustdoc/clean/types.rs | 6 +++--- src/librustdoc/html/format.rs | 4 ++-- src/librustdoc/html/render/print_item.rs | 4 ++-- src/librustdoc/passes/stripper.rs | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 3033af333df..0139c4e879a 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1498,9 +1498,9 @@ impl Type { } } - crate fn is_generic(&self) -> bool { + crate fn is_assoc_ty(&self) -> bool { match self { - ResolvedPath { path, .. } => path.is_generic(), + ResolvedPath { path, .. } => path.is_assoc_ty(), _ => false, } } @@ -1990,7 +1990,7 @@ impl Path { + &self.segments.iter().map(|s| s.name.to_string()).collect::>().join("::") } - crate fn is_generic(&self) -> bool { + crate fn is_assoc_ty(&self) -> bool { match self.res { Res::SelfTy(..) if self.segments.len() != 1 => true, Res::Def(DefKind::TyParam, _) if self.segments.len() != 1 => true, diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 722d91ca69b..d11781581a8 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -754,7 +754,7 @@ fn fmt_type<'cx>( clean::Generic(name) => write!(f, "{}", name), clean::ResolvedPath { did, ref path } => { // Paths like `T::Output` and `Self::Output` should be rendered with all segments. - resolved_path(f, did, path, path.is_generic(), use_absolute, cx) + resolved_path(f, did, path, path.is_assoc_ty(), use_absolute, cx) } clean::DynTrait(ref bounds, ref lt) => { f.write_str("dyn ")?; @@ -826,7 +826,7 @@ fn fmt_type<'cx>( hir::Mutability::Not => "const", }; - if matches!(**t, clean::Generic(_)) || t.is_generic() { + if matches!(**t, clean::Generic(_)) || t.is_assoc_ty() { let text = if f.alternate() { format!("*{} {:#}", m, t.print(cx)) } else { diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 90fed020119..1906a7bc3f3 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -715,7 +715,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra clean::ResolvedPath { ref path, did, .. } | clean::BorrowedRef { type_: box clean::ResolvedPath { ref path, did, .. }, .. - } if !path.is_generic() => { + } if !path.is_assoc_ty() => { let &mut (prev_did, ref mut has_duplicates) = implementor_dups.entry(path.last()).or_insert((did, false)); if prev_did != did { @@ -1411,7 +1411,7 @@ fn render_implementor( let use_absolute = match implementor.inner_impl().for_ { clean::ResolvedPath { ref path, .. } | clean::BorrowedRef { type_: box clean::ResolvedPath { ref path, .. }, .. } - if !path.is_generic() => + if !path.is_assoc_ty() => { implementor_dups[&path.last()].1 } diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index a93880453ba..90300dbd16b 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -128,7 +128,7 @@ impl<'a> DocFolder for ImplStripper<'a> { return None; } if let Some(did) = imp.for_.def_id() { - if did.is_local() && !imp.for_.is_generic() && !self.retained.contains(&did.into()) + if did.is_local() && !imp.for_.is_assoc_ty() && !self.retained.contains(&did.into()) { debug!("ImplStripper: impl item for stripped type; removing"); return None; From dace2ee674da9550618563babe351f313ede799d Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Sat, 25 Sep 2021 09:50:12 -0700 Subject: [PATCH 3/3] rustdoc: Document `is_assoc_ty()` It's adapted from the old documentation for the `is_generic` field. --- src/librustdoc/clean/types.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 0139c4e879a..986729dd044 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1498,6 +1498,7 @@ impl Type { } } + /// Checks if this is a `T::Name` path for an associated type. crate fn is_assoc_ty(&self) -> bool { match self { ResolvedPath { path, .. } => path.is_assoc_ty(), @@ -1990,6 +1991,7 @@ impl Path { + &self.segments.iter().map(|s| s.name.to_string()).collect::>().join("::") } + /// Checks if this is a `T::Name` path for an associated type. crate fn is_assoc_ty(&self) -> bool { match self.res { Res::SelfTy(..) if self.segments.len() != 1 => true,