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: various cross-crate reexport fixes #103885

Merged
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 @@ -1652,6 +1676,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an object lifetime default query, so you can just test equality.

Copy link
Member Author

@fmease fmease Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I am aware of the existence of tcx.object_lifetime_default but it's not the whole solution to the problem, it's only part of it.

The query doesn't take a Region, it takes the DefId of a type parameter of an item, e.g. the T of Box. Anyways, that's all fine and good and explained in the linked Zulip topic. I'd really prefer to implement that in a follow-up PR to avoid bitrotting, to make reviewing easier and since that logic will be a little bit more complicated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so regarding trait-object-type lifetime-bound defaults, my local work-in-progress patch is currently +168/-43 in size and it's not as straightforward as this PR meaning I won't add it to it.

let lifetime = clean_middle_region(*reg);
let mut bounds = dids
.map(|did| {
Expand Down Expand Up @@ -1679,8 +1706,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;
}