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: correctly clean cross-crate opaque types #116735

Closed
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
35 changes: 19 additions & 16 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2316,23 +2316,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.tcx,
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
52 changes: 47 additions & 5 deletions src/librustdoc/clean/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

use rustc_data_structures::fx::FxIndexMap;
use rustc_hir::def_id::DefId;
use rustc_middle::ty;
use rustc_middle::ty::{self, TyCtxt};
use thin_vec::ThinVec;

use crate::clean;
Expand Down 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>(
tcx: TyCtxt<'tcx>,
child: ty::TraitRef<'tcx>,
trait_: ty::TraitRef<'tcx>,
) -> bool {
if child == trait_ {
return true;
}
let predicates = tcx.super_predicates_of(child.def_id);
debug_assert!(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() != tcx.types.self_param {
return None;
}

Some(ty::EarlyBinder::bind(pred.trait_ref).instantiate(tcx, child.args))
})
.any(|child| trait_is_same_or_supertrait2(tcx, 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>> + '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>> + '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;
Loading