Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustdoc: elide cross-crate default generic arguments #112463

Merged
merged 1 commit into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
if ty_args.skip_binder().is_empty() {
// Fast path which avoids executing the query `generics_of`.
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 it mostly concerns 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.
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.
// Having projections means that the terms can potentially be further normalized thereby possibly
// revealing that they are equal after all. Regarding late-bound regions, they could to be
// liberated allowing us to consider more types to be equal by ignoring the names of binders
// (e.g., `for<'a> TYPE<'a>` and `for<'b> TYPE<'b>`).
//
// However, we are mostly interested in “reeliding” generic args, i.e., eliding generic args that
// were originally elided by the user and later filled in by the compiler contrary to eliding
// arbitrary generic arguments if they happen to semantically coincide with the default (of course,
// we cannot possibly distinguish these two cases). Therefore and for performance reasons, it
// suffices to only perform a syntactic / structural check by comparing the memory addresses of
// the interned arguments.
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>;
104 changes: 104 additions & 0 deletions tests/rustdoc/inline_cross/default-generic-args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
#![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
// @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
// @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
Loading