Skip to content

Commit 6b6610b

Browse files
committed
Auto merge of rust-lang#102707 - fmease:rustdoc-render-more-cross-crate-hrtbs-properly, r=GuillaumeGomez
rustdoc: render more cross-crate HRTBs properly Follow-up to rust-lang#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`
2 parents 4bd3078 + 73c239e commit 6b6610b

10 files changed

+124
-41
lines changed

src/librustdoc/clean/auto_trait.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,12 @@ where
475475

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

478+
// FIXME: This code shares much of the logic found in `clean_ty_generics` and
479+
// `simplify::where_clause`. Consider deduplicating it to avoid diverging
480+
// implementations.
481+
// Further, the code below does not merge (partially re-sugared) bounds like
482+
// `Tr<A = T>` & `Tr<B = U>` and it does not render higher-ranked parameters
483+
// originating from equality predicates.
478484
for p in clean_where_predicates {
479485
let (orig_p, p) = (p, clean_predicate(p, self.cx));
480486
if p.is_none() {
@@ -549,8 +555,8 @@ where
549555
WherePredicate::RegionPredicate { lifetime, bounds } => {
550556
lifetime_to_bounds.entry(lifetime).or_default().extend(bounds);
551557
}
552-
WherePredicate::EqPredicate { lhs, rhs } => {
553-
match lhs {
558+
WherePredicate::EqPredicate { lhs, rhs, bound_params } => {
559+
match *lhs {
554560
Type::QPath(box QPathData {
555561
ref assoc, ref self_type, ref trait_, ..
556562
}) => {
@@ -585,13 +591,14 @@ where
585591
GenericArgs::AngleBracketed { ref mut bindings, .. } => {
586592
bindings.push(TypeBinding {
587593
assoc: assoc.clone(),
588-
kind: TypeBindingKind::Equality { term: rhs },
594+
kind: TypeBindingKind::Equality { term: *rhs },
589595
});
590596
}
591597
GenericArgs::Parenthesized { .. } => {
592598
existing_predicates.push(WherePredicate::EqPredicate {
593599
lhs: lhs.clone(),
594600
rhs,
601+
bound_params,
595602
});
596603
continue; // If something other than a Fn ends up
597604
// with parentheses, leave it alone

src/librustdoc/clean/mod.rs

+45-17
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,9 @@ fn clean_where_predicate<'tcx>(
292292
},
293293

294294
hir::WherePredicate::EqPredicate(ref wrp) => WherePredicate::EqPredicate {
295-
lhs: clean_ty(wrp.lhs_ty, cx),
296-
rhs: clean_ty(wrp.rhs_ty, cx).into(),
295+
lhs: Box::new(clean_ty(wrp.lhs_ty, cx)),
296+
rhs: Box::new(clean_ty(wrp.rhs_ty, cx).into()),
297+
bound_params: Vec::new(),
297298
},
298299
})
299300
}
@@ -309,7 +310,9 @@ pub(crate) fn clean_predicate<'tcx>(
309310
}
310311
ty::PredicateKind::RegionOutlives(pred) => clean_region_outlives_predicate(pred),
311312
ty::PredicateKind::TypeOutlives(pred) => clean_type_outlives_predicate(pred, cx),
312-
ty::PredicateKind::Projection(pred) => Some(clean_projection_predicate(pred, cx)),
313+
ty::PredicateKind::Projection(pred) => {
314+
Some(clean_projection_predicate(bound_predicate.rebind(pred), cx))
315+
}
313316
ty::PredicateKind::ConstEvaluatable(..) => None,
314317
ty::PredicateKind::WellFormed(..) => None,
315318

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

389392
fn clean_projection_predicate<'tcx>(
390-
pred: ty::ProjectionPredicate<'tcx>,
393+
pred: ty::Binder<'tcx, ty::ProjectionPredicate<'tcx>>,
391394
cx: &mut DocContext<'tcx>,
392395
) -> WherePredicate {
393-
let ty::ProjectionPredicate { projection_ty, term } = pred;
396+
let late_bound_regions = cx
397+
.tcx
398+
.collect_referenced_late_bound_regions(&pred)
399+
.into_iter()
400+
.filter_map(|br| match br {
401+
ty::BrNamed(_, name) if name != kw::UnderscoreLifetime => Some(Lifetime(name)),
402+
_ => None,
403+
})
404+
.collect();
405+
406+
let ty::ProjectionPredicate { projection_ty, term } = pred.skip_binder();
407+
394408
WherePredicate::EqPredicate {
395-
lhs: clean_projection(projection_ty, cx, None),
396-
rhs: clean_middle_term(term, cx),
409+
lhs: Box::new(clean_projection(projection_ty, cx, None)),
410+
rhs: Box::new(clean_middle_term(term, cx)),
411+
bound_params: late_bound_regions,
397412
}
398413
}
399414

@@ -655,8 +670,9 @@ fn clean_ty_generics<'tcx>(
655670
})
656671
.collect::<Vec<GenericParamDef>>();
657672

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

661677
let where_predicates = preds
662678
.predicates
@@ -715,6 +731,14 @@ fn clean_ty_generics<'tcx>(
715731
trait_did,
716732
name,
717733
rhs.ty().unwrap(),
734+
p.get_bound_params()
735+
.into_iter()
736+
.flatten()
737+
.map(|param| GenericParamDef {
738+
name: param.0,
739+
kind: GenericParamDefKind::Lifetime { outlives: Vec::new() },
740+
})
741+
.collect(),
718742
));
719743
}
720744

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

733-
if let crate::core::ImplTraitParam::ParamIndex(idx) = param {
734-
if let Some(proj) = impl_trait_proj.remove(&idx) {
735-
for (trait_did, name, rhs) in proj {
736-
let rhs = clean_middle_ty(rhs, cx, None);
737-
simplify::merge_bounds(cx, &mut bounds, trait_did, name, &Term::Type(rhs));
738-
}
757+
let crate::core::ImplTraitParam::ParamIndex(idx) = param else { unreachable!() };
758+
if let Some(proj) = impl_trait_proj.remove(&idx) {
759+
for (trait_did, name, rhs, bound_params) in proj {
760+
let rhs = clean_middle_ty(rhs, cx, None);
761+
simplify::merge_bounds(
762+
cx,
763+
&mut bounds,
764+
bound_params,
765+
trait_did,
766+
name,
767+
&Term::Type(rhs),
768+
);
739769
}
740-
} else {
741-
unreachable!();
742770
}
743771

744772
cx.impl_trait_bounds.insert(param, bounds);

src/librustdoc/clean/simplify.rs

+25-12
Original file line numberDiff line numberDiff line change
@@ -39,23 +39,23 @@ pub(crate) fn where_clauses(cx: &DocContext<'_>, clauses: Vec<WP>) -> Vec<WP> {
3939
WP::RegionPredicate { lifetime, bounds } => {
4040
lifetimes.push((lifetime, bounds));
4141
}
42-
WP::EqPredicate { lhs, rhs } => equalities.push((lhs, rhs)),
42+
WP::EqPredicate { lhs, rhs, bound_params } => equalities.push((lhs, rhs, bound_params)),
4343
}
4444
}
4545

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

6161
// And finally, let's reassemble everything
@@ -68,13 +68,18 @@ pub(crate) fn where_clauses(cx: &DocContext<'_>, clauses: Vec<WP>) -> Vec<WP> {
6868
bounds,
6969
bound_params,
7070
}));
71-
clauses.extend(equalities.into_iter().map(|(lhs, rhs)| WP::EqPredicate { lhs, rhs }));
71+
clauses.extend(equalities.into_iter().map(|(lhs, rhs, bound_params)| WP::EqPredicate {
72+
lhs,
73+
rhs,
74+
bound_params,
75+
}));
7276
clauses
7377
}
7478

7579
pub(crate) fn merge_bounds(
7680
cx: &clean::DocContext<'_>,
7781
bounds: &mut Vec<clean::GenericBound>,
82+
mut bound_params: Vec<clean::GenericParamDef>,
7883
trait_did: DefId,
7984
assoc: clean::PathSegment,
8085
rhs: &clean::Term,
@@ -91,6 +96,14 @@ pub(crate) fn merge_bounds(
9196
return false;
9297
}
9398
let last = trait_ref.trait_.segments.last_mut().expect("segments were empty");
99+
100+
trait_ref.generic_params.append(&mut bound_params);
101+
// Since the parameters (probably) originate from `tcx.collect_*_late_bound_regions` which
102+
// returns a hash set, sort them alphabetically to guarantee a stable and deterministic
103+
// output (and to fully deduplicate them).
104+
trait_ref.generic_params.sort_unstable_by(|p, q| p.name.as_str().cmp(q.name.as_str()));
105+
trait_ref.generic_params.dedup_by_key(|p| p.name);
106+
94107
match last.args {
95108
PP::AngleBracketed { ref mut bindings, .. } => {
96109
bindings.push(clean::TypeBinding {

src/librustdoc/clean/types.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -1350,7 +1350,7 @@ impl Lifetime {
13501350
pub(crate) enum WherePredicate {
13511351
BoundPredicate { ty: Type, bounds: Vec<GenericBound>, bound_params: Vec<Lifetime> },
13521352
RegionPredicate { lifetime: Lifetime, bounds: Vec<GenericBound> },
1353-
EqPredicate { lhs: Type, rhs: Term },
1353+
EqPredicate { lhs: Box<Type>, rhs: Box<Term>, bound_params: Vec<Lifetime> },
13541354
}
13551355

13561356
impl WherePredicate {
@@ -1361,6 +1361,15 @@ impl WherePredicate {
13611361
_ => None,
13621362
}
13631363
}
1364+
1365+
pub(crate) fn get_bound_params(&self) -> Option<&[Lifetime]> {
1366+
match self {
1367+
Self::BoundPredicate { bound_params, .. } | Self::EqPredicate { bound_params, .. } => {
1368+
Some(bound_params)
1369+
}
1370+
_ => None,
1371+
}
1372+
}
13641373
}
13651374

13661375
#[derive(Clone, PartialEq, Eq, Debug, Hash)]

src/librustdoc/html/format.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,8 @@ pub(crate) fn print_where_clause<'a, 'tcx: 'a>(
331331
bounds_display.truncate(bounds_display.len() - " + ".len());
332332
write!(f, "{}: {bounds_display}", lifetime.print())
333333
}
334-
clean::WherePredicate::EqPredicate { lhs, rhs } => {
334+
// FIXME(fmease): Render bound params.
335+
clean::WherePredicate::EqPredicate { lhs, rhs, bound_params: _ } => {
335336
if f.alternate() {
336337
write!(f, "{:#} == {:#}", lhs.print(cx), rhs.print(cx))
337338
} else {

src/librustdoc/json/conversions.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,9 @@ impl FromWithTcx<clean::WherePredicate> for WherePredicate {
432432
lifetime: convert_lifetime(lifetime),
433433
bounds: bounds.into_tcx(tcx),
434434
},
435-
EqPredicate { lhs, rhs } => {
436-
WherePredicate::EqPredicate { lhs: lhs.into_tcx(tcx), rhs: rhs.into_tcx(tcx) }
435+
// FIXME(fmease): Convert bound parameters as well.
436+
EqPredicate { lhs, rhs, bound_params: _ } => {
437+
WherePredicate::EqPredicate { lhs: (*lhs).into_tcx(tcx), rhs: (*rhs).into_tcx(tcx) }
437438
}
438439
}
439440
}

src/test/rustdoc/inline_cross/assoc_item_trait_bounds_with_bindings.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ extern crate assoc_item_trait_bounds_with_bindings as aux;
88

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

1412
// @has main/trait.Main.html
1513
// @has - '//*[@id="associatedtype.Out0"]' 'type Out0: Support<Item = ()>'
@@ -18,13 +16,14 @@ extern crate assoc_item_trait_bounds_with_bindings as aux;
1816
// @has - '//*[@id="associatedtype.Out3"]' 'type Out3: Support<Produce<()> = bool>'
1917
// @has - '//*[@id="associatedtype.Out4"]' 'type Out4<T>: Support<Produce<T> = T>'
2018
// @has - '//*[@id="associatedtype.Out5"]' "type Out5: Support<Output<'static> = &'static ()>"
21-
// @has - '//*[@id="associatedtype.Out6"]' "type Out6: Support<Output<'a> = &'a ()>"
19+
// @has - '//*[@id="associatedtype.Out6"]' "type Out6: for<'a> Support<Output<'a> = &'a ()>"
2220
// @has - '//*[@id="associatedtype.Out7"]' "type Out7: Support<Item = String, Produce<i32> = u32> + Unrelated"
2321
// @has - '//*[@id="associatedtype.Out8"]' "type Out8: Unrelated + Protocol<i16, Q1 = u128, Q0 = ()>"
2422
// @has - '//*[@id="associatedtype.Out9"]' "type Out9: FnMut(i32) -> bool + Clone"
2523
// @has - '//*[@id="associatedtype.Out10"]' "type Out10<'q>: Support<Output<'q> = ()>"
26-
// @has - '//*[@id="associatedtype.Out11"]' "type Out11: Helper<A<'s> = &'s (), B<'r> = ()>"
27-
// @has - '//*[@id="associatedtype.Out12"]' "type Out12: Helper<B<'w> = Cow<'w, str>, A<'w> = bool>"
24+
// @has - '//*[@id="associatedtype.Out11"]' "type Out11: for<'r, 's> Helper<A<'s> = &'s (), B<'r> = ()>"
25+
// @has - '//*[@id="associatedtype.Out12"]' "type Out12: for<'w> Helper<B<'w> = Cow<'w, str>, A<'w> = bool>"
26+
// @has - '//*[@id="associatedtype.Out13"]' "type Out13: for<'fst, 'snd> Aid<'snd, Result<'fst> = &'fst mut str>"
2827
//
2928
// Snapshots: Check that we do not render any where-clauses for those associated types since all of
3029
// the trait bounds contained within were moved to the bounds of the respective item.

src/test/rustdoc/inline_cross/auxiliary/assoc_item_trait_bounds_with_bindings.rs

+5
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub trait Main {
1414
type Out10<'q>: Support<Output<'q> = ()>;
1515
type Out11: for<'r, 's> Helper<A<'s> = &'s (), B<'r> = ()>;
1616
type Out12: for<'w> Helper<B<'w> = std::borrow::Cow<'w, str>, A<'w> = bool>;
17+
type Out13: for<'fst, 'snd> Aid<'snd, Result<'fst> = &'fst mut str>;
1718

1819
fn make<F>(_: F, _: impl FnMut(&str) -> bool)
1920
where
@@ -38,3 +39,7 @@ pub trait Helper {
3839
type A<'q>;
3940
type B<'q>;
4041
}
42+
43+
pub trait Aid<'src> {
44+
type Result<'inter>;
45+
}

src/test/rustdoc/inline_cross/auxiliary/impl_trait_aux.rs

+13
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,19 @@ pub fn func3(_x: impl Iterator<Item = impl Iterator<Item = u8>> + Clone) {}
1313

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

16+
pub fn func5(
17+
_f: impl for<'any> Fn(&'any str, &'any str) -> bool + for<'r> Other<T<'r> = ()>,
18+
_a: impl for<'alpha, 'beta> Auxiliary<'alpha, Item<'beta> = fn(&'beta ())>,
19+
) {}
20+
21+
pub trait Other {
22+
type T<'dependency>;
23+
}
24+
25+
pub trait Auxiliary<'arena> {
26+
type Item<'input>;
27+
}
28+
1629
pub async fn async_fn() {}
1730

1831
pub struct Foo;

src/test/rustdoc/inline_cross/impl_trait.rs

+7
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ pub use impl_trait_aux::func3;
2626
// @has - '//pre[@class="rust fn"]' "T: Iterator<Item = impl Clone>,"
2727
pub use impl_trait_aux::func4;
2828

29+
// @has impl_trait/fn.func5.html
30+
// @has - '//pre[@class="rust fn"]' "func5("
31+
// @has - '//pre[@class="rust fn"]' "_f: impl for<'any> Fn(&'any str, &'any str) -> bool + for<'r> Other<T<'r> = ()>,"
32+
// @has - '//pre[@class="rust fn"]' "_a: impl for<'alpha, 'beta> Auxiliary<'alpha, Item<'beta> = fn(&'beta ())>"
33+
// @!has - '//pre[@class="rust fn"]' 'where'
34+
pub use impl_trait_aux::func5;
35+
2936
// @has impl_trait/fn.async_fn.html
3037
// @has - '//pre[@class="rust fn"]' "pub async fn async_fn()"
3138
pub use impl_trait_aux::async_fn;

0 commit comments

Comments
 (0)