Skip to content

Commit

Permalink
Rollup merge of #103885 - fmease:rustdoc-various-cross-crate-reexport…
Browse files Browse the repository at this point in the history
…-fixes, r=cjgillot,GuillaumeGomez

rustdoc: various cross-crate reexport fixes

Fixes for various smaller cross-crate reexport issues.
The PR is split into several commits for easier review. Will be squashed after approval.

Most notable changes:

* We finally render late-bound lifetimes in the generic parameter list of cross-crate functions & methods.
  Previously, we would display the re-export of `pub fn f<'s>(x: &'s str) {}` as `pub fn f(x: &'s str)`
* We now render unnamed parameters of cross-crate functions and function pointers as underscores
  since that's exactly what we do for local definitions, too. Mentioned as a bug in #44306.
* From now on, the rendering of cross-crate trait-object types is more correct:
  * `for<>` parameter lists (for higher-ranked lifetimes) are now shown
  * the return type of `Fn{,Mut,Once}` trait bounds is now displayed

Regarding the last list item, here is a diff for visualization (before vs. after):

```patch
- dyn FnOnce(&'any str) + 'static
+ dyn for<'any> FnOnce(&'any str) -> bool + 'static
```

The redundant `+ 'static` will be removed in a follow-up PR that will hide trait-object lifetime-bounds if they coincide with [their default](https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes) (see [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/clean_middle_ty.3A.20I.20need.20to.20add.20a.20parameter/near/307143097)). `FIXME(fmease)`s were added.

``@rustbot`` label A-cross-crate-reexports
r? ``@GuillaumeGomez``
  • Loading branch information
Yuki Okushi authored Nov 7, 2022
2 parents 7eef946 + 5ccaed2 commit 63f78d1
Show file tree
Hide file tree
Showing 14 changed files with 148 additions and 51 deletions.
5 changes: 1 addition & 4 deletions src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,7 @@ where
match br {
// We only care about named late bound regions, as we need to add them
// to the 'for<>' section
ty::BrNamed(_, name) => Some(GenericParamDef {
name,
kind: GenericParamDefKind::Lifetime { outlives: vec![] },
}),
ty::BrNamed(_, name) => Some(GenericParamDef::lifetime(name)),
_ => None,
}
})
Expand Down
13 changes: 11 additions & 2 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,19 @@ pub(crate) fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean
fn build_external_function<'tcx>(cx: &mut DocContext<'tcx>, did: DefId) -> Box<clean::Function> {
let sig = cx.tcx.fn_sig(did);

let predicates = cx.tcx.predicates_of(did);
let late_bound_regions = sig.bound_vars().into_iter().filter_map(|var| match var {
ty::BoundVariableKind::Region(ty::BrNamed(_, name)) if name != kw::UnderscoreLifetime => {
Some(clean::GenericParamDef::lifetime(name))
}
_ => None,
});

let predicates = cx.tcx.explicit_predicates_of(did);
let (generics, decl) = clean::enter_impl_trait(cx, |cx| {
// NOTE: generics need to be cleaned before the decl!
let generics = clean_ty_generics(cx, cx.tcx.generics_of(did), predicates);
let mut generics = clean_ty_generics(cx, cx.tcx.generics_of(did), predicates);
// FIXME: This does not place parameters in source order (late-bound ones come last)
generics.params.extend(late_bound_regions);
let decl = clean_fn_decl_from_did_and_sig(cx, Some(did), sig);
(generics, decl)
});
Expand Down
81 changes: 61 additions & 20 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub(crate) mod utils;

use rustc_ast as ast;
use rustc_attr as attr;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
Expand Down Expand Up @@ -182,10 +182,9 @@ fn clean_poly_trait_ref_with_bindings<'tcx>(
.collect_referenced_late_bound_regions(&poly_trait_ref)
.into_iter()
.filter_map(|br| match br {
ty::BrNamed(_, name) if name != kw::UnderscoreLifetime => Some(GenericParamDef {
name,
kind: GenericParamDefKind::Lifetime { outlives: vec![] },
}),
ty::BrNamed(_, name) if name != kw::UnderscoreLifetime => {
Some(GenericParamDef::lifetime(name))
}
_ => None,
})
.collect();
Expand Down Expand Up @@ -741,10 +740,7 @@ fn clean_ty_generics<'tcx>(
p.get_bound_params()
.into_iter()
.flatten()
.map(|param| GenericParamDef {
name: param.0,
kind: GenericParamDefKind::Lifetime { outlives: Vec::new() },
})
.map(|param| GenericParamDef::lifetime(param.0))
.collect(),
));
}
Expand Down Expand Up @@ -957,12 +953,14 @@ fn clean_args_from_types_and_names<'tcx>(
values: types
.iter()
.enumerate()
.map(|(i, ty)| {
let mut name = names.get(i).map_or(kw::Empty, |ident| ident.name);
if name.is_empty() {
name = kw::Underscore;
}
Argument { name, type_: clean_ty(ty, cx), is_const: false }
.map(|(i, ty)| Argument {
type_: clean_ty(ty, cx),
name: names
.get(i)
.map(|ident| ident.name)
.filter(|ident| !ident.is_empty())
.unwrap_or(kw::Underscore),
is_const: false,
})
.collect(),
}
Expand Down Expand Up @@ -1024,7 +1022,11 @@ fn clean_fn_decl_from_did_and_sig<'tcx>(
.iter()
.map(|t| Argument {
type_: clean_middle_ty(*t, cx, None),
name: names.next().map_or(kw::Empty, |i| i.name),
name: names
.next()
.map(|i| i.name)
.filter(|i| !i.is_empty())
.unwrap_or(kw::Underscore),
is_const: false,
})
.collect(),
Expand Down Expand Up @@ -1144,12 +1146,25 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
}
}
ty::AssocKind::Fn => {
let generics = clean_ty_generics(
let sig = tcx.fn_sig(assoc_item.def_id);

let late_bound_regions = sig.bound_vars().into_iter().filter_map(|var| match var {
ty::BoundVariableKind::Region(ty::BrNamed(_, name))
if name != kw::UnderscoreLifetime =>
{
Some(GenericParamDef::lifetime(name))
}
_ => None,
});

let mut generics = clean_ty_generics(
cx,
tcx.generics_of(assoc_item.def_id),
tcx.explicit_predicates_of(assoc_item.def_id),
);
let sig = tcx.fn_sig(assoc_item.def_id);
// FIXME: This does not place parameters in source order (late-bound ones come last)
generics.params.extend(late_bound_regions);

let mut decl = clean_fn_decl_from_did_and_sig(cx, Some(assoc_item.def_id), sig);

if assoc_item.fn_has_self_parameter {
Expand Down Expand Up @@ -1281,7 +1296,16 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
..
}) = generics.params.iter_mut().find(|param| &param.name == arg)
{
param_bounds.extend(mem::take(bounds));
param_bounds.append(bounds);
} else if let WherePredicate::RegionPredicate { lifetime: Lifetime(arg), bounds } = &mut pred
&& let Some(GenericParamDef {
kind: GenericParamDefKind::Lifetime { outlives: param_bounds },
..
}) = generics.params.iter_mut().find(|param| &param.name == arg) {
param_bounds.extend(bounds.drain(..).map(|bound| match bound {
GenericBound::Outlives(lifetime) => lifetime,
_ => unreachable!(),
}));
} else {
where_predicates.push(pred);
}
Expand Down Expand Up @@ -1653,6 +1677,9 @@ pub(crate) fn clean_middle_ty<'tcx>(

inline::record_extern_fqn(cx, did, ItemType::Trait);

// FIXME(fmease): Hide the trait-object lifetime bound if it coincides with its default
// to partially address #44306. Follow the rules outlined at
// https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes
let lifetime = clean_middle_region(*reg);
let mut bounds = dids
.map(|did| {
Expand Down Expand Up @@ -1680,8 +1707,22 @@ pub(crate) fn clean_middle_ty<'tcx>(
})
.collect();

let late_bound_regions: FxIndexSet<_> = obj
.iter()
.flat_map(|pb| pb.bound_vars())
.filter_map(|br| match br {
ty::BoundVariableKind::Region(ty::BrNamed(_, name))
if name != kw::UnderscoreLifetime =>
{
Some(GenericParamDef::lifetime(name))
}
_ => None,
})
.collect();
let late_bound_regions = late_bound_regions.into_iter().collect();

let path = external_path(cx, did, false, bindings, substs);
bounds.insert(0, PolyTrait { trait_: path, generic_params: Vec::new() });
bounds.insert(0, PolyTrait { trait_: path, generic_params: late_bound_regions });

DynTrait(bounds, lifetime)
}
Expand Down
10 changes: 3 additions & 7 deletions src/librustdoc/clean/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ pub(crate) fn where_clauses(cx: &DocContext<'_>, clauses: Vec<WP>) -> ThinVec<WP
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() },
})
.map(|param| clean::GenericParamDef::lifetime(param.0))
.collect();
merge_bounds(cx, bounds, bound_params, trait_did, name, rhs)
});
Expand Down Expand Up @@ -99,9 +96,8 @@ pub(crate) fn merge_bounds(
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).
// Sort parameters (likely) originating from a hashset alphabetically to
// produce predictable output (and to allow for full deduplication).
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);

Expand Down
4 changes: 4 additions & 0 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1437,6 +1437,10 @@ pub(crate) struct GenericParamDef {
}

impl GenericParamDef {
pub(crate) fn lifetime(name: Symbol) -> Self {
Self { name, kind: GenericParamDefKind::Lifetime { outlives: Vec::new() } }
}

pub(crate) fn is_synthetic_type_param(&self) -> bool {
match self.kind {
GenericParamDefKind::Lifetime { .. } | GenericParamDefKind::Const { .. } => false,
Expand Down
14 changes: 7 additions & 7 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::clean::render_macro_matchers::render_macro_matcher;
use crate::clean::{
clean_doc_module, clean_middle_const, clean_middle_region, clean_middle_ty, inline, Crate,
ExternalCrate, Generic, GenericArg, GenericArgs, ImportSource, Item, ItemKind, Lifetime, Path,
PathSegment, Primitive, PrimitiveType, Type, TypeBinding,
PathSegment, Primitive, PrimitiveType, Term, Type, TypeBinding, TypeBindingKind,
};
use crate::core::DocContext;
use crate::html::format::visibility_to_src_with_space;
Expand Down Expand Up @@ -113,12 +113,12 @@ fn external_generic_args<'tcx>(
ty::Tuple(tys) => tys.iter().map(|t| clean_middle_ty(t, cx, None)).collect::<Vec<_>>().into(),
_ => return GenericArgs::AngleBracketed { args: args.into(), bindings },
};
let output = None;
// FIXME(#20299) return type comes from a projection now
// match types[1].kind {
// ty::Tuple(ref v) if v.is_empty() => None, // -> ()
// _ => Some(types[1].clean(cx))
// };
let output = bindings.into_iter().next().and_then(|binding| match binding.kind {
TypeBindingKind::Equality { term: Term::Type(ty) } if ty != Type::Tuple(Vec::new()) => {
Some(Box::new(ty))
}
_ => None,
});
GenericArgs::Parenthesized { inputs, output }
} else {
GenericArgs::AngleBracketed { args: args.into(), bindings: bindings.into() }
Expand Down
11 changes: 4 additions & 7 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1232,9 +1232,8 @@ impl clean::Arguments {
) -> impl fmt::Display + 'a + Captures<'tcx> {
display_fn(move |f| {
for (i, input) in self.values.iter().enumerate() {
if !input.name.is_empty() {
write!(f, "{}: ", input.name)?;
}
write!(f, "{}: ", input.name)?;

if f.alternate() {
write!(f, "{:#}", input.type_.print(cx))?;
} else {
Expand Down Expand Up @@ -1367,10 +1366,8 @@ impl clean::FnDecl {
args.push_str("const ");
args_plain.push_str("const ");
}
if !input.name.is_empty() {
write!(args, "{}: ", input.name);
write!(args_plain, "{}: ", input.name);
}
write!(args, "{}: ", input.name);
write!(args_plain, "{}: ", input.name);

if f.alternate() {
write!(args, "{:#}", input.type_.print(cx));
Expand Down
1 change: 1 addition & 0 deletions src/test/rustdoc/assoc-consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub fn f(_: &(ToString + 'static)) {}
impl Bar {
// @has assoc_consts/struct.Bar.html '//*[@id="associatedconstant.F"]' \
// "const F: fn(_: &(dyn ToString + 'static))"
// FIXME(fmease): Hide default lifetime, render "const F: fn(_: &dyn ToString)"
pub const F: fn(_: &(ToString + 'static)) = f;
}

Expand Down
6 changes: 5 additions & 1 deletion src/test/rustdoc/inline_cross/assoc_item_trait_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@ extern crate assoc_item_trait_bounds as aux;
// @snapshot out9 - '//*[@id="associatedtype.Out9"]/*[@class="code-header"]'
//
// @has - '//*[@id="tymethod.make"]' \
// "fn make<F>(F, impl FnMut(&str) -> bool)\
// "fn make<F>(_: F, _: impl FnMut(&str) -> bool)\
// where \
// F: FnOnce(u32) -> String, \
// Self::Out2<()>: Protocol<u8, Q0 = Self::Item, Q1 = ()>"
pub use aux::Main;

// @has main/trait.Aid.html
// @has - '//*[@id="associatedtype.Result"]' "type Result<'inter: 'src>"
pub use aux::Aid;
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ pub trait Helper {
}

pub trait Aid<'src> {
type Result<'inter>;
type Result<'inter: 'src>;
}
17 changes: 17 additions & 0 deletions src/test/rustdoc/inline_cross/auxiliary/dyn_trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
pub type Ty0 = dyn for<'any> FnOnce(&'any str) -> bool;

pub type Ty1<'obj> = dyn std::fmt::Display + 'obj;

pub type Ty2 = dyn for<'a, 'r> Container<'r, Item<'a, 'static> = ()>;

pub type Ty3<'s> = &'s dyn ToString;

pub fn func0(_: &(dyn Fn() + '_)) {}

pub fn func1<'func>(_: &(dyn Fn() + 'func)) {}

pub trait Container<'r> {
type Item<'a, 'ctx>;
}

pub trait Shape<'a> {}
31 changes: 31 additions & 0 deletions src/test/rustdoc/inline_cross/dyn_trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#![crate_name = "user"]

// aux-crate:dyn_trait=dyn_trait.rs
// edition:2021

// @has user/type.Ty0.html
// @has - '//*[@class="item-decl"]//code' "dyn for<'any> FnOnce(&'any str) -> bool + 'static"
// FIXME(fmease): Hide default lifetime bound `'static`
pub use dyn_trait::Ty0;

// @has user/type.Ty1.html
// @has - '//*[@class="item-decl"]//code' "dyn Display + 'obj"
pub use dyn_trait::Ty1;

// @has user/type.Ty2.html
// @has - '//*[@class="item-decl"]//code' "dyn for<'a, 'r> Container<'r, Item<'a, 'static> = ()>"
pub use dyn_trait::Ty2;

// @has user/type.Ty3.html
// @has - '//*[@class="item-decl"]//code' "&'s (dyn ToString + 's)"
// FIXME(fmease): Hide default lifetime bound, render "&'s dyn ToString"
pub use dyn_trait::Ty3;

// @has user/fn.func0.html
// @has - '//pre[@class="rust fn"]' "func0(_: &dyn Fn())"
// FIXME(fmease): Show placeholder-lifetime bound, render "func0(_: &(dyn Fn() + '_))"
pub use dyn_trait::func0;

// @has user/fn.func1.html
// @has - '//pre[@class="rust fn"]' "func1<'func>(_: &(dyn Fn() + 'func))"
pub use dyn_trait::func1;
2 changes: 1 addition & 1 deletion src/test/rustdoc/inline_cross/impl_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ 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"]' "_a: impl for<'alpha, 'beta> Auxiliary<'alpha, Item<'beta> = fn(_: &'beta ())>"
// @!has - '//pre[@class="rust fn"]' 'where'
pub use impl_trait_aux::func5;

Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/issue-20727.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ pub mod reexport {
// @has - '//*[@class="rust trait"]' 'trait Deref {'
// @has - '//*[@class="rust trait"]' 'type Target: ?Sized;'
// @has - '//*[@class="rust trait"]' \
// "fn deref(&'a self) -> &'a Self::Target;"
// "fn deref<'a>(&'a self) -> &'a Self::Target;"
pub use issue_20727::Deref;
}

0 comments on commit 63f78d1

Please sign in to comment.