Skip to content

Commit fcd65ef

Browse files
committed
Auto merge of rust-lang#112463 - fmease:rustdoc-elide-x-crate-def-gen-args, r=<try>
rustdoc: elide cross-crate default generic arguments Elide cross-crate generic arguments if they coincide with their default. TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on. Fixes rust-lang#80379. Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637. r? `@ghost`
2 parents 8ce4540 + 31c5cff commit fcd65ef

File tree

8 files changed

+265
-31
lines changed

8 files changed

+265
-31
lines changed

src/librustdoc/clean/utils.rs

+102-21
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
1717
use rustc_metadata::rendered_const;
1818
use rustc_middle::mir;
1919
use rustc_middle::ty::{self, GenericArgKind, GenericArgsRef, TyCtxt};
20+
use rustc_middle::ty::{TypeVisitable, TypeVisitableExt};
2021
use rustc_span::symbol::{kw, sym, Symbol};
2122
use std::fmt::Write as _;
2223
use std::mem;
@@ -76,40 +77,120 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate {
7677

7778
pub(crate) fn ty_args_to_args<'tcx>(
7879
cx: &mut DocContext<'tcx>,
79-
args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
80+
ty_args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
8081
has_self: bool,
8182
container: Option<DefId>,
8283
) -> Vec<GenericArg> {
83-
let mut skip_first = has_self;
84-
let mut ret_val =
85-
Vec::with_capacity(args.skip_binder().len().saturating_sub(if skip_first { 1 } else { 0 }));
86-
87-
ret_val.extend(args.iter().enumerate().filter_map(|(index, kind)| {
88-
match kind.skip_binder().unpack() {
89-
GenericArgKind::Lifetime(lt) => {
90-
Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided())))
91-
}
92-
GenericArgKind::Type(_) if skip_first => {
93-
skip_first = false;
94-
None
84+
let params = container.map(|container| &cx.tcx.generics_of(container).params);
85+
let mut elision_has_failed_once_before = false;
86+
87+
let offset = if has_self { 1 } else { 0 };
88+
let mut args = Vec::with_capacity(ty_args.skip_binder().len().saturating_sub(offset));
89+
90+
let ty_arg_to_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() {
91+
GenericArgKind::Lifetime(lt) => {
92+
Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided())))
93+
}
94+
GenericArgKind::Type(_) if has_self && index == 0 => None,
95+
GenericArgKind::Type(ty) => {
96+
if !elision_has_failed_once_before
97+
&& let Some(params) = params
98+
&& let Some(default) = params[index].default_value(cx.tcx)
99+
{
100+
let default =
101+
ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_ty());
102+
103+
if can_elide_generic_arg(ty_args.rebind(ty), default) {
104+
return None;
105+
}
106+
107+
elision_has_failed_once_before = true;
95108
}
96-
GenericArgKind::Type(ty) => Some(GenericArg::Type(clean_middle_ty(
97-
kind.rebind(ty),
109+
110+
Some(GenericArg::Type(clean_middle_ty(
111+
ty_args.rebind(ty),
98112
cx,
99113
None,
100114
container.map(|container| crate::clean::ContainerTy::Regular {
101115
ty: container,
102-
args,
116+
args: ty_args,
103117
has_self,
104118
arg: index,
105119
}),
106-
))),
107-
GenericArgKind::Const(ct) => {
108-
Some(GenericArg::Const(Box::new(clean_middle_const(kind.rebind(ct), cx))))
120+
)))
121+
}
122+
GenericArgKind::Const(ct) => {
123+
if !elision_has_failed_once_before
124+
&& let Some(params) = params
125+
&& let Some(default) = params[index].default_value(cx.tcx)
126+
{
127+
let default =
128+
ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_const());
129+
130+
if can_elide_generic_arg(ty_args.rebind(ct), default) {
131+
return None;
132+
}
133+
134+
elision_has_failed_once_before = true;
109135
}
136+
137+
Some(GenericArg::Const(Box::new(clean_middle_const(ty_args.rebind(ct), cx))))
110138
}
111-
}));
112-
ret_val
139+
};
140+
141+
args.extend(ty_args.skip_binder().iter().enumerate().rev().filter_map(ty_arg_to_arg));
142+
args.reverse();
143+
args
144+
}
145+
146+
/// Check if the generic argument `actual` coincides with the `default` and can therefore be elided.
147+
///
148+
/// This uses a very conservative approach for performance and correctness reasons, meaning for
149+
/// several classes of terms it claims that they cannot be elided even if they theoretically could.
150+
/// This is absolutely fine since this concerns mostly edge cases.
151+
fn can_elide_generic_arg<'tcx, Term>(
152+
actual: ty::Binder<'tcx, Term>,
153+
default: ty::Binder<'tcx, Term>,
154+
) -> bool
155+
where
156+
Term: Eq + TypeVisitable<TyCtxt<'tcx>>,
157+
{
158+
// In practice, we shouldn't have any inference variables at this point. However to be safe, we
159+
// bail out if we do happen to stumble upon them. For performance reasons, we don't want to
160+
// construct an `InferCtxt` here to properly handle them.
161+
if actual.has_infer() || default.has_infer() {
162+
return false;
163+
}
164+
165+
// Since we don't properly keep track of bound variables in rustdoc (yet), we don't attempt to
166+
// make any sense out of escaping bound variables. We simply don't have enough context and it
167+
// would be incorrect to try to do so anyway.
168+
if actual.has_escaping_bound_vars() || default.has_escaping_bound_vars() {
169+
return false;
170+
}
171+
172+
// Theoretically we could now check if either term contains (non-escaping) late-bound regions or
173+
// projections, relate the two using an `InferCtxt` and check if the resulting obligations hold
174+
// since having projections means that the terms can potentially be further normalized thereby
175+
// revealing if they are equal after all. Regarding late-bound regions, they would need to be
176+
// liberated allowing us to consider more types to be equal by ignoring the names of binders
177+
// (e.g., `for<'a> ...` and `for<'b> ...`).
178+
//
179+
// However, we are mostly interested in eliding generic args that were originally elided by the
180+
// user and later filled in by the compiler (i.e., re-eliding) compared to eliding arbitrary
181+
// generic arguments if they happen to coincide with the default ignoring the fact we can't
182+
// possibly distinguish these two cases. Therefore and for performance reasons, we just bail out
183+
// instead.
184+
if actual.has_late_bound_regions()
185+
|| actual.has_projections()
186+
|| default.has_late_bound_regions()
187+
|| default.has_projections()
188+
{
189+
return false;
190+
}
191+
192+
// Check the memory addresses of the interned arguments for equality.
193+
actual.skip_binder() == default.skip_binder()
113194
}
114195

115196
fn external_generic_args<'tcx>(

tests/rustdoc/const-generics/add-impl.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ pub struct Simd<T, const WIDTH: usize> {
77
inner: T,
88
}
99

10-
// @has foo/struct.Simd.html '//div[@id="trait-implementations-list"]//h3[@class="code-header"]' 'impl Add<Simd<u8, 16>> for Simd<u8, 16>'
10+
// @has foo/struct.Simd.html '//div[@id="trait-implementations-list"]//h3[@class="code-header"]' 'impl Add for Simd<u8, 16>'
1111
impl Add for Simd<u8, 16> {
1212
type Output = Self;
1313

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
pub type BoxedStr = Box<str>;
2+
pub type IntMap = std::collections::HashMap<i64, u64>;
3+
4+
pub struct TyPair<T, U = T>(T, U);
5+
6+
pub type T0 = TyPair<i32>;
7+
pub type T1 = TyPair<i32, u32>;
8+
pub type T2<K> = TyPair<i32, K>;
9+
pub type T3<Q> = TyPair<Q, Q>;
10+
11+
pub struct CtPair<const C: u32, const D: u32 = C>;
12+
13+
pub type C0 = CtPair<43, 43>;
14+
pub type C1 = CtPair<0, 1>;
15+
pub type C2 = CtPair<{1 + 2}, 3>;
16+
17+
pub struct Re<'a, U = &'a ()>(&'a (), U);
18+
19+
pub type R0<'q> = Re<'q>;
20+
pub type R1<'q> = Re<'q, &'q ()>;
21+
pub type R2<'q> = Re<'q, &'static ()>;
22+
pub type H0 = fn(for<'a> fn(Re<'a>));
23+
pub type H1 = for<'b> fn(for<'a> fn(Re<'a, &'b ()>));
24+
pub type H2 = for<'a> fn(for<'b> fn(Re<'a, &'b ()>));
25+
26+
pub struct Proj<T: Basis, U = <T as Basis>::Assoc>(T, U);
27+
pub trait Basis { type Assoc; }
28+
impl Basis for () { type Assoc = bool; }
29+
30+
pub type P0 = Proj<()>;
31+
pub type P1 = Proj<(), bool>;
32+
pub type P2 = Proj<(), ()>;
33+
34+
pub struct Alpha<T = for<'any> fn(&'any ())>(T);
35+
36+
pub type A0 = Alpha;
37+
pub type A1 = Alpha<for<'arbitrary> fn(&'arbitrary ())>;
38+
39+
pub struct Multi<A = u64, B = u64>(A, B);
40+
41+
pub type M0 = Multi<u64, ()>;
42+
43+
pub trait Trait<'a, T = &'a ()> {}
44+
45+
pub type F = dyn for<'a> Trait<'a>;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
#![crate_name = "user"]
2+
// aux-crate:default_generic_args=default-generic-args.rs
3+
// edition:2021
4+
5+
// @has user/type.BoxedStr.html
6+
// @has - '//*[@class="rust item-decl"]//code' "Box<str>"
7+
pub use default_generic_args::BoxedStr;
8+
9+
// @has user/type.IntMap.html
10+
// @has - '//*[@class="rust item-decl"]//code' "HashMap<i64, u64>"
11+
pub use default_generic_args::IntMap;
12+
13+
// @has user/type.T0.html
14+
// @has - '//*[@class="rust item-decl"]//code' "TyPair<i32>"
15+
pub use default_generic_args::T0;
16+
17+
// @has user/type.T1.html
18+
// @has - '//*[@class="rust item-decl"]//code' "TyPair<i32, u32>"
19+
pub use default_generic_args::T1;
20+
21+
// @has user/type.T2.html
22+
// @has - '//*[@class="rust item-decl"]//code' "TyPair<i32, K>"
23+
pub use default_generic_args::T2;
24+
25+
// @has user/type.T3.html
26+
// @has - '//*[@class="rust item-decl"]//code' "TyPair<Q>"
27+
pub use default_generic_args::T3;
28+
29+
// @has user/type.C0.html
30+
// @has - '//*[@class="rust item-decl"]//code' "CtPair<43>"
31+
pub use default_generic_args::C0;
32+
33+
// @has user/type.C1.html
34+
// @has - '//*[@class="rust item-decl"]//code' "CtPair<0, 1>"
35+
pub use default_generic_args::C1;
36+
37+
// @has user/type.C2.html
38+
// FIXME: Add a comment here.
39+
// @has - '//*[@class="rust item-decl"]//code' "CtPair<default_generic_args::::C2::{constant#0}, 3>"
40+
pub use default_generic_args::C2;
41+
42+
// @has user/type.R0.html
43+
// @has - '//*[@class="rust item-decl"]//code' "Re<'q>"
44+
pub use default_generic_args::R0;
45+
46+
// @has user/type.R1.html
47+
// @has - '//*[@class="rust item-decl"]//code' "Re<'q>"
48+
pub use default_generic_args::R1;
49+
50+
// @has user/type.R2.html
51+
// Check that we consider regions:
52+
// @has - '//*[@class="rust item-decl"]//code' "Re<'q, &'static ()>"
53+
pub use default_generic_args::R2;
54+
55+
// @has user/type.H0.html
56+
// FIXME: Update this comment: Check that we handle higher-ranked regions correctly:
57+
// FIXME: Ideally we would also print the *binders* here.
58+
// @has - '//*[@class="rust item-decl"]//code' "fn(_: fn(_: Re<'a, &'a ()>))"
59+
pub use default_generic_args::H0;
60+
61+
// @has user/type.H1.html
62+
// FIXME: Update this: Check that we don't conflate distinct universially quantified regions (#1):
63+
// FIXME: Ideally we would also print the *binders* here.
64+
// @has - '//*[@class="rust item-decl"]//code' "fn(_: fn(_: Re<'a, &'b ()>))"
65+
pub use default_generic_args::H1;
66+
67+
// @has user/type.H2.html
68+
// Check that we don't conflate distinct universially quantified regions (#2):
69+
// @has - '//*[@class="rust item-decl"]//code' "fn(_: fn(_: Re<'a, &'b ()>))"
70+
pub use default_generic_args::H2;
71+
72+
// @has user/type.P0.html
73+
// FIXME: Add comment here.
74+
// @has - '//*[@class="rust item-decl"]//code' "Proj<(), <() as Basis>::Assoc>"
75+
pub use default_generic_args::P0;
76+
77+
// @has user/type.P1.html
78+
// @has - '//*[@class="rust item-decl"]//code' "Proj<(), bool>"
79+
pub use default_generic_args::P1;
80+
81+
// @has user/type.P2.html
82+
// @has - '//*[@class="rust item-decl"]//code' "Proj<(), ()>"
83+
pub use default_generic_args::P2;
84+
85+
// @has user/type.A0.html
86+
// Ensure that we elide generic arguments that are alpha-equivalent to their respective
87+
// generic parameter (modulo substs) (#1):
88+
// @has - '//*[@class="rust item-decl"]//code' "Alpha"
89+
pub use default_generic_args::A0;
90+
91+
// @has user/type.A1.html
92+
// Ensure that we elide generic arguments that are alpha-equivalent to their respective
93+
// generic parameter (modulo substs) (#1):
94+
// @has - '//*[@class="rust item-decl"]//code' "Alpha"
95+
pub use default_generic_args::A1;
96+
97+
// @has user/type.M0.html
98+
// Test that we don't elide `u64` even if it coincides with `A`'s default precisely because
99+
// `()` is not the default of `B`. Mindlessly eliding `u64` would lead to `M<()>` which is a
100+
// different type (`M<(), u64>` versus `M<u64, ()>`).
101+
// @has - '//*[@class="rust item-decl"]//code' "Multi<u64, ()>"
102+
pub use default_generic_args::M0;
103+
104+
// @has user/type.F.html
105+
// FIXME: Ideally, we would elide `&'a ()` but `'a` is an escaping bound var which we can't reason
106+
// about at the moment since we don't keep track of bound vars.
107+
// @has - '//*[@class="rust item-decl"]//code' "dyn for<'a> Trait<'a, &'a ()>"
108+
pub use default_generic_args::F;

tests/rustdoc/inline_cross/dyn_trait.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,16 @@ pub use dyn_trait::AmbiguousBoundWrappedEarly1;
7575
pub use dyn_trait::AmbiguousBoundWrappedStatic;
7676

7777
// @has user/type.NoBoundsWrappedDefaulted.html
78-
// @has - '//*[@class="rust item-decl"]//code' "Box<dyn Trait, Global>;"
78+
// @has - '//*[@class="rust item-decl"]//code' "Box<dyn Trait>;"
7979
pub use dyn_trait::NoBoundsWrappedDefaulted;
8080
// @has user/type.NoBoundsWrappedEarly.html
81-
// @has - '//*[@class="rust item-decl"]//code' "Box<dyn Trait + 'e, Global>;"
81+
// @has - '//*[@class="rust item-decl"]//code' "Box<dyn Trait + 'e>;"
8282
pub use dyn_trait::NoBoundsWrappedEarly;
8383
// @has user/fn.nbwl.html
84-
// @has - '//pre[@class="rust item-decl"]' "nbwl<'l>(_: Box<dyn Trait + 'l, Global>)"
84+
// @has - '//pre[@class="rust item-decl"]' "nbwl<'l>(_: Box<dyn Trait + 'l>)"
8585
pub use dyn_trait::no_bounds_wrapped_late as nbwl;
8686
// @has user/fn.nbwel.html
87-
// @has - '//pre[@class="rust item-decl"]' "nbwel(_: Box<dyn Trait + '_, Global>)"
87+
// @has - '//pre[@class="rust item-decl"]' "nbwel(_: Box<dyn Trait + '_>)"
8888
// NB: It might seem counterintuitive to display the explicitly elided lifetime `'_` here instead of
8989
// eliding it but this behavior is correct: The default is `'static` here which != `'_`.
9090
pub use dyn_trait::no_bounds_wrapped_elided as nbwel;

tests/rustdoc/inline_cross/impl_trait.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
extern crate impl_trait_aux;
55

66
// @has impl_trait/fn.func.html
7-
// @has - '//pre[@class="rust item-decl"]' "pub fn func<'a>(_x: impl Clone + Into<Vec<u8, Global>> + 'a)"
7+
// @has - '//pre[@class="rust item-decl"]' "pub fn func<'a>(_x: impl Clone + Into<Vec<u8>> + 'a)"
88
// @!has - '//pre[@class="rust item-decl"]' 'where'
99
pub use impl_trait_aux::func;
1010

@@ -34,6 +34,6 @@ pub use impl_trait_aux::func4;
3434
pub use impl_trait_aux::func5;
3535

3636
// @has impl_trait/struct.Foo.html
37-
// @has - '//*[@id="method.method"]//h4[@class="code-header"]' "pub fn method<'a>(_x: impl Clone + Into<Vec<u8, Global>> + 'a)"
37+
// @has - '//*[@id="method.method"]//h4[@class="code-header"]' "pub fn method<'a>(_x: impl Clone + Into<Vec<u8>> + 'a)"
3838
// @!has - '//*[@id="method.method"]//h4[@class="code-header"]' 'where'
3939
pub use impl_trait_aux::Foo;

tests/rustdoc/normalize-assoc-item.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ pub fn f2() -> <isize as Trait>::X {
3030
}
3131

3232
pub struct S {
33-
// @has 'normalize_assoc_item/struct.S.html' '//span[@id="structfield.box_me_up"]' 'box_me_up: Box<S, Global>'
33+
// @has 'normalize_assoc_item/struct.S.html' '//span[@id="structfield.box_me_up"]' 'box_me_up: Box<S>'
3434
pub box_me_up: <S as Trait>::X,
3535
// @has 'normalize_assoc_item/struct.S.html' '//span[@id="structfield.generic"]' 'generic: (usize, isize)'
3636
pub generic: <Generic<usize> as Trait>::X,
@@ -76,7 +76,7 @@ extern crate inner;
7676
// @has 'normalize_assoc_item/fn.foo.html' '//pre[@class="rust item-decl"]' "pub fn foo() -> i32"
7777
pub use inner::foo;
7878

79-
// @has 'normalize_assoc_item/fn.h.html' '//pre[@class="rust item-decl"]' "pub fn h<T>() -> IntoIter<T, Global>"
79+
// @has 'normalize_assoc_item/fn.h.html' '//pre[@class="rust item-decl"]' "pub fn h<T>() -> IntoIter<T>"
8080
pub fn h<T>() -> <Vec<T> as IntoIterator>::IntoIter {
8181
vec![].into_iter()
8282
}

tests/rustdoc/where-clause-order.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ where
77
}
88

99
// @has 'foo/trait.SomeTrait.html'
10-
// @has - "//*[@id='impl-SomeTrait%3C(A,+B,+C,+D,+E)%3E-for-(A,+B,+C,+D,+E)']/h3" "impl<A, B, C, D, E> SomeTrait<(A, B, C, D, E)> for (A, B, C, D, E)where A: PartialOrd<A> + PartialEq<A>, B: PartialOrd<B> + PartialEq<B>, C: PartialOrd<C> + PartialEq<C>, D: PartialOrd<D> + PartialEq<D>, E: PartialOrd<E> + PartialEq<E> + ?Sized, "
10+
// @has - "//*[@id='impl-SomeTrait-for-(A,+B,+C,+D,+E)']/h3" "impl<A, B, C, D, E> SomeTrait for (A, B, C, D, E)where A: PartialOrd<A> + PartialEq<A>, B: PartialOrd<B> + PartialEq<B>, C: PartialOrd<C> + PartialEq<C>, D: PartialOrd<D> + PartialEq<D>, E: PartialOrd<E> + PartialEq<E> + ?Sized, "
1111
impl<A, B, C, D, E> SomeTrait<(A, B, C, D, E)> for (A, B, C, D, E)
1212
where
1313
A: PartialOrd<A> + PartialEq<A>,

0 commit comments

Comments
 (0)