Skip to content

Commit

Permalink
Auto merge of #102707 - fmease:rustdoc-render-more-cross-crate-hrtbs-…
Browse files Browse the repository at this point in the history
…properly, r=GuillaumeGomez

rustdoc: render more cross-crate HRTBs properly

Follow-up to #102439.
Render the `for<>` parameter lists of cross-crate higher-rank trait bounds (in where-clauses and in `impl Trait`).

I've added a new field `bound_params` to `clean::WherePredicate::EqPredicate` (mirroring its sibling variant `BoundPredicate`). However, I had to box the existing fields since `EqPredicate` used to be the largest variant (128 bytes on 64-bit systems) and it would only have gotten bigger).
Not sure if you like that approach. As an alternative, I could pass the uncleaned `ty::Predicate` alongside the cleaned `WherePredicate` to the various re-sugaring methods (similar to what `clean::AutoTraitFinder::param_env_to_generics` does).

I haven't yet added the HTML & JSON rendering code for the newly added `bound_params` field since I am waiting for your opinion. Those two rendering code paths should actually be unreachable in practice given we re-sugar all(?) equality predicates to associated type bindings (and arbitrary equality predicates are not part of the Rust surface language at the time of this writing).

If you agree with storing `bound_params` in `EqPredicate`, I think I can use it to greatly simplify the `clean::auto_trait` module (by also using `simplify::merge_bounds`). Maybe I can do that in any case though.

`@rustbot` label T-rustdoc A-cross-crate-reexports
r? `@GuillaumeGomez`
  • Loading branch information
bors committed Oct 6, 2022
2 parents 4bd3078 + 73c239e commit 6b6610b
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 41 deletions.
13 changes: 10 additions & 3 deletions src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,12 @@ where

let mut ty_to_fn: FxHashMap<Type, (PolyTrait, Option<Type>)> = Default::default();

// FIXME: This code shares much of the logic found in `clean_ty_generics` and
// `simplify::where_clause`. Consider deduplicating it to avoid diverging
// implementations.
// Further, the code below does not merge (partially re-sugared) bounds like
// `Tr<A = T>` & `Tr<B = U>` and it does not render higher-ranked parameters
// originating from equality predicates.
for p in clean_where_predicates {
let (orig_p, p) = (p, clean_predicate(p, self.cx));
if p.is_none() {
Expand Down Expand Up @@ -549,8 +555,8 @@ where
WherePredicate::RegionPredicate { lifetime, bounds } => {
lifetime_to_bounds.entry(lifetime).or_default().extend(bounds);
}
WherePredicate::EqPredicate { lhs, rhs } => {
match lhs {
WherePredicate::EqPredicate { lhs, rhs, bound_params } => {
match *lhs {
Type::QPath(box QPathData {
ref assoc, ref self_type, ref trait_, ..
}) => {
Expand Down Expand Up @@ -585,13 +591,14 @@ where
GenericArgs::AngleBracketed { ref mut bindings, .. } => {
bindings.push(TypeBinding {
assoc: assoc.clone(),
kind: TypeBindingKind::Equality { term: rhs },
kind: TypeBindingKind::Equality { term: *rhs },
});
}
GenericArgs::Parenthesized { .. } => {
existing_predicates.push(WherePredicate::EqPredicate {
lhs: lhs.clone(),
rhs,
bound_params,
});
continue; // If something other than a Fn ends up
// with parentheses, leave it alone
Expand Down
62 changes: 45 additions & 17 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@ fn clean_where_predicate<'tcx>(
},

hir::WherePredicate::EqPredicate(ref wrp) => WherePredicate::EqPredicate {
lhs: clean_ty(wrp.lhs_ty, cx),
rhs: clean_ty(wrp.rhs_ty, cx).into(),
lhs: Box::new(clean_ty(wrp.lhs_ty, cx)),
rhs: Box::new(clean_ty(wrp.rhs_ty, cx).into()),
bound_params: Vec::new(),
},
})
}
Expand All @@ -309,7 +310,9 @@ pub(crate) fn clean_predicate<'tcx>(
}
ty::PredicateKind::RegionOutlives(pred) => clean_region_outlives_predicate(pred),
ty::PredicateKind::TypeOutlives(pred) => clean_type_outlives_predicate(pred, cx),
ty::PredicateKind::Projection(pred) => Some(clean_projection_predicate(pred, cx)),
ty::PredicateKind::Projection(pred) => {
Some(clean_projection_predicate(bound_predicate.rebind(pred), cx))
}
ty::PredicateKind::ConstEvaluatable(..) => None,
ty::PredicateKind::WellFormed(..) => None,

Expand Down Expand Up @@ -387,13 +390,25 @@ fn clean_hir_term<'tcx>(term: &hir::Term<'tcx>, cx: &mut DocContext<'tcx>) -> Te
}

fn clean_projection_predicate<'tcx>(
pred: ty::ProjectionPredicate<'tcx>,
pred: ty::Binder<'tcx, ty::ProjectionPredicate<'tcx>>,
cx: &mut DocContext<'tcx>,
) -> WherePredicate {
let ty::ProjectionPredicate { projection_ty, term } = pred;
let late_bound_regions = cx
.tcx
.collect_referenced_late_bound_regions(&pred)
.into_iter()
.filter_map(|br| match br {
ty::BrNamed(_, name) if name != kw::UnderscoreLifetime => Some(Lifetime(name)),
_ => None,
})
.collect();

let ty::ProjectionPredicate { projection_ty, term } = pred.skip_binder();

WherePredicate::EqPredicate {
lhs: clean_projection(projection_ty, cx, None),
rhs: clean_middle_term(term, cx),
lhs: Box::new(clean_projection(projection_ty, cx, None)),
rhs: Box::new(clean_middle_term(term, cx)),
bound_params: late_bound_regions,
}
}

Expand Down Expand Up @@ -655,8 +670,9 @@ fn clean_ty_generics<'tcx>(
})
.collect::<Vec<GenericParamDef>>();

// param index -> [(DefId of trait, associated type name and generics, type)]
let mut impl_trait_proj = FxHashMap::<u32, Vec<(DefId, PathSegment, Ty<'_>)>>::default();
// param index -> [(trait DefId, associated type name & generics, type, higher-ranked params)]
let mut impl_trait_proj =
FxHashMap::<u32, Vec<(DefId, PathSegment, Ty<'_>, Vec<GenericParamDef>)>>::default();

let where_predicates = preds
.predicates
Expand Down Expand Up @@ -715,6 +731,14 @@ fn clean_ty_generics<'tcx>(
trait_did,
name,
rhs.ty().unwrap(),
p.get_bound_params()
.into_iter()
.flatten()
.map(|param| GenericParamDef {
name: param.0,
kind: GenericParamDefKind::Lifetime { outlives: Vec::new() },
})
.collect(),
));
}

Expand All @@ -730,15 +754,19 @@ fn clean_ty_generics<'tcx>(
// Move trait bounds to the front.
bounds.sort_by_key(|b| !matches!(b, GenericBound::TraitBound(..)));

if let crate::core::ImplTraitParam::ParamIndex(idx) = param {
if let Some(proj) = impl_trait_proj.remove(&idx) {
for (trait_did, name, rhs) in proj {
let rhs = clean_middle_ty(rhs, cx, None);
simplify::merge_bounds(cx, &mut bounds, trait_did, name, &Term::Type(rhs));
}
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);
simplify::merge_bounds(
cx,
&mut bounds,
bound_params,
trait_did,
name,
&Term::Type(rhs),
);
}
} else {
unreachable!();
}

cx.impl_trait_bounds.insert(param, bounds);
Expand Down
37 changes: 25 additions & 12 deletions src/librustdoc/clean/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,23 @@ pub(crate) fn where_clauses(cx: &DocContext<'_>, clauses: Vec<WP>) -> Vec<WP> {
WP::RegionPredicate { lifetime, bounds } => {
lifetimes.push((lifetime, bounds));
}
WP::EqPredicate { lhs, rhs } => equalities.push((lhs, rhs)),
WP::EqPredicate { lhs, rhs, bound_params } => equalities.push((lhs, rhs, bound_params)),
}
}

// Look for equality predicates on associated types that can be merged into
// general bound predicates.
equalities.retain(|&(ref lhs, ref rhs)| {
equalities.retain(|&(ref lhs, ref rhs, ref bound_params)| {
let Some((ty, trait_did, name)) = lhs.projection() else { return true; };
// FIXME(fmease): We don't handle HRTBs correctly here.
// Pass `_bound_params` (higher-rank lifetimes) to a modified version of
// `merge_bounds`. That vector is currently always empty though since we
// don't keep track of late-bound lifetimes when cleaning projection
// predicates to cleaned equality predicates while we should first query
// them with `collect_referenced_late_bound_regions` and then store them
// (or something similar). For prior art, see `clean::auto_trait`.
let Some((bounds, _bound_params)) = tybounds.get_mut(ty) else { return true };
merge_bounds(cx, bounds, trait_did, name, rhs)
let Some((bounds, _)) = tybounds.get_mut(ty) else { return true };
let bound_params = bound_params
.into_iter()
.map(|param| clean::GenericParamDef {
name: param.0,
kind: clean::GenericParamDefKind::Lifetime { outlives: Vec::new() },
})
.collect();
merge_bounds(cx, bounds, bound_params, trait_did, name, rhs)
});

// And finally, let's reassemble everything
Expand All @@ -68,13 +68,18 @@ pub(crate) fn where_clauses(cx: &DocContext<'_>, clauses: Vec<WP>) -> Vec<WP> {
bounds,
bound_params,
}));
clauses.extend(equalities.into_iter().map(|(lhs, rhs)| WP::EqPredicate { lhs, rhs }));
clauses.extend(equalities.into_iter().map(|(lhs, rhs, bound_params)| WP::EqPredicate {
lhs,
rhs,
bound_params,
}));
clauses
}

pub(crate) fn merge_bounds(
cx: &clean::DocContext<'_>,
bounds: &mut Vec<clean::GenericBound>,
mut bound_params: Vec<clean::GenericParamDef>,
trait_did: DefId,
assoc: clean::PathSegment,
rhs: &clean::Term,
Expand All @@ -91,6 +96,14 @@ pub(crate) fn merge_bounds(
return false;
}
let last = trait_ref.trait_.segments.last_mut().expect("segments were empty");

trait_ref.generic_params.append(&mut bound_params);
// Since the parameters (probably) originate from `tcx.collect_*_late_bound_regions` which
// returns a hash set, sort them alphabetically to guarantee a stable and deterministic
// output (and to fully deduplicate them).
trait_ref.generic_params.sort_unstable_by(|p, q| p.name.as_str().cmp(q.name.as_str()));
trait_ref.generic_params.dedup_by_key(|p| p.name);

match last.args {
PP::AngleBracketed { ref mut bindings, .. } => {
bindings.push(clean::TypeBinding {
Expand Down
11 changes: 10 additions & 1 deletion src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ impl Lifetime {
pub(crate) enum WherePredicate {
BoundPredicate { ty: Type, bounds: Vec<GenericBound>, bound_params: Vec<Lifetime> },
RegionPredicate { lifetime: Lifetime, bounds: Vec<GenericBound> },
EqPredicate { lhs: Type, rhs: Term },
EqPredicate { lhs: Box<Type>, rhs: Box<Term>, bound_params: Vec<Lifetime> },
}

impl WherePredicate {
Expand All @@ -1361,6 +1361,15 @@ impl WherePredicate {
_ => None,
}
}

pub(crate) fn get_bound_params(&self) -> Option<&[Lifetime]> {
match self {
Self::BoundPredicate { bound_params, .. } | Self::EqPredicate { bound_params, .. } => {
Some(bound_params)
}
_ => None,
}
}
}

#[derive(Clone, PartialEq, Eq, Debug, Hash)]
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ pub(crate) fn print_where_clause<'a, 'tcx: 'a>(
bounds_display.truncate(bounds_display.len() - " + ".len());
write!(f, "{}: {bounds_display}", lifetime.print())
}
clean::WherePredicate::EqPredicate { lhs, rhs } => {
// FIXME(fmease): Render bound params.
clean::WherePredicate::EqPredicate { lhs, rhs, bound_params: _ } => {
if f.alternate() {
write!(f, "{:#} == {:#}", lhs.print(cx), rhs.print(cx))
} else {
Expand Down
5 changes: 3 additions & 2 deletions src/librustdoc/json/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,9 @@ impl FromWithTcx<clean::WherePredicate> for WherePredicate {
lifetime: convert_lifetime(lifetime),
bounds: bounds.into_tcx(tcx),
},
EqPredicate { lhs, rhs } => {
WherePredicate::EqPredicate { lhs: lhs.into_tcx(tcx), rhs: rhs.into_tcx(tcx) }
// FIXME(fmease): Convert bound parameters as well.
EqPredicate { lhs, rhs, bound_params: _ } => {
WherePredicate::EqPredicate { lhs: (*lhs).into_tcx(tcx), rhs: (*rhs).into_tcx(tcx) }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ extern crate assoc_item_trait_bounds_with_bindings as aux;

// FIXME(fmease): Don't render an incorrect `T: ?Sized` where-clause for parameters
// of GATs like `Main::Out{2,4}`. Add a snapshot test once it's fixed.
// FIXME(fmease): Print the `for<>` parameter list in the bounds of
// `Main::Out{6,11,12}`.

// @has main/trait.Main.html
// @has - '//*[@id="associatedtype.Out0"]' 'type Out0: Support<Item = ()>'
Expand All @@ -18,13 +16,14 @@ extern crate assoc_item_trait_bounds_with_bindings as aux;
// @has - '//*[@id="associatedtype.Out3"]' 'type Out3: Support<Produce<()> = bool>'
// @has - '//*[@id="associatedtype.Out4"]' 'type Out4<T>: Support<Produce<T> = T>'
// @has - '//*[@id="associatedtype.Out5"]' "type Out5: Support<Output<'static> = &'static ()>"
// @has - '//*[@id="associatedtype.Out6"]' "type Out6: Support<Output<'a> = &'a ()>"
// @has - '//*[@id="associatedtype.Out6"]' "type Out6: for<'a> Support<Output<'a> = &'a ()>"
// @has - '//*[@id="associatedtype.Out7"]' "type Out7: Support<Item = String, Produce<i32> = u32> + Unrelated"
// @has - '//*[@id="associatedtype.Out8"]' "type Out8: Unrelated + Protocol<i16, Q1 = u128, Q0 = ()>"
// @has - '//*[@id="associatedtype.Out9"]' "type Out9: FnMut(i32) -> bool + Clone"
// @has - '//*[@id="associatedtype.Out10"]' "type Out10<'q>: Support<Output<'q> = ()>"
// @has - '//*[@id="associatedtype.Out11"]' "type Out11: Helper<A<'s> = &'s (), B<'r> = ()>"
// @has - '//*[@id="associatedtype.Out12"]' "type Out12: Helper<B<'w> = Cow<'w, str>, A<'w> = bool>"
// @has - '//*[@id="associatedtype.Out11"]' "type Out11: for<'r, 's> Helper<A<'s> = &'s (), B<'r> = ()>"
// @has - '//*[@id="associatedtype.Out12"]' "type Out12: for<'w> Helper<B<'w> = Cow<'w, str>, A<'w> = bool>"
// @has - '//*[@id="associatedtype.Out13"]' "type Out13: for<'fst, 'snd> Aid<'snd, Result<'fst> = &'fst mut str>"
//
// Snapshots: Check that we do not render any where-clauses for those associated types since all of
// the trait bounds contained within were moved to the bounds of the respective item.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub trait Main {
type Out10<'q>: Support<Output<'q> = ()>;
type Out11: for<'r, 's> Helper<A<'s> = &'s (), B<'r> = ()>;
type Out12: for<'w> Helper<B<'w> = std::borrow::Cow<'w, str>, A<'w> = bool>;
type Out13: for<'fst, 'snd> Aid<'snd, Result<'fst> = &'fst mut str>;

fn make<F>(_: F, _: impl FnMut(&str) -> bool)
where
Expand All @@ -38,3 +39,7 @@ pub trait Helper {
type A<'q>;
type B<'q>;
}

pub trait Aid<'src> {
type Result<'inter>;
}
13 changes: 13 additions & 0 deletions src/test/rustdoc/inline_cross/auxiliary/impl_trait_aux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,19 @@ pub fn func3(_x: impl Iterator<Item = impl Iterator<Item = u8>> + Clone) {}

pub fn func4<T: Iterator<Item = impl Clone>>(_x: T) {}

pub fn func5(
_f: impl for<'any> Fn(&'any str, &'any str) -> bool + for<'r> Other<T<'r> = ()>,
_a: impl for<'alpha, 'beta> Auxiliary<'alpha, Item<'beta> = fn(&'beta ())>,
) {}

pub trait Other {
type T<'dependency>;
}

pub trait Auxiliary<'arena> {
type Item<'input>;
}

pub async fn async_fn() {}

pub struct Foo;
Expand Down
7 changes: 7 additions & 0 deletions src/test/rustdoc/inline_cross/impl_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ pub use impl_trait_aux::func3;
// @has - '//pre[@class="rust fn"]' "T: Iterator<Item = impl Clone>,"
pub use impl_trait_aux::func4;

// @has impl_trait/fn.func5.html
// @has - '//pre[@class="rust fn"]' "func5("
// @has - '//pre[@class="rust fn"]' "_f: impl for<'any> Fn(&'any str, &'any str) -> bool + for<'r> Other<T<'r> = ()>,"
// @has - '//pre[@class="rust fn"]' "_a: impl for<'alpha, 'beta> Auxiliary<'alpha, Item<'beta> = fn(&'beta ())>"
// @!has - '//pre[@class="rust fn"]' 'where'
pub use impl_trait_aux::func5;

// @has impl_trait/fn.async_fn.html
// @has - '//pre[@class="rust fn"]' "pub async fn async_fn()"
pub use impl_trait_aux::async_fn;
Expand Down

0 comments on commit 6b6610b

Please sign in to comment.