Skip to content

Commit

Permalink
rustdoc: correctly clean cross-crate opaque types
Browse files Browse the repository at this point in the history
  • Loading branch information
fmease committed Oct 14, 2023
1 parent 495c5dd commit 6835191
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 39 deletions.
35 changes: 19 additions & 16 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2299,23 +2299,26 @@ fn clean_middle_opaque_bounds<'tcx>(
let bindings: ThinVec<_> = bounds
.iter()
.filter_map(|bound| {
if let ty::ClauseKind::Projection(proj) = bound.kind().skip_binder() {
if proj.projection_ty.trait_ref(cx.tcx) == trait_ref.skip_binder() {
Some(TypeBinding {
assoc: projection_to_path_segment(
bound.kind().rebind(proj.projection_ty),
cx,
),
kind: TypeBindingKind::Equality {
term: clean_middle_term(bound.kind().rebind(proj.term), cx),
},
})
} else {
None
}
} else {
None
let ty::ClauseKind::Projection(proj) = bound.kind().skip_binder() else {
return None;
};
if !simplify::trait_is_same_or_supertrait2(
cx,
trait_ref.skip_binder(),
proj.projection_ty.trait_ref(cx.tcx),
) {
return None;
}

Some(TypeBinding {
assoc: projection_to_path_segment(
bound.kind().rebind(proj.projection_ty),
cx,
),
kind: TypeBindingKind::Equality {
term: clean_middle_term(bound.kind().rebind(proj.term), cx),
},
})
})
.collect();

Expand Down
50 changes: 46 additions & 4 deletions src/librustdoc/clean/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ pub(crate) fn where_clauses(cx: &DocContext<'_>, clauses: Vec<WP>) -> ThinVec<WP
clauses
}

// NOTE(fmease): This basically does the same bound cleaning as `clean_middle_opaque_bounds`.
pub(crate) fn merge_bounds(
cx: &clean::DocContext<'_>,
bounds: &mut Vec<clean::GenericBound>,
Expand All @@ -76,9 +77,8 @@ pub(crate) fn merge_bounds(
rhs: &clean::Term,
) -> bool {
!bounds.iter_mut().any(|b| {
let trait_ref = match *b {
clean::GenericBound::TraitBound(ref mut tr, _) => tr,
clean::GenericBound::Outlives(..) => return false,
let clean::GenericBound::TraitBound(trait_ref, _) = b else {
return false;
};
// If this QPath's trait `trait_did` is the same as, or a supertrait
// of, the bound's trait `did` then we can keep going, otherwise
Expand Down Expand Up @@ -108,7 +108,21 @@ pub(crate) fn merge_bounds(
})
}

fn trait_is_same_or_supertrait(cx: &DocContext<'_>, child: DefId, trait_: DefId) -> bool {
// FIXME(fmease): This doesn't take into account the generic args of traits.
//
// Get rid of this in favor of `trait_is_same_or_supertrait2`.
//
// This is soft-blocked on `merge_bounds` and `where_clauses`
// operating on `rustc_middle` types directly instead of cleaned
// types (part of refactoring #115379).
// I suppose it's possible to not do that and to instead clean
// `ty::TraitRef`s in `trait_is_same_or_supertrait` as we go
// but that seems fragile and bad for perf.
pub(crate) fn trait_is_same_or_supertrait(
cx: &DocContext<'_>,
child: DefId,
trait_: DefId,
) -> bool {
if child == trait_ {
return true;
}
Expand All @@ -128,6 +142,34 @@ fn trait_is_same_or_supertrait(cx: &DocContext<'_>, child: DefId, trait_: DefId)
.any(|did| trait_is_same_or_supertrait(cx, did, trait_))
}

// FIXME(fmease): Add docs clarifying why we need to check the generics args for equality.
// FIXME(fmease): Drop the `2` from the name.
pub(crate) fn trait_is_same_or_supertrait2<'tcx>(
cx: &DocContext<'tcx>,
child: ty::TraitRef<'tcx>,
trait_: ty::TraitRef<'tcx>,
) -> bool {
if child == trait_ {
return true;
}
let predicates = cx.tcx.super_predicates_of(child.def_id);
debug_assert!(cx.tcx.generics_of(child.def_id).has_self);
predicates
.predicates
.iter()
.filter_map(|(pred, _)| {
let ty::ClauseKind::Trait(pred) = pred.kind().skip_binder() else {
return None;
};
if pred.trait_ref.self_ty() != cx.tcx.types.self_param {
return None;
}

Some(ty::EarlyBinder::bind(pred.trait_ref).instantiate(cx.tcx, child.args))
})
.any(|child| trait_is_same_or_supertrait2(cx, child, trait_))
}

/// Move bounds that are (likely) directly attached to generic parameters from the where-clause to
/// the respective parameter.
///
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// edition:2018

use std::ops::Deref;

pub fn func<'a>(_x: impl Clone + Into<Vec<u8>> + 'a) {}
Expand All @@ -26,10 +24,22 @@ pub trait Auxiliary<'arena> {
type Item<'input>;
}

pub async fn async_fn() {}

pub struct Foo;

impl Foo {
pub fn method<'a>(_x: impl Clone + Into<Vec<u8>> + 'a) {}
}

// Regression test for issue #113015, subitem (1) / bevyengine/bevy#8898.
// Check that we pick up on the return type of cross-crate opaque `Fn`s and
// `FnMut`s (`FnOnce` already used to work fine).
pub fn rpit_fn() -> impl Fn() -> bool {
|| false
}

pub fn rpit_fn_mut() -> impl for<'a> FnMut(&'a str) -> &'a str {
|source| source
}

// FIXME(fmease): Add more tests that demonstrate the importance of checking the
// generic args of supertrait bounds.
37 changes: 22 additions & 15 deletions tests/rustdoc/inline_cross/impl_trait.rs
Original file line number Diff line number Diff line change
@@ -1,39 +1,46 @@
// aux-build:impl_trait_aux.rs
// aux-crate:impl_trait=impl_trait.rs
// edition:2018
#![crate_name = "user"]

extern crate impl_trait_aux;

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

// @has impl_trait/fn.func2.html
// @has user/fn.func2.html
// @has - '//pre[@class="rust item-decl"]' "func2<T>("
// @has - '//pre[@class="rust item-decl"]' "_x: impl Deref<Target = Option<T>> + Iterator<Item = T>,"
// @has - '//pre[@class="rust item-decl"]' "_y: impl Iterator<Item = u8> )"
// @!has - '//pre[@class="rust item-decl"]' 'where'
pub use impl_trait_aux::func2;
pub use impl_trait::func2;

// @has impl_trait/fn.func3.html
// @has user/fn.func3.html
// @has - '//pre[@class="rust item-decl"]' "func3("
// @has - '//pre[@class="rust item-decl"]' "_x: impl Iterator<Item = impl Iterator<Item = u8>> + Clone)"
// @!has - '//pre[@class="rust item-decl"]' 'where'
pub use impl_trait_aux::func3;
pub use impl_trait::func3;

// @has impl_trait/fn.func4.html
// @has user/fn.func4.html
// @has - '//pre[@class="rust item-decl"]' "func4<T>("
// @has - '//pre[@class="rust item-decl"]' "T: Iterator<Item = impl Clone>,"
pub use impl_trait_aux::func4;
pub use impl_trait::func4;

// @has impl_trait/fn.func5.html
// @has user/fn.func5.html
// @has - '//pre[@class="rust item-decl"]' "func5("
// @has - '//pre[@class="rust item-decl"]' "_f: impl for<'any> Fn(&'any str, &'any str) -> bool + for<'r> Other<T<'r> = ()>,"
// @has - '//pre[@class="rust item-decl"]' "_a: impl for<'beta, 'alpha, '_gamma> Auxiliary<'alpha, Item<'beta> = fn(_: &'beta ())>"
// @!has - '//pre[@class="rust item-decl"]' 'where'
pub use impl_trait_aux::func5;
pub use impl_trait::func5;

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

// @has user/fn.rpit_fn.html
// @has - '//pre[@class="rust item-decl"]' "rpit_fn() -> impl Fn() -> bool"
pub use impl_trait::rpit_fn;

// @has user/fn.rpit_fn_mut.html
// @has - '//pre[@class="rust item-decl"]' "rpit_fn_mut() -> impl for<'a> FnMut(&'a str) -> &'a str"
pub use impl_trait::rpit_fn_mut;

0 comments on commit 6835191

Please sign in to comment.