Skip to content

Commit cb6ab95

Browse files
committed
Auto merge of #113956 - fmease:rustdoc-fix-x-crate-rpitits, r=GuillaumeGomez,compiler-errors
rustdoc: handle cross-crate RPITITs correctly Filter out the internal associated types synthesized during the desugaring of RPITITs, they really shouldn't show up in the docs. This also fixes #113929 since we're no longer invoking `is_impossible_associated_item` (renamed from `is_impossible_method`) which cannot handle them (leading to an ICE). I don't think it makes sense to try to make `is_impossible_associated_item` handle this exotic kind of associated type (CC original author `@compiler-errors).` @ T-rustdoc reviewers, currently I'm throwing out ITIT assoc tys before cleaning assoc tys at each usage-site. I'm thinking about making `clean_middle_assoc_item` return an `Option<_>` instead and doing the check inside of it to prevent any call sites from forgetting the check for ITITs. Since I wasn't sure if you would like that approach, I didn't go through with it. Let me know what you think. <details><summary>Explanation on why <code>is_impossible_associated_item(itit_assoc_ty)</code> leads to an ICE</summary> Given the following code: ```rs pub trait Trait { fn def<T>() -> impl Default {} } impl Trait for () {} ``` The generated associated type looks something like (simplified): ```rs type {opaque#0}<T>: Default = impl Default; // the name is actually `kw::Empty` but this is the `def_path_str` repr ``` The query `is_impossible_associated_item` goes through all predicates of the associated item – in this case `<T as Sized>` – to check if they contain any generic parameters from the (generic) associated type itself. For predicates that don't contain any *own* generics, it does further processing, part of which is instantiating the predicate with the generic arguments of the impl block (which is only correct if they truly don't contain any own generics since they wouldn't get instantiated this way leading to an ICE). It checks if `parent_def_id(T) == assoc_ty_def_id` to get to know if `T` is owned by the assoc ty. Unfortunately this doesn't work for ITIT assoc tys. In this case, the parent of `T` is `Trait::def` (!) which is the associated function (I'm pretty sure this is very intentional) which is of course not equal to the assoc ty `Trait::{opaque#0}`. </details> `@rustbot` label A-cross-crate-reexports
2 parents 48c0c25 + 5924043 commit cb6ab95

File tree

7 files changed

+84
-8
lines changed

7 files changed

+84
-8
lines changed

compiler/rustc_middle/src/query/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2064,9 +2064,9 @@ rustc_queries! {
20642064
}
20652065
}
20662066

2067-
query is_impossible_method(key: (DefId, DefId)) -> bool {
2067+
query is_impossible_associated_item(key: (DefId, DefId)) -> bool {
20682068
desc { |tcx|
2069-
"checking if `{}` is impossible to call within `{}`",
2069+
"checking if `{}` is impossible to reference within `{}`",
20702070
tcx.def_path_str(key.1),
20712071
tcx.def_path_str(key.0),
20722072
}

compiler/rustc_trait_selection/src/traits/mod.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -474,11 +474,14 @@ fn subst_and_check_impossible_predicates<'tcx>(
474474
result
475475
}
476476

477-
/// Checks whether a trait's method is impossible to call on a given impl.
477+
/// Checks whether a trait's associated item is impossible to reference on a given impl.
478478
///
479479
/// This only considers predicates that reference the impl's generics, and not
480480
/// those that reference the method's generics.
481-
fn is_impossible_method(tcx: TyCtxt<'_>, (impl_def_id, trait_item_def_id): (DefId, DefId)) -> bool {
481+
fn is_impossible_associated_item(
482+
tcx: TyCtxt<'_>,
483+
(impl_def_id, trait_item_def_id): (DefId, DefId),
484+
) -> bool {
482485
struct ReferencesOnlyParentGenerics<'tcx> {
483486
tcx: TyCtxt<'tcx>,
484487
generics: &'tcx ty::Generics,
@@ -556,7 +559,7 @@ pub fn provide(providers: &mut Providers) {
556559
specializes: specialize::specializes,
557560
subst_and_check_impossible_predicates,
558561
check_tys_might_be_eq: misc::check_tys_might_be_eq,
559-
is_impossible_method,
562+
is_impossible_associated_item,
560563
..*providers
561564
};
562565
}

src/librustdoc/clean/blanket_impl.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
121121
.tcx
122122
.associated_items(impl_def_id)
123123
.in_definition_order()
124-
.map(|x| clean_middle_assoc_item(x, cx))
124+
.filter(|item| !item.is_impl_trait_in_trait())
125+
.map(|item| clean_middle_assoc_item(item, cx))
125126
.collect::<Vec<_>>(),
126127
polarity: ty::ImplPolarity::Positive,
127128
kind: ImplKind::Blanket(Box::new(clean_middle_ty(

src/librustdoc/clean/inline.rs

+2
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ pub(crate) fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean
216216
.tcx
217217
.associated_items(did)
218218
.in_definition_order()
219+
.filter(|item| !item.is_impl_trait_in_trait())
219220
.map(|item| clean_middle_assoc_item(item, cx))
220221
.collect();
221222

@@ -459,6 +460,7 @@ pub(crate) fn build_impl(
459460
None => (
460461
tcx.associated_items(did)
461462
.in_definition_order()
463+
.filter(|item| !item.is_impl_trait_in_trait())
462464
.filter(|item| {
463465
// If this is a trait impl, filter out associated items whose corresponding item
464466
// in the associated trait is marked `doc(hidden)`.

src/librustdoc/html/render/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1678,11 +1678,11 @@ fn render_impl(
16781678
rendering_params: ImplRenderingParameters,
16791679
) {
16801680
for trait_item in &t.items {
1681-
// Skip over any default trait items that are impossible to call
1681+
// Skip over any default trait items that are impossible to reference
16821682
// (e.g. if it has a `Self: Sized` bound on an unsized type).
16831683
if let Some(impl_def_id) = parent.item_id.as_def_id()
16841684
&& let Some(trait_item_def_id) = trait_item.item_id.as_def_id()
1685-
&& cx.tcx().is_impossible_method((impl_def_id, trait_item_def_id))
1685+
&& cx.tcx().is_impossible_associated_item((impl_def_id, trait_item_def_id))
16861686
{
16871687
continue;
16881688
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#![feature(return_position_impl_trait_in_trait)]
2+
3+
pub trait Trait {
4+
fn create() -> impl Iterator<Item = u64> {
5+
std::iter::empty()
6+
}
7+
}
8+
9+
pub struct Basic;
10+
pub struct Intermediate;
11+
pub struct Advanced;
12+
13+
impl Trait for Basic {
14+
// method provided by the trait
15+
}
16+
17+
impl Trait for Intermediate {
18+
fn create() -> std::ops::Range<u64> { // concrete return type
19+
0..1
20+
}
21+
}
22+
23+
impl Trait for Advanced {
24+
fn create() -> impl Iterator<Item = u64> { // opaque return type
25+
std::iter::repeat(0)
26+
}
27+
}
28+
29+
// Regression test for issue #113929:
30+
31+
pub trait Def {
32+
fn def<T>() -> impl Default {}
33+
}
34+
35+
impl Def for () {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#![crate_name = "user"]
2+
// aux-crate:rpitit=ret-pos-impl-trait-in-trait.rs
3+
// edition:2021
4+
5+
// Test that we can correctly render cross-crate RPITITs.
6+
// In particular, check that we don't render the internal associated type generated by
7+
// their desugaring. We count the number of associated items and ensure that it is exactly one.
8+
// This is more robust than checking for the absence of the associated type.
9+
10+
// @has user/trait.Trait.html
11+
// @has - '//*[@id="method.create"]' 'fn create() -> impl Iterator<Item = u64>'
12+
// The class "method" is used for all three kinds of associated items at the time of writing.
13+
// @count - '//*[@id="main-content"]//section[@class="method"]' 1
14+
pub use rpitit::Trait;
15+
16+
// @has user/struct.Basic.html
17+
// @has - '//*[@id="method.create"]' 'fn create() -> impl Iterator<Item = u64>'
18+
// @count - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]' 1
19+
pub use rpitit::Basic;
20+
21+
// @has user/struct.Intermediate.html
22+
// @has - '//*[@id="method.create"]' 'fn create() -> Range<u64>'
23+
// @count - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]' 1
24+
pub use rpitit::Intermediate;
25+
26+
// @has user/struct.Advanced.html
27+
// @has - '//*[@id="method.create"]' 'fn create() -> impl Iterator<Item = u64>'
28+
// @count - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]' 1
29+
pub use rpitit::Advanced;
30+
31+
// Regression test for issue #113929:
32+
33+
// @has user/trait.Def.html
34+
// @has - '//*[@id="method.def"]' 'fn def<T>() -> impl Default'
35+
pub use rpitit::Def;

0 commit comments

Comments
 (0)