From 3490a510d552390f1970cb36d5ca76569f571a97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Fri, 4 Nov 2022 17:05:56 +0100 Subject: [PATCH 1/2] rustdoc: re-elide cross-crate default trait object lifetime bounds --- src/librustdoc/clean/auto_trait.rs | 2 +- src/librustdoc/clean/blanket_impl.rs | 8 +- src/librustdoc/clean/inline.rs | 19 +- src/librustdoc/clean/mod.rs | 245 ++++++++++++++++-- src/librustdoc/clean/utils.rs | 43 +-- tests/rustdoc/assoc-consts.rs | 1 - .../inline_cross/auxiliary/dyn_trait.rs | 60 ++++- tests/rustdoc/inline_cross/dyn_trait.rs | 125 ++++++++- 8 files changed, 435 insertions(+), 68 deletions(-) diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index baf2b0a858529..c8a40e0150112 100644 --- a/src/librustdoc/clean/auto_trait.rs +++ b/src/librustdoc/clean/auto_trait.rs @@ -124,7 +124,7 @@ where unsafety: hir::Unsafety::Normal, generics: new_generics, trait_: Some(clean_trait_ref_with_bindings(self.cx, trait_ref, ThinVec::new())), - for_: clean_middle_ty(ty::Binder::dummy(ty), self.cx, None), + for_: clean_middle_ty(ty::Binder::dummy(ty), self.cx, None, None), items: Vec::new(), polarity, kind: ImplKind::Auto, diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index 9183fdaa08704..a36041588510f 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -107,7 +107,12 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { ty::Binder::dummy(trait_ref.subst_identity()), ThinVec::new(), )), - for_: clean_middle_ty(ty::Binder::dummy(ty.subst_identity()), cx, None), + for_: clean_middle_ty( + ty::Binder::dummy(ty.subst_identity()), + cx, + None, + None, + ), items: cx .tcx .associated_items(impl_def_id) @@ -119,6 +124,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { ty::Binder::dummy(trait_ref.subst_identity().self_ty()), cx, None, + None, ))), }))), cfg: None, diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 7dc08b3b1ffa6..3aa98da1c803a 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -278,8 +278,12 @@ fn build_union(cx: &mut DocContext<'_>, did: DefId) -> clean::Union { fn build_type_alias(cx: &mut DocContext<'_>, did: DefId) -> Box { let predicates = cx.tcx.explicit_predicates_of(did); - let type_ = - clean_middle_ty(ty::Binder::dummy(cx.tcx.type_of(did).subst_identity()), cx, Some(did)); + let type_ = clean_middle_ty( + ty::Binder::dummy(cx.tcx.type_of(did).subst_identity()), + cx, + Some(did), + None, + ); Box::new(clean::Typedef { type_, @@ -386,9 +390,12 @@ pub(crate) fn build_impl( let for_ = match &impl_item { Some(impl_) => clean_ty(impl_.self_ty, cx), - None => { - clean_middle_ty(ty::Binder::dummy(tcx.type_of(did).subst_identity()), cx, Some(did)) - } + None => clean_middle_ty( + ty::Binder::dummy(tcx.type_of(did).subst_identity()), + cx, + Some(did), + None, + ), }; // Only inline impl if the implementing type is @@ -630,6 +637,7 @@ fn build_const(cx: &mut DocContext<'_>, def_id: DefId) -> clean::Constant { ty::Binder::dummy(cx.tcx.type_of(def_id).subst_identity()), cx, Some(def_id), + None, ), kind: clean::ConstantKind::Extern { def_id }, } @@ -641,6 +649,7 @@ fn build_static(cx: &mut DocContext<'_>, did: DefId, mutable: bool) -> clean::St ty::Binder::dummy(cx.tcx.type_of(did).subst_identity()), cx, Some(did), + None, ), mutability: if mutable { Mutability::Mut } else { Mutability::Not }, expr: None, diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index b56b81279962b..526c9a4a35fae 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -253,6 +253,7 @@ pub(crate) fn clean_const<'tcx>(constant: &hir::ConstArg, cx: &mut DocContext<'t ty::Binder::dummy(cx.tcx.type_of(def_id).subst_identity()), cx, Some(def_id), + None, ), kind: ConstantKind::Anonymous { body: constant.value.body }, } @@ -264,7 +265,7 @@ pub(crate) fn clean_middle_const<'tcx>( ) -> Constant { // FIXME: instead of storing the stringified expression, store `self` directly instead. Constant { - type_: clean_middle_ty(constant.map_bound(|c| c.ty()), cx, None), + type_: clean_middle_ty(constant.map_bound(|c| c.ty()), cx, None, None), kind: ConstantKind::TyConst { expr: constant.skip_binder().to_string().into() }, } } @@ -370,7 +371,7 @@ fn clean_poly_trait_predicate<'tcx>( let poly_trait_ref = pred.map_bound(|pred| pred.trait_ref); Some(WherePredicate::BoundPredicate { - ty: clean_middle_ty(poly_trait_ref.self_ty(), cx, None), + ty: clean_middle_ty(poly_trait_ref.self_ty(), cx, None, None), bounds: vec![clean_poly_trait_ref_with_bindings(cx, poly_trait_ref, ThinVec::new())], bound_params: Vec::new(), }) @@ -396,7 +397,7 @@ fn clean_type_outlives_predicate<'tcx>( let ty::OutlivesPredicate(ty, lt) = pred.skip_binder(); Some(WherePredicate::BoundPredicate { - ty: clean_middle_ty(pred.rebind(ty), cx, None), + ty: clean_middle_ty(pred.rebind(ty), cx, None, None), bounds: vec![GenericBound::Outlives( clean_middle_region(lt).expect("failed to clean lifetimes"), )], @@ -409,7 +410,7 @@ fn clean_middle_term<'tcx>( cx: &mut DocContext<'tcx>, ) -> Term { match term.skip_binder().unpack() { - ty::TermKind::Ty(ty) => Term::Type(clean_middle_ty(term.rebind(ty), cx, None)), + ty::TermKind::Ty(ty) => Term::Type(clean_middle_ty(term.rebind(ty), cx, None, None)), ty::TermKind::Const(c) => Term::Constant(clean_middle_const(term.rebind(c), cx)), } } @@ -462,7 +463,7 @@ fn clean_projection<'tcx>( let trait_ = clean_trait_ref_with_bindings(cx, ty.map_bound(|ty| ty.trait_ref(cx.tcx)), ThinVec::new()); - let self_type = clean_middle_ty(ty.map_bound(|ty| ty.self_ty()), cx, None); + let self_type = clean_middle_ty(ty.map_bound(|ty| ty.self_ty()), cx, None, None); let self_def_id = if let Some(def_id) = def_id { cx.tcx.opt_parent(def_id).or(Some(def_id)) } else { @@ -493,8 +494,13 @@ fn projection_to_path_segment<'tcx>( PathSegment { name: item.name, args: GenericArgs::AngleBracketed { - args: substs_to_args(cx, ty.map_bound(|ty| &ty.substs[generics.parent_count..]), false) - .into(), + args: substs_to_args( + cx, + ty.map_bound(|ty| &ty.substs[generics.parent_count..]), + false, + None, + ) + .into(), bindings: Default::default(), }, } @@ -514,6 +520,7 @@ fn clean_generic_param_def<'tcx>( ty::Binder::dummy(cx.tcx.type_of(def.def_id).subst_identity()), cx, Some(def.def_id), + None, )) } else { None @@ -540,6 +547,7 @@ fn clean_generic_param_def<'tcx>( ), cx, Some(def.def_id), + None, )), default: match has_default { true => Some(Box::new( @@ -871,7 +879,7 @@ fn clean_ty_generics<'tcx>( let crate::core::ImplTraitParam::ParamIndex(idx) = param else { unreachable!() }; if let Some(proj) = impl_trait_proj.remove(&idx) { for (trait_did, name, rhs, bound_params) in proj { - let rhs = clean_middle_ty(rhs, cx, None); + let rhs = clean_middle_ty(rhs, cx, None, None); simplify::merge_bounds( cx, &mut bounds, @@ -1126,7 +1134,7 @@ fn clean_fn_decl_from_did_and_sig<'tcx>( // We assume all empty tuples are default return type. This theoretically can discard `-> ()`, // but shouldn't change any code meaning. - let output = clean_middle_ty(sig.output(), cx, None); + let output = clean_middle_ty(sig.output(), cx, None, None); FnDecl { output, @@ -1136,7 +1144,7 @@ fn clean_fn_decl_from_did_and_sig<'tcx>( .inputs() .iter() .map(|t| Argument { - type_: clean_middle_ty(t.map_bound(|t| *t), cx, None), + type_: clean_middle_ty(t.map_bound(|t| *t), cx, None, None), name: names .next() .map(|i| i.name) @@ -1190,8 +1198,12 @@ fn clean_trait_item<'tcx>(trait_item: &hir::TraitItem<'tcx>, cx: &mut DocContext hir::TraitItemKind::Type(bounds, Some(default)) => { let generics = enter_impl_trait(cx, |cx| clean_generics(trait_item.generics, cx)); let bounds = bounds.iter().filter_map(|x| clean_generic_bound(x, cx)).collect(); - let item_type = - clean_middle_ty(ty::Binder::dummy(hir_ty_to_ty(cx.tcx, default)), cx, None); + let item_type = clean_middle_ty( + ty::Binder::dummy(hir_ty_to_ty(cx.tcx, default)), + cx, + None, + None, + ); AssocTypeItem( Box::new(Typedef { type_: clean_ty(default, cx), @@ -1230,8 +1242,12 @@ pub(crate) fn clean_impl_item<'tcx>( hir::ImplItemKind::Type(hir_ty) => { let type_ = clean_ty(hir_ty, cx); let generics = clean_generics(impl_.generics, cx); - let item_type = - clean_middle_ty(ty::Binder::dummy(hir_ty_to_ty(cx.tcx, hir_ty)), cx, None); + let item_type = clean_middle_ty( + ty::Binder::dummy(hir_ty_to_ty(cx.tcx, hir_ty)), + cx, + None, + None, + ); AssocTypeItem( Box::new(Typedef { type_, generics, item_type: Some(item_type) }), Vec::new(), @@ -1254,6 +1270,7 @@ pub(crate) fn clean_middle_assoc_item<'tcx>( ty::Binder::dummy(tcx.type_of(assoc_item.def_id).subst_identity()), cx, Some(assoc_item.def_id), + None, ); let provided = match assoc_item.container { @@ -1447,6 +1464,7 @@ pub(crate) fn clean_middle_assoc_item<'tcx>( ty::Binder::dummy(tcx.type_of(assoc_item.def_id).subst_identity()), cx, Some(assoc_item.def_id), + None, ), generics, // FIXME: should we obtain the Type from HIR and pass it on here? @@ -1465,6 +1483,7 @@ pub(crate) fn clean_middle_assoc_item<'tcx>( ty::Binder::dummy(tcx.type_of(assoc_item.def_id).subst_identity()), cx, Some(assoc_item.def_id), + None, ), generics: Generics { params: ThinVec::new(), @@ -1510,7 +1529,7 @@ fn clean_qpath<'tcx>(hir_ty: &hir::Ty<'tcx>, cx: &mut DocContext<'tcx>) -> Type if !ty.has_escaping_bound_vars() && let Some(normalized_value) = normalize(cx, ty::Binder::dummy(ty)) { - return clean_middle_ty(normalized_value, cx, None); + return clean_middle_ty(normalized_value, cx, None, None); } let trait_segments = &p.segments[..p.segments.len() - 1]; @@ -1738,11 +1757,174 @@ fn normalize<'tcx>( } } +fn clean_trait_object_lifetime_bound<'tcx>( + region: ty::Region<'tcx>, + container: Option>, + trait_: DefId, + substs: ty::Binder<'tcx, &ty::List>>, + tcx: TyCtxt<'tcx>, +) -> Option { + if can_elide_trait_object_lifetime_bound(region, container, trait_, substs, tcx) { + return None; + } + + // Since there is a semantic difference between an implicitly elided (i.e. "defaulted") object + // lifetime and an explicitly elided object lifetime (`'_`), we intentionally don't hide the + // latter contrary to `clean_middle_region`. + match *region { + ty::ReStatic => Some(Lifetime::statik()), + ty::ReEarlyBound(region) if region.name != kw::Empty => Some(Lifetime(region.name)), + ty::ReLateBound(_, ty::BoundRegion { kind: ty::BrNamed(_, name), .. }) + if name != kw::Empty => + { + Some(Lifetime(name)) + } + ty::ReEarlyBound(_) + | ty::ReLateBound(..) + | ty::ReFree(_) + | ty::ReVar(_) + | ty::RePlaceholder(_) + | ty::ReErased + | ty::ReError(_) => None, + } +} + +fn can_elide_trait_object_lifetime_bound<'tcx>( + region: ty::Region<'tcx>, + container: Option>, + trait_: DefId, + substs: ty::Binder<'tcx, &ty::List>>, + tcx: TyCtxt<'tcx>, +) -> bool { + // Below we quote extracts from https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes + + // > If the trait object is used as a type argument of a generic type then the containing type is + // > first used to try to infer a bound. + let default = container + .map_or(ObjectLifetimeDefault::Empty, |container| container.object_lifetime_default(tcx)); + + // > If there is a unique bound from the containing type then that is the default + // If there is a default object lifetime and the given region is lexically equal to it, elide it. + match default { + ObjectLifetimeDefault::Static => return *region == ty::ReStatic, + // FIXME(fmease): Don't compare lexically but respect de Bruijn indices etc. to handle shadowing correctly. + ObjectLifetimeDefault::Arg(default) => return region.get_name() == default.get_name(), + // > If there is more than one bound from the containing type then an explicit bound must be specified + // Due to ambiguity there is no default trait-object lifetime and thus elision is impossible. + // Don't elide the lifetime. + ObjectLifetimeDefault::Ambiguous => return false, + // There is no meaningful bound. Further processing is needed... + ObjectLifetimeDefault::Empty => {} + } + + // We filter out any escaping regions below, thus it's fine to skip the binder. + let substs = substs.skip_binder(); + + // > If neither of those rules apply, then the bounds on the trait are used: + let mut trait_regions: Vec<_> = tcx + .predicates_of(trait_) + .predicates + .iter() + .filter_map(|(pred, _)| { + // Look for bounds of the form `Self: 'a` for any region `'a`. + if let ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate(ty, region))) = pred.kind().skip_binder() + && let ty::Param(param) = ty.kind() + && param.name == kw::SelfUpper + { + Some(ty::EarlyBinder::bind(region).subst(tcx, tcx.mk_substs_trait(ty, substs))) + .filter(|region| !region.has_escaping_bound_vars()) + } else { + None + } + }) + .collect(); + + // As a result of the substitutions above, we might be left with duplicate regions. + // Consider `<'a, 'b> Self: 'a + 'b` with substitution `<'r, 'r>`. Deduplicate. + trait_regions.dedup(); + + // > If 'static is used for any lifetime bound then 'static is used. + // If the list contains `'static`, throw out everyhing else as it outlives any of them. + if let Some(index) = trait_regions.iter().position(|region| region.is_static()) { + let static_ = trait_regions.swap_remove(index); + trait_regions.clear(); + trait_regions.push(static_); + } + + match *trait_regions { + // > If the trait has no lifetime bounds, then the lifetime is inferred in expressions + // > and is 'static outside of expressions. + // FIXME: If we are in an expression context (i.e. fn bodies and const exprs) then the default is + // `'_` and not `'static`. Only if we are in a non-expression one, the default is `'static`. + // Note however that at the time of this writing it should be fine to disregard this subtlety + // as we neither render const exprs faithfully anyway (hiding them in some places or using `_` instead) + // nor show the contents of fn bodies. + [] => *region == ty::ReStatic, + // > If the trait is defined with a single lifetime bound then that bound is used. + // FIXME(fmease): Don't compare lexically but respect de Bruijn indices etc. to handle shadowing correctly. + [trait_region] => trait_region.get_name() == region.get_name(), + // There are several distinct trait regions and none are `'static` (thanks to the preprocessing above). + // Due to ambiguity there is no default trait-object lifetime and thus elision is impossible. + // Don't elide the lifetime. + _ => false, + } +} + +#[derive(Debug)] +pub(crate) enum ContainerTy<'tcx> { + Ref(ty::Region<'tcx>), + Regular { ty: DefId, substs: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>, arg: usize }, +} + +impl<'tcx> ContainerTy<'tcx> { + fn object_lifetime_default(self, tcx: TyCtxt<'tcx>) -> ObjectLifetimeDefault<'tcx> { + match self { + Self::Ref(region) => ObjectLifetimeDefault::Arg(region), + Self::Regular { ty: container, substs, arg: index } => { + let (DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::TyAlias + | DefKind::Trait + | DefKind::AssocTy + | DefKind::Variant) = tcx.def_kind(container) + else { + return ObjectLifetimeDefault::Empty; + }; + + let generics = tcx.generics_of(container); + let param = generics.params[index].def_id; + let default = tcx.object_lifetime_default(param); + + match default { + rbv::ObjectLifetimeDefault::Param(lifetime) => { + let index = generics.param_def_id_to_index[&lifetime]; + let arg = substs.skip_binder()[index as usize].expect_region(); + ObjectLifetimeDefault::Arg(arg) + } + rbv::ObjectLifetimeDefault::Empty => ObjectLifetimeDefault::Empty, + rbv::ObjectLifetimeDefault::Static => ObjectLifetimeDefault::Static, + rbv::ObjectLifetimeDefault::Ambiguous => ObjectLifetimeDefault::Ambiguous, + } + } + } + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) enum ObjectLifetimeDefault<'tcx> { + Empty, + Static, + Ambiguous, + Arg(ty::Region<'tcx>), +} + #[instrument(level = "trace", skip(cx), ret)] pub(crate) fn clean_middle_ty<'tcx>( bound_ty: ty::Binder<'tcx, Ty<'tcx>>, cx: &mut DocContext<'tcx>, parent_def_id: Option, + container: Option>, ) -> Type { let bound_ty = normalize(cx, bound_ty).unwrap_or(bound_ty); match *bound_ty.skip_binder().kind() { @@ -1753,19 +1935,24 @@ pub(crate) fn clean_middle_ty<'tcx>( ty::Uint(uint_ty) => Primitive(uint_ty.into()), ty::Float(float_ty) => Primitive(float_ty.into()), ty::Str => Primitive(PrimitiveType::Str), - ty::Slice(ty) => Slice(Box::new(clean_middle_ty(bound_ty.rebind(ty), cx, None))), + ty::Slice(ty) => Slice(Box::new(clean_middle_ty(bound_ty.rebind(ty), cx, None, None))), ty::Array(ty, mut n) => { n = n.eval(cx.tcx, ty::ParamEnv::reveal_all()); let n = print_const(cx, n); - Array(Box::new(clean_middle_ty(bound_ty.rebind(ty), cx, None)), n.into()) + Array(Box::new(clean_middle_ty(bound_ty.rebind(ty), cx, None, None)), n.into()) } ty::RawPtr(mt) => { - RawPointer(mt.mutbl, Box::new(clean_middle_ty(bound_ty.rebind(mt.ty), cx, None))) + RawPointer(mt.mutbl, Box::new(clean_middle_ty(bound_ty.rebind(mt.ty), cx, None, None))) } ty::Ref(r, ty, mutbl) => BorrowedRef { lifetime: clean_middle_region(r), mutability: mutbl, - type_: Box::new(clean_middle_ty(bound_ty.rebind(ty), cx, None)), + type_: Box::new(clean_middle_ty( + bound_ty.rebind(ty), + cx, + None, + Some(ContainerTy::Ref(r)), + )), }, ty::FnDef(..) | ty::FnPtr(_) => { // FIXME: should we merge the outer and inner binders somehow? @@ -1817,10 +2004,8 @@ pub(crate) fn clean_middle_ty<'tcx>( inline::record_extern_fqn(cx, did, ItemType::Trait); - // FIXME(fmease): Hide the trait-object lifetime bound if it coincides with its default - // to partially address #44306. Follow the rules outlined at - // https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes - let lifetime = clean_middle_region(*reg); + let lifetime = clean_trait_object_lifetime_bound(*reg, container, did, substs, cx.tcx); + let mut bounds = dids .map(|did| { let empty = ty::Binder::dummy(InternalSubsts::empty()); @@ -1869,7 +2054,7 @@ pub(crate) fn clean_middle_ty<'tcx>( DynTrait(bounds, lifetime) } ty::Tuple(t) => { - Tuple(t.iter().map(|t| clean_middle_ty(bound_ty.rebind(t), cx, None)).collect()) + Tuple(t.iter().map(|t| clean_middle_ty(bound_ty.rebind(t), cx, None, None)).collect()) } ty::Alias(ty::Projection, ref data) => { @@ -1878,7 +2063,7 @@ pub(crate) fn clean_middle_ty<'tcx>( ty::Alias(ty::Inherent, alias_ty) => { let alias_ty = bound_ty.rebind(alias_ty); - let self_type = clean_middle_ty(alias_ty.map_bound(|ty| ty.self_ty()), cx, None); + let self_type = clean_middle_ty(alias_ty.map_bound(|ty| ty.self_ty()), cx, None, None); Type::QPath(Box::new(QPathData { assoc: PathSegment { @@ -1888,6 +2073,7 @@ pub(crate) fn clean_middle_ty<'tcx>( cx, alias_ty.map_bound(|ty| ty.substs.as_slice()), true, + None, ) .into(), bindings: Default::default(), @@ -2023,6 +2209,7 @@ pub(crate) fn clean_middle_field<'tcx>(field: &ty::FieldDef, cx: &mut DocContext ty::Binder::dummy(cx.tcx.type_of(field.did).subst_identity()), cx, Some(field.did), + None, ), cx, ) @@ -2314,7 +2501,12 @@ fn clean_maybe_renamed_item<'tcx>( ItemKind::TyAlias(hir_ty, generics) => { *cx.current_type_aliases.entry(def_id).or_insert(0) += 1; let rustdoc_ty = clean_ty(hir_ty, cx); - let ty = clean_middle_ty(ty::Binder::dummy(hir_ty_to_ty(cx.tcx, hir_ty)), cx, None); + let ty = clean_middle_ty( + ty::Binder::dummy(hir_ty_to_ty(cx.tcx, hir_ty)), + cx, + None, + None, + ); let generics = clean_generics(generics, cx); if let Some(count) = cx.current_type_aliases.get_mut(&def_id) { *count -= 1; @@ -2418,6 +2610,7 @@ fn clean_impl<'tcx>( ty::Binder::dummy(tcx.type_of(def_id).subst_identity()), cx, Some(def_id.to_def_id()), + None, )), _ => None, }); diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 366f93952963f..294de12cea8e2 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -73,8 +73,9 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate { pub(crate) fn substs_to_args<'tcx>( cx: &mut DocContext<'tcx>, - substs: ty::Binder<'tcx, &[ty::subst::GenericArg<'tcx>]>, + substs: ty::Binder<'tcx, &'tcx [ty::subst::GenericArg<'tcx>]>, mut skip_first: bool, + container: Option, ) -> Vec { let mut ret_val = Vec::with_capacity(substs.skip_binder().len().saturating_sub(if skip_first { @@ -82,19 +83,29 @@ pub(crate) fn substs_to_args<'tcx>( } else { 0 })); - ret_val.extend(substs.iter().filter_map(|kind| match kind.skip_binder().unpack() { - GenericArgKind::Lifetime(lt) => { - Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided()))) - } - GenericArgKind::Type(_) if skip_first => { - skip_first = false; - None - } - GenericArgKind::Type(ty) => { - Some(GenericArg::Type(clean_middle_ty(kind.rebind(ty), cx, None))) - } - GenericArgKind::Const(ct) => { - Some(GenericArg::Const(Box::new(clean_middle_const(kind.rebind(ct), cx)))) + + ret_val.extend(substs.iter().enumerate().filter_map(|(index, kind)| { + match kind.skip_binder().unpack() { + GenericArgKind::Lifetime(lt) => { + Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided()))) + } + GenericArgKind::Type(_) if skip_first => { + skip_first = false; + None + } + GenericArgKind::Type(ty) => Some(GenericArg::Type(clean_middle_ty( + kind.rebind(ty), + cx, + None, + container.map(|container| crate::clean::ContainerTy::Regular { + ty: container, + substs, + arg: index, + }), + ))), + GenericArgKind::Const(ct) => { + Some(GenericArg::Const(Box::new(clean_middle_const(kind.rebind(ct), cx)))) + } } })); ret_val @@ -107,7 +118,7 @@ fn external_generic_args<'tcx>( bindings: ThinVec, substs: ty::Binder<'tcx, SubstsRef<'tcx>>, ) -> GenericArgs { - let args = substs_to_args(cx, substs.map_bound(|substs| &substs[..]), has_self); + let args = substs_to_args(cx, substs.map_bound(|substs| &substs[..]), has_self, Some(did)); if cx.tcx.fn_trait_kind_from_def_id(did).is_some() { let ty = substs @@ -118,7 +129,7 @@ fn external_generic_args<'tcx>( let inputs = // The trait's first substitution is the one after self, if there is one. match ty.skip_binder().kind() { - ty::Tuple(tys) => tys.iter().map(|t| clean_middle_ty(ty.rebind(t), cx, None)).collect::>().into(), + ty::Tuple(tys) => tys.iter().map(|t| clean_middle_ty(ty.rebind(t), cx, None, None)).collect::>().into(), _ => return GenericArgs::AngleBracketed { args: args.into(), bindings }, }; let output = bindings.into_iter().next().and_then(|binding| match binding.kind { diff --git a/tests/rustdoc/assoc-consts.rs b/tests/rustdoc/assoc-consts.rs index 68a11c57b5292..08dfa879d4363 100644 --- a/tests/rustdoc/assoc-consts.rs +++ b/tests/rustdoc/assoc-consts.rs @@ -46,7 +46,6 @@ pub fn f(_: &(ToString + 'static)) {} impl Bar { // @has assoc_consts/struct.Bar.html '//*[@id="associatedconstant.F"]' \ // "const F: fn(_: &(dyn ToString + 'static))" - // FIXME(fmease): Hide default lifetime, render "const F: fn(_: &dyn ToString)" pub const F: fn(_: &(ToString + 'static)) = f; } diff --git a/tests/rustdoc/inline_cross/auxiliary/dyn_trait.rs b/tests/rustdoc/inline_cross/auxiliary/dyn_trait.rs index 9ac2e3d96debd..644d0699e9d07 100644 --- a/tests/rustdoc/inline_cross/auxiliary/dyn_trait.rs +++ b/tests/rustdoc/inline_cross/auxiliary/dyn_trait.rs @@ -1,3 +1,5 @@ +// ignore-tidy-linelength + pub type Ty0 = dyn for<'any> FnOnce(&'any str) -> bool; pub type Ty1<'obj> = dyn std::fmt::Display + 'obj; @@ -6,12 +8,60 @@ pub type Ty2 = dyn for<'a, 'r> Container<'r, Item<'a, 'static> = ()>; pub type Ty3<'s> = &'s dyn ToString; -pub fn func0(_: &(dyn Fn() + '_)) {} - -pub fn func1<'func>(_: &(dyn Fn() + 'func)) {} - pub trait Container<'r> { type Item<'a, 'ctx>; } -pub trait Shape<'a> {} +// Trait-object types inside of a container type that has lifetime bounds ("wrapped"). + +pub fn late_bound_wrapped_elided(_: &(dyn Fn() + '_)) {} +pub fn late_bound_wrapped_late0<'f>(_: &mut (dyn Fn() + 'f)) {} +pub fn late_bound_wrapped_defaulted0<'f>(_: &'f mut dyn Fn()) {} +pub type EarlyBoundWrappedDefaulted0<'x> = std::cell::Ref<'x, dyn Trait>; +pub type EarlyBoundWrappedDefaulted1<'x> = &'x dyn Trait; +pub type EarlyBoundWrappedEarly<'x, 'y> = std::cell::Ref<'x, dyn Trait + 'y>; +pub type EarlyBoundWrappedStatic<'x> = std::cell::Ref<'x, dyn Trait + 'static>; +pub fn late_bound_wrapped_defaulted1<'l>(_: std::cell::Ref<'l, dyn Trait>) {} +pub fn late_bound_wrapped_late1<'l, 'm>(_: std::cell::Ref<'l, dyn Trait + 'm>) {} +pub fn late_bound_wrapped_early<'e, 'l>(_: std::cell::Ref<'l, dyn Trait + 'e>) where 'e: {} // `'e` is early-bound +pub fn elided_bound_wrapped_defaulted(_: std::cell::Ref<'_, dyn Trait>) {} +pub type StaticBoundWrappedDefaulted0 = std::cell::Ref<'static, dyn Trait>; +pub type StaticBoundWrappedDefaulted1 = &'static dyn Trait; +pub type AmbiguousBoundWrappedEarly0<'r, 's> = AmbiguousBoundWrapper<'s, 'r, dyn Trait + 's>; +pub type AmbiguousBoundWrappedEarly1<'r, 's> = AmbiguousBoundWrapper<'s, 'r, dyn Trait + 'r>; +pub type AmbiguousBoundWrappedStatic<'q> = AmbiguousBoundWrapper<'q, 'q, dyn Trait + 'static>; + +// Trait-object types inside of a container type that doesn't have lifetime bounds ("wrapped"). + +pub type NoBoundsWrappedDefaulted = Box; +pub type NoBoundsWrappedEarly<'e> = Box; +pub fn no_bounds_wrapped_late<'l>(_: Box) {} +pub fn no_bounds_wrapped_elided(_: Box) {} + +// Trait-object types outside of a container (“bare”). + +pub type BareNoBoundsDefaulted = dyn Trait; +pub type BareNoBoundsEarly<'p> = dyn Trait + 'p; +pub type BareEarlyBoundDefaulted0<'u> = dyn EarlyBoundTrait0<'u>; +pub type BareEarlyBoundDefaulted1 = dyn for<'any> EarlyBoundTrait0<'any>; +pub type BareEarlyBoundDefaulted2<'w> = dyn EarlyBoundTrait1<'static, 'w>; +pub type BareEarlyBoundEarly<'i, 'j> = dyn EarlyBoundTrait0<'i> + 'j; +pub type BareEarlyBoundStatic<'i> = dyn EarlyBoundTrait0<'i> + 'static; +pub type BareStaticBoundDefaulted = dyn StaticBoundTrait; +pub type BareHigherRankedBoundDefaulted0 = dyn HigherRankedBoundTrait0; +pub type BareHigherRankedBoundDefaulted1<'r> = dyn HigherRankedBoundTrait1<'r>; +pub type BareAmbiguousBoundEarly0<'m, 'n> = dyn AmbiguousBoundTrait<'m, 'n> + 'm; +pub type BareAmbiguousBoundEarly1<'m, 'n> = dyn AmbiguousBoundTrait<'m, 'n> + 'n; +pub type BareAmbiguousBoundStatic<'o> = dyn AmbiguousBoundTrait<'o, 'o> + 'static; + +// Trait and container definitions. + +pub trait Trait {} // no bounds +pub trait EarlyBoundTrait0<'b>: 'b {} +pub trait EarlyBoundTrait1<'unused, 'c>: 'c {} +pub trait StaticBoundTrait: 'static {} +pub trait HigherRankedBoundTrait0 where for<'a> Self: 'a {} +pub trait HigherRankedBoundTrait1<'e> where for<'l> Self: 'e + 'l {} +pub trait AmbiguousBoundTrait<'a, 'b>: 'a + 'b {} + +pub struct AmbiguousBoundWrapper<'a, 'b, T: ?Sized + 'a + 'b>(&'a T, &'b T); diff --git a/tests/rustdoc/inline_cross/dyn_trait.rs b/tests/rustdoc/inline_cross/dyn_trait.rs index 649d98f71396a..1de01af83d13e 100644 --- a/tests/rustdoc/inline_cross/dyn_trait.rs +++ b/tests/rustdoc/inline_cross/dyn_trait.rs @@ -1,31 +1,130 @@ #![crate_name = "user"] +// In each test case, we include the trailing semicolon to ensure that nothing extra comes +// after the type like an unwanted outlives-bound. + // aux-crate:dyn_trait=dyn_trait.rs // edition:2021 // @has user/type.Ty0.html -// @has - '//*[@class="rust item-decl"]//code' "dyn for<'any> FnOnce(&'any str) -> bool + 'static" -// FIXME(fmease): Hide default lifetime bound `'static` +// @has - '//*[@class="rust item-decl"]//code' "dyn for<'any> FnOnce(&'any str) -> bool;" pub use dyn_trait::Ty0; // @has user/type.Ty1.html -// @has - '//*[@class="rust item-decl"]//code' "dyn Display + 'obj" +// @has - '//*[@class="rust item-decl"]//code' "dyn Display + 'obj;" pub use dyn_trait::Ty1; // @has user/type.Ty2.html -// @has - '//*[@class="rust item-decl"]//code' "dyn for<'a, 'r> Container<'r, Item<'a, 'static> = ()>" +// @has - '//*[@class="rust item-decl"]//code' "dyn for<'a, 'r> Container<'r, Item<'a, 'static> = ()>;" pub use dyn_trait::Ty2; // @has user/type.Ty3.html -// @has - '//*[@class="rust item-decl"]//code' "&'s (dyn ToString + 's)" -// FIXME(fmease): Hide default lifetime bound, render "&'s dyn ToString" +// @has - '//*[@class="rust item-decl"]//code' "&'s dyn ToString;" pub use dyn_trait::Ty3; -// @has user/fn.func0.html -// @has - '//pre[@class="rust item-decl"]' "func0(_: &dyn Fn())" -// FIXME(fmease): Show placeholder-lifetime bound, render "func0(_: &(dyn Fn() + '_))" -pub use dyn_trait::func0; +// Below we check if we correctly elide trait-object lifetime bounds if they coincide with their +// default (known as "object lifetime default" or "default trait object lifetime"). + +// @has user/fn.lbwel.html +// @has - '//pre[@class="rust item-decl"]' "lbwel(_: &dyn Fn())" +pub use dyn_trait::late_bound_wrapped_elided as lbwel; +// @has user/fn.lbwl0.html +// has - '//pre[@class="rust item-decl"]' "lbwl0<'f>(_: &mut (dyn Fn() + 'f))" +pub use dyn_trait::late_bound_wrapped_late0 as lbwl0; +// @has user/fn.lbwd0.html +// has - '//pre[@class="rust item-decl"]' "lbwd0<'f>(_: &'f mut dyn Fn())" +pub use dyn_trait::late_bound_wrapped_defaulted0 as lbwd0; +// @has user/type.EarlyBoundWrappedDefaulted0.html +// @has - '//*[@class="rust item-decl"]//code' "Ref<'x, dyn Trait>;" +pub use dyn_trait::EarlyBoundWrappedDefaulted0; +// @has user/type.EarlyBoundWrappedDefaulted1.html +// @has - '//*[@class="rust item-decl"]//code' "&'x dyn Trait;" +pub use dyn_trait::EarlyBoundWrappedDefaulted1; +// @has user/type.EarlyBoundWrappedEarly.html +// @has - '//*[@class="rust item-decl"]//code' "Ref<'x, dyn Trait + 'y>" +pub use dyn_trait::EarlyBoundWrappedEarly; +// @has user/type.EarlyBoundWrappedStatic.html +// @has - '//*[@class="rust item-decl"]//code' "Ref<'x, dyn Trait + 'static>" +pub use dyn_trait::EarlyBoundWrappedStatic; +// @has user/fn.lbwd1.html +// @has - '//pre[@class="rust item-decl"]' "lbwd1<'l>(_: Ref<'l, dyn Trait>)" +pub use dyn_trait::late_bound_wrapped_defaulted1 as lbwd1; +// @has user/fn.lbwl1.html +// @has - '//pre[@class="rust item-decl"]' "lbwl1<'l, 'm>(_: Ref<'l, dyn Trait + 'm>)" +pub use dyn_trait::late_bound_wrapped_late1 as lbwl1; +// @has user/fn.lbwe.html +// @has - '//pre[@class="rust item-decl"]' "lbwe<'e, 'l>(_: Ref<'l, dyn Trait + 'e>)" +pub use dyn_trait::late_bound_wrapped_early as lbwe; +// @has user/fn.ebwd.html +// @has - '//pre[@class="rust item-decl"]' "ebwd(_: Ref<'_, dyn Trait>)" +pub use dyn_trait::elided_bound_wrapped_defaulted as ebwd; +// @has user/type.StaticBoundWrappedDefaulted0.html +// @has - '//*[@class="rust item-decl"]//code' "Ref<'static, dyn Trait>;" +pub use dyn_trait::StaticBoundWrappedDefaulted0; +// @has user/type.StaticBoundWrappedDefaulted1.html +// @has - '//*[@class="rust item-decl"]//code' "&'static dyn Trait;" +pub use dyn_trait::StaticBoundWrappedDefaulted1; +// @has user/type.AmbiguousBoundWrappedEarly0.html +// @has - '//*[@class="rust item-decl"]//code' "AmbiguousBoundWrapper<'s, 'r, dyn Trait + 's>;" +pub use dyn_trait::AmbiguousBoundWrappedEarly0; +// @has user/type.AmbiguousBoundWrappedEarly1.html +// @has - '//*[@class="rust item-decl"]//code' "AmbiguousBoundWrapper<'s, 'r, dyn Trait + 'r>;" +pub use dyn_trait::AmbiguousBoundWrappedEarly1; +// @has user/type.AmbiguousBoundWrappedStatic.html +// @has - '//*[@class="rust item-decl"]//code' "AmbiguousBoundWrapper<'q, 'q, dyn Trait + 'static>;" +pub use dyn_trait::AmbiguousBoundWrappedStatic; + +// @has user/type.NoBoundsWrappedDefaulted.html +// @has - '//*[@class="rust item-decl"]//code' "Box;" +pub use dyn_trait::NoBoundsWrappedDefaulted; +// @has user/type.NoBoundsWrappedEarly.html +// @has - '//*[@class="rust item-decl"]//code' "Box;" +pub use dyn_trait::NoBoundsWrappedEarly; +// @has user/fn.nbwl.html +// @has - '//pre[@class="rust item-decl"]' "nbwl<'l>(_: Box)" +pub use dyn_trait::no_bounds_wrapped_late as nbwl; +// @has user/fn.nbwel.html +// @has - '//pre[@class="rust item-decl"]' "nbwel(_: Box)" +// NB: It might seem counterintuitive to display the explicitly elided lifetime `'_` here instead of +// eliding it but this behavior is correct: The default is `'static` here which != `'_`. +pub use dyn_trait::no_bounds_wrapped_elided as nbwel; -// @has user/fn.func1.html -// @has - '//pre[@class="rust item-decl"]' "func1<'func>(_: &(dyn Fn() + 'func))" -pub use dyn_trait::func1; +// @has user/type.BareNoBoundsDefaulted.html +// @has - '//*[@class="rust item-decl"]//code' "dyn Trait;" +pub use dyn_trait::BareNoBoundsDefaulted; +// @has user/type.BareNoBoundsEarly.html +// @has - '//*[@class="rust item-decl"]//code' "dyn Trait + 'p;" +pub use dyn_trait::BareNoBoundsEarly; +// @has user/type.BareEarlyBoundDefaulted0.html +// @has - '//*[@class="rust item-decl"]//code' "dyn EarlyBoundTrait0<'u>;" +pub use dyn_trait::BareEarlyBoundDefaulted0; +// @has user/type.BareEarlyBoundDefaulted1.html +// @has - '//*[@class="rust item-decl"]//code' "dyn for<'any> EarlyBoundTrait0<'any>;" +pub use dyn_trait::BareEarlyBoundDefaulted1; +// @has user/type.BareEarlyBoundDefaulted2.html +// @has - '//*[@class="rust item-decl"]//code' "dyn EarlyBoundTrait1<'static, 'w>;" +pub use dyn_trait::BareEarlyBoundDefaulted2; +// @has user/type.BareEarlyBoundEarly.html +// @has - '//*[@class="rust item-decl"]//code' "dyn EarlyBoundTrait0<'i> + 'j;" +pub use dyn_trait::BareEarlyBoundEarly; +// @has user/type.BareEarlyBoundStatic.html +// @has - '//*[@class="rust item-decl"]//code' "dyn EarlyBoundTrait0<'i> + 'static;" +pub use dyn_trait::BareEarlyBoundStatic; +// @has user/type.BareStaticBoundDefaulted.html +// @has - '//*[@class="rust item-decl"]//code' "dyn StaticBoundTrait;" +pub use dyn_trait::BareStaticBoundDefaulted; +// @has user/type.BareHigherRankedBoundDefaulted0.html +// @has - '//*[@class="rust item-decl"]//code' "dyn HigherRankedBoundTrait0;" +pub use dyn_trait::BareHigherRankedBoundDefaulted0; +// @has user/type.BareHigherRankedBoundDefaulted1.html +// @has - '//*[@class="rust item-decl"]//code' "dyn HigherRankedBoundTrait1<'r>;" +pub use dyn_trait::BareHigherRankedBoundDefaulted1; +// @has user/type.BareAmbiguousBoundEarly0.html +// @has - '//*[@class="rust item-decl"]//code' "dyn AmbiguousBoundTrait<'m, 'n> + 'm;" +pub use dyn_trait::BareAmbiguousBoundEarly0; +// @has user/type.BareAmbiguousBoundEarly1.html +// @has - '//*[@class="rust item-decl"]//code' "dyn AmbiguousBoundTrait<'m, 'n> + 'n;" +pub use dyn_trait::BareAmbiguousBoundEarly1; +// @has user/type.BareAmbiguousBoundStatic.html +// @has - '//*[@class="rust item-decl"]//code' "dyn AmbiguousBoundTrait<'o, 'o> + 'static;" +pub use dyn_trait::BareAmbiguousBoundStatic; From 5b5d84fd6a36a8ea4f39e3b730262321cc8f8622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 6 Jun 2023 20:17:07 +0200 Subject: [PATCH 2/2] use wf::object_region_bounds --- src/librustdoc/clean/mod.rs | 51 +++++++------------------------------ 1 file changed, 9 insertions(+), 42 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 526c9a4a35fae..0c7950bfd3ddd 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -31,6 +31,7 @@ use rustc_middle::{bug, span_bug}; use rustc_span::hygiene::{AstPass, MacroKind}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{self, ExpnKind}; +use rustc_trait_selection::traits::wf::object_region_bounds; use std::borrow::Cow; use std::collections::hash_map::Entry; @@ -1760,11 +1761,10 @@ fn normalize<'tcx>( fn clean_trait_object_lifetime_bound<'tcx>( region: ty::Region<'tcx>, container: Option>, - trait_: DefId, - substs: ty::Binder<'tcx, &ty::List>>, + preds: &'tcx ty::List>, tcx: TyCtxt<'tcx>, ) -> Option { - if can_elide_trait_object_lifetime_bound(region, container, trait_, substs, tcx) { + if can_elide_trait_object_lifetime_bound(region, container, preds, tcx) { return None; } @@ -1792,8 +1792,7 @@ fn clean_trait_object_lifetime_bound<'tcx>( fn can_elide_trait_object_lifetime_bound<'tcx>( region: ty::Region<'tcx>, container: Option>, - trait_: DefId, - substs: ty::Binder<'tcx, &ty::List>>, + preds: &'tcx ty::List>, tcx: TyCtxt<'tcx>, ) -> bool { // Below we quote extracts from https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes @@ -1817,41 +1816,8 @@ fn can_elide_trait_object_lifetime_bound<'tcx>( ObjectLifetimeDefault::Empty => {} } - // We filter out any escaping regions below, thus it's fine to skip the binder. - let substs = substs.skip_binder(); - // > If neither of those rules apply, then the bounds on the trait are used: - let mut trait_regions: Vec<_> = tcx - .predicates_of(trait_) - .predicates - .iter() - .filter_map(|(pred, _)| { - // Look for bounds of the form `Self: 'a` for any region `'a`. - if let ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate(ty, region))) = pred.kind().skip_binder() - && let ty::Param(param) = ty.kind() - && param.name == kw::SelfUpper - { - Some(ty::EarlyBinder::bind(region).subst(tcx, tcx.mk_substs_trait(ty, substs))) - .filter(|region| !region.has_escaping_bound_vars()) - } else { - None - } - }) - .collect(); - - // As a result of the substitutions above, we might be left with duplicate regions. - // Consider `<'a, 'b> Self: 'a + 'b` with substitution `<'r, 'r>`. Deduplicate. - trait_regions.dedup(); - - // > If 'static is used for any lifetime bound then 'static is used. - // If the list contains `'static`, throw out everyhing else as it outlives any of them. - if let Some(index) = trait_regions.iter().position(|region| region.is_static()) { - let static_ = trait_regions.swap_remove(index); - trait_regions.clear(); - trait_regions.push(static_); - } - - match *trait_regions { + match *object_region_bounds(tcx, preds) { // > If the trait has no lifetime bounds, then the lifetime is inferred in expressions // > and is 'static outside of expressions. // FIXME: If we are in an expression context (i.e. fn bodies and const exprs) then the default is @@ -1861,9 +1827,10 @@ fn can_elide_trait_object_lifetime_bound<'tcx>( // nor show the contents of fn bodies. [] => *region == ty::ReStatic, // > If the trait is defined with a single lifetime bound then that bound is used. + // > If 'static is used for any lifetime bound then 'static is used. // FIXME(fmease): Don't compare lexically but respect de Bruijn indices etc. to handle shadowing correctly. - [trait_region] => trait_region.get_name() == region.get_name(), - // There are several distinct trait regions and none are `'static` (thanks to the preprocessing above). + [object_region] => object_region.get_name() == region.get_name(), + // There are several distinct trait regions and none are `'static`. // Due to ambiguity there is no default trait-object lifetime and thus elision is impossible. // Don't elide the lifetime. _ => false, @@ -2004,7 +1971,7 @@ pub(crate) fn clean_middle_ty<'tcx>( inline::record_extern_fqn(cx, did, ItemType::Trait); - let lifetime = clean_trait_object_lifetime_bound(*reg, container, did, substs, cx.tcx); + let lifetime = clean_trait_object_lifetime_bound(*reg, container, obj, cx.tcx); let mut bounds = dids .map(|did| {