Skip to content

Commit

Permalink
Auto merge of rust-lang#112463 - fmease:rustdoc-elide-x-crate-def-gen…
Browse files Browse the repository at this point in the history
…-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`
  • Loading branch information
bors committed Oct 30, 2023
2 parents 91bbdd9 + 2f953bb commit dbdc31c
Show file tree
Hide file tree
Showing 9 changed files with 274 additions and 41 deletions.
12 changes: 7 additions & 5 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,16 +475,17 @@ fn projection_to_path_segment<'tcx>(
ty: ty::Binder<'tcx, ty::AliasTy<'tcx>>,
cx: &mut DocContext<'tcx>,
) -> PathSegment {
let item = cx.tcx.associated_item(ty.skip_binder().def_id);
let generics = cx.tcx.generics_of(ty.skip_binder().def_id);
let def_id = ty.skip_binder().def_id;
let item = cx.tcx.associated_item(def_id);
let generics = cx.tcx.generics_of(def_id);
PathSegment {
name: item.name,
args: GenericArgs::AngleBracketed {
args: ty_args_to_args(
cx,
ty.map_bound(|ty| &ty.args[generics.parent_count..]),
false,
None,
def_id,
)
.into(),
bindings: Default::default(),
Expand Down Expand Up @@ -2200,18 +2201,19 @@ pub(crate) fn clean_middle_ty<'tcx>(
}

ty::Alias(ty::Inherent, alias_ty) => {
let def_id = alias_ty.def_id;
let alias_ty = bound_ty.rebind(alias_ty);
let self_type = clean_middle_ty(alias_ty.map_bound(|ty| ty.self_ty()), cx, None, None);

Type::QPath(Box::new(QPathData {
assoc: PathSegment {
name: cx.tcx.associated_item(alias_ty.skip_binder().def_id).name,
name: cx.tcx.associated_item(def_id).name,
args: GenericArgs::AngleBracketed {
args: ty_args_to_args(
cx,
alias_ty.map_bound(|ty| ty.args.as_slice()),
true,
None,
def_id,
)
.into(),
bindings: Default::default(),
Expand Down
132 changes: 106 additions & 26 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
use rustc_metadata::rendered_const;
use rustc_middle::mir;
use rustc_middle::ty::{self, GenericArgKind, GenericArgsRef, TyCtxt};
use rustc_middle::ty::{TypeVisitable, TypeVisitableExt};
use rustc_span::symbol::{kw, sym, Symbol};
use std::fmt::Write as _;
use std::mem;
Expand Down Expand Up @@ -76,44 +77,123 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate {

pub(crate) fn ty_args_to_args<'tcx>(
cx: &mut DocContext<'tcx>,
args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
ty_args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
has_self: bool,
container: Option<DefId>,
owner: DefId,
) -> Vec<GenericArg> {
let mut skip_first = has_self;
let mut ret_val =
Vec::with_capacity(args.skip_binder().len().saturating_sub(if skip_first { 1 } else { 0 }));

ret_val.extend(args.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
// Fast path which avoids executing the query `generics_of`.
if ty_args.skip_binder().is_empty() {
return Vec::new();
}

let params = &cx.tcx.generics_of(owner).params;
let mut elision_has_failed_once_before = false;

let offset = if has_self { 1 } else { 0 };
let mut args = Vec::with_capacity(ty_args.skip_binder().len().saturating_sub(offset));

let ty_arg_to_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() {
GenericArgKind::Lifetime(lt) => {
Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided())))
}
GenericArgKind::Type(_) if has_self && index == 0 => None,
GenericArgKind::Type(ty) => {
if !elision_has_failed_once_before
&& let Some(default) = params[index].default_value(cx.tcx)
{
let default =
ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_ty());

if can_elide_generic_arg(ty_args.rebind(ty), default) {
return None;
}

elision_has_failed_once_before = true;
}
GenericArgKind::Type(ty) => Some(GenericArg::Type(clean_middle_ty(
kind.rebind(ty),

Some(GenericArg::Type(clean_middle_ty(
ty_args.rebind(ty),
cx,
None,
container.map(|container| crate::clean::ContainerTy::Regular {
ty: container,
args,
Some(crate::clean::ContainerTy::Regular {
ty: owner,
args: ty_args,
has_self,
arg: index,
}),
))),
)))
}
GenericArgKind::Const(ct) => {
// FIXME(effects): this relies on the host effect being called `host`, which users could also name
// their const generics.
// FIXME(effects): this causes `host = true` and `host = false` generics to also be emitted.
GenericArgKind::Const(ct) if let ty::ConstKind::Param(p) = ct.kind() && p.name == sym::host => None,
GenericArgKind::Const(ct) => {
Some(GenericArg::Const(Box::new(clean_middle_const(kind.rebind(ct), cx))))
if let ty::ConstKind::Param(p) = ct.kind()
&& p.name == sym::host
{
return None;
}

if !elision_has_failed_once_before
&& let Some(default) = params[index].default_value(cx.tcx)
{
let default =
ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_const());

if can_elide_generic_arg(ty_args.rebind(ct), default) {
return None;
}

elision_has_failed_once_before = true;
}

Some(GenericArg::Const(Box::new(clean_middle_const(ty_args.rebind(ct), cx))))
}
}));
ret_val
};

args.extend(ty_args.skip_binder().iter().enumerate().rev().filter_map(ty_arg_to_arg));
args.reverse();
args
}

/// Check if the generic argument `actual` coincides with the `default` and can therefore be elided.
///
/// This uses a very conservative approach for performance and correctness reasons, meaning for
/// several classes of terms it claims that they cannot be elided even if they theoretically could.
/// This is absolutely fine since this concerns mostly edge cases.
fn can_elide_generic_arg<'tcx, Term>(
actual: ty::Binder<'tcx, Term>,
default: ty::Binder<'tcx, Term>,
) -> bool
where
Term: Eq + TypeVisitable<TyCtxt<'tcx>>,
{
// In practice, we shouldn't have any inference variables at this point. However to be safe, we
// bail out if we do happen to stumble upon them. For performance reasons, we don't want to
// construct an `InferCtxt` here to properly handle them.
if actual.has_infer() || default.has_infer() {
return false;
}

// Since we don't properly keep track of bound variables in rustdoc (yet), we don't attempt to
// make any sense out of escaping bound variables. We simply don't have enough context and it
// would be incorrect to try to do so anyway.
if actual.has_escaping_bound_vars() || default.has_escaping_bound_vars() {
return false;
}

// Theoretically we could now check if either term contains (non-escaping) late-bound regions or
// projections, relate the two using an `InferCtxt` and check if the resulting obligations hold
// since having projections means that the terms can potentially be further normalized thereby
// revealing if they are equal after all. Regarding late-bound regions, they would need to be
// liberated allowing us to consider more types to be equal by ignoring the names of binders
// (e.g., `for<'a> ...` and `for<'b> ...`).
//
// However, we are mostly interested in eliding generic args that were originally elided by the
// user and later filled in by the compiler (i.e., re-eliding) compared to eliding arbitrary
// generic arguments if they happen to coincide with the default ignoring the fact we can't
// possibly distinguish these two cases. Therefore and for performance reasons, we just xheck
// the memory addresses of the interned arguments for equality.
actual.skip_binder() == default.skip_binder()
}

fn external_generic_args<'tcx>(
Expand All @@ -123,7 +203,7 @@ fn external_generic_args<'tcx>(
bindings: ThinVec<TypeBinding>,
ty_args: ty::Binder<'tcx, GenericArgsRef<'tcx>>,
) -> GenericArgs {
let args = ty_args_to_args(cx, ty_args.map_bound(|args| &args[..]), has_self, Some(did));
let args = ty_args_to_args(cx, ty_args.map_bound(|args| &args[..]), has_self, did);

if cx.tcx.fn_trait_kind_from_def_id(did).is_some() {
let ty = ty_args
Expand Down
2 changes: 1 addition & 1 deletion tests/rustdoc/const-generics/add-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub struct Simd<T, const WIDTH: usize> {
inner: T,
}

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

Expand Down
45 changes: 45 additions & 0 deletions tests/rustdoc/inline_cross/auxiliary/default-generic-args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
pub type BoxedStr = Box<str>;
pub type IntMap = std::collections::HashMap<i64, u64>;

pub struct TyPair<T, U = T>(T, U);

pub type T0 = TyPair<i32>;
pub type T1 = TyPair<i32, u32>;
pub type T2<K> = TyPair<i32, K>;
pub type T3<Q> = TyPair<Q, Q>;

pub struct CtPair<const C: u32, const D: u32 = C>;

pub type C0 = CtPair<43, 43>;
pub type C1 = CtPair<0, 1>;
pub type C2 = CtPair<{1 + 2}, 3>;

pub struct Re<'a, U = &'a ()>(&'a (), U);

pub type R0<'q> = Re<'q>;
pub type R1<'q> = Re<'q, &'q ()>;
pub type R2<'q> = Re<'q, &'static ()>;
pub type H0 = fn(for<'a> fn(Re<'a>));
pub type H1 = for<'b> fn(for<'a> fn(Re<'a, &'b ()>));
pub type H2 = for<'a> fn(for<'b> fn(Re<'a, &'b ()>));

pub struct Proj<T: Basis, U = <T as Basis>::Assoc>(T, U);
pub trait Basis { type Assoc; }
impl Basis for () { type Assoc = bool; }

pub type P0 = Proj<()>;
pub type P1 = Proj<(), bool>;
pub type P2 = Proj<(), ()>;

pub struct Alpha<T = for<'any> fn(&'any ())>(T);

pub type A0 = Alpha;
pub type A1 = Alpha<for<'arbitrary> fn(&'arbitrary ())>;

pub struct Multi<A = u64, B = u64>(A, B);

pub type M0 = Multi<u64, ()>;

pub trait Trait<'a, T = &'a ()> {}

pub type F = dyn for<'a> Trait<'a>;
106 changes: 106 additions & 0 deletions tests/rustdoc/inline_cross/default-generic-args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
#![crate_name = "user"]
// aux-crate:default_generic_args=default-generic-args.rs
// edition:2021

// @has user/type.BoxedStr.html
// @has - '//*[@class="rust item-decl"]//code' "Box<str>"
pub use default_generic_args::BoxedStr;

// @has user/type.IntMap.html
// @has - '//*[@class="rust item-decl"]//code' "HashMap<i64, u64>"
pub use default_generic_args::IntMap;

// @has user/type.T0.html
// @has - '//*[@class="rust item-decl"]//code' "TyPair<i32>"
pub use default_generic_args::T0;

// @has user/type.T1.html
// @has - '//*[@class="rust item-decl"]//code' "TyPair<i32, u32>"
pub use default_generic_args::T1;

// @has user/type.T2.html
// @has - '//*[@class="rust item-decl"]//code' "TyPair<i32, K>"
pub use default_generic_args::T2;

// @has user/type.T3.html
// @has - '//*[@class="rust item-decl"]//code' "TyPair<Q>"
pub use default_generic_args::T3;

// @has user/type.C0.html
// @has - '//*[@class="rust item-decl"]//code' "CtPair<43>"
pub use default_generic_args::C0;

// @has user/type.C1.html
// @has - '//*[@class="rust item-decl"]//code' "CtPair<0, 1>"
pub use default_generic_args::C1;

// @has user/type.C2.html
// FIXME: Add a comment here.
// @has - '//*[@class="rust item-decl"]//code' "CtPair<default_generic_args::::C2::{constant#0}, 3>"
pub use default_generic_args::C2;

// @has user/type.R0.html
// @has - '//*[@class="rust item-decl"]//code' "Re<'q>"
pub use default_generic_args::R0;

// @has user/type.R1.html
// @has - '//*[@class="rust item-decl"]//code' "Re<'q>"
pub use default_generic_args::R1;

// @has user/type.R2.html
// Check that we consider regions:
// @has - '//*[@class="rust item-decl"]//code' "Re<'q, &'static ()>"
pub use default_generic_args::R2;

// @has user/type.H0.html
// Check that we handle higher-ranked regions correctly:
// @has - '//*[@class="rust item-decl"]//code' "fn(_: for<'a> fn(_: Re<'a>))"
pub use default_generic_args::H0;

// @has user/type.H1.html
// Check that we don't conflate distinct universially quantified regions (#1):
// @has - '//*[@class="rust item-decl"]//code' "for<'b> fn(_: for<'a> fn(_: Re<'a, &'b ()>))"
pub use default_generic_args::H1;

// @has user/type.H2.html
// Check that we don't conflate distinct universially quantified regions (#2):
// @has - '//*[@class="rust item-decl"]//code' "for<'a> fn(_: for<'b> fn(_: Re<'a, &'b ()>))"
pub use default_generic_args::H2;

// @has user/type.P0.html
// FIXME: Add comment here.
// @has - '//*[@class="rust item-decl"]//code' "Proj<()>"
pub use default_generic_args::P0;

// @has user/type.P1.html
// @has - '//*[@class="rust item-decl"]//code' "Proj<(), bool>"
pub use default_generic_args::P1;

// @has user/type.P2.html
// @has - '//*[@class="rust item-decl"]//code' "Proj<(), ()>"
pub use default_generic_args::P2;

// @has user/type.A0.html
// Ensure that we elide generic arguments that are alpha-equivalent to their respective
// generic parameter (modulo substs) (#1):
// @has - '//*[@class="rust item-decl"]//code' "Alpha"
pub use default_generic_args::A0;

// @has user/type.A1.html
// Ensure that we elide generic arguments that are alpha-equivalent to their respective
// generic parameter (modulo substs) (#1):
// @has - '//*[@class="rust item-decl"]//code' "Alpha"
pub use default_generic_args::A1;

// @has user/type.M0.html
// Test that we don't elide `u64` even if it coincides with `A`'s default precisely because
// `()` is not the default of `B`. Mindlessly eliding `u64` would lead to `M<()>` which is a
// different type (`M<(), u64>` versus `M<u64, ()>`).
// @has - '//*[@class="rust item-decl"]//code' "Multi<u64, ()>"
pub use default_generic_args::M0;

// @has user/type.F.html
// FIXME: Ideally, we would elide `&'a ()` but `'a` is an escaping bound var which we can't reason
// about at the moment since we don't keep track of bound vars.
// @has - '//*[@class="rust item-decl"]//code' "dyn for<'a> Trait<'a, &'a ()>"
pub use default_generic_args::F;
8 changes: 4 additions & 4 deletions tests/rustdoc/inline_cross/dyn_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,16 @@ pub use dyn_trait::AmbiguousBoundWrappedEarly1;
pub use dyn_trait::AmbiguousBoundWrappedStatic;

// @has user/type.NoBoundsWrappedDefaulted.html
// @has - '//*[@class="rust item-decl"]//code' "Box<dyn Trait, Global>;"
// @has - '//*[@class="rust item-decl"]//code' "Box<dyn Trait>;"
pub use dyn_trait::NoBoundsWrappedDefaulted;
// @has user/type.NoBoundsWrappedEarly.html
// @has - '//*[@class="rust item-decl"]//code' "Box<dyn Trait + 'e, Global>;"
// @has - '//*[@class="rust item-decl"]//code' "Box<dyn Trait + 'e>;"
pub use dyn_trait::NoBoundsWrappedEarly;
// @has user/fn.nbwl.html
// @has - '//pre[@class="rust item-decl"]' "nbwl<'l>(_: Box<dyn Trait + 'l, Global>)"
// @has - '//pre[@class="rust item-decl"]' "nbwl<'l>(_: Box<dyn Trait + 'l>)"
pub use dyn_trait::no_bounds_wrapped_late as nbwl;
// @has user/fn.nbwel.html
// @has - '//pre[@class="rust item-decl"]' "nbwel(_: Box<dyn Trait + '_, Global>)"
// @has - '//pre[@class="rust item-decl"]' "nbwel(_: Box<dyn Trait + '_>)"
// 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;
Expand Down
Loading

0 comments on commit dbdc31c

Please sign in to comment.