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: Re-exported impls render all bounds as where clauses, regardless of source appearance #89180

Open
camelid opened this issue Sep 22, 2021 · 4 comments
Assignees
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate A-traits Area: Trait system C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@camelid
Copy link
Member

camelid commented Sep 22, 2021

There is a "bug" related to cross-crate re-exports and where clauses: In the core docs, the impl looks like this (which is how it's actually written in the source):

image

But in the std docs, the impl looks like this, with all of the bounds expressed as where clauses:

image

I think the re-exported version is actually better, because it's easier to read, but they should at least be consistent.

Originally posted by @camelid in #88809 (comment)

Note: It's possible this is triggered by any re-exports, not just cross-crate ones, but I haven't checked.

@camelid camelid added A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate A-traits Area: Trait system C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 22, 2021
@cynecx
Copy link
Contributor

cynecx commented Sep 22, 2021

Re-exported types/traits are weird in general (eg. #84579). Is there a reason why re-exported types/trait/impls are rendered differently?

@camelid
Copy link
Member Author

camelid commented Sep 22, 2021

Re-exported types/traits are weird in general (eg. #84579). Is there a reason why re-exported types/trait/impls are rendered differently?

Not to my knowledge. I think it's just that re-exports have a different code path, so the two paths can get out of sync. In most if not all cases, it's just subtle bugs in rustdoc :)

@camelid camelid changed the title rustdoc: Re-exported impls render all bounds in where clauses, regardless of source appearance rustdoc: Re-exported impls render all bounds as where clauses, regardless of source appearance Sep 30, 2021
@fmease
Copy link
Member

fmease commented Jun 24, 2023

Is there a reason why re-exported types/trait/impls are rendered differently?

Disclaimer: I might've gotten some details wrong.

When rustdoc documents the "current" crate (i.e. the root crate of "the dependency graph" (in the conceptual sense, not in the Cargo sense)), it has access to the HIR which is close to the AST allowing the rendered output to quite faithfully mirror the user-written code. For re-exported items, rustdoc has to take a look at the metadata files (.rlib & .rmeta) of the dependency crates which don't contain HIR data types but rustc_middle / rustc_type_ir ones which are way more removed from the source code forcing rustdoc to manually piece together ("reconstruct") what they likely corresponded to.

In my current understanding it is outright impossible to tell the "origin" / "source correspondence" of most of these pieces of information as data gets lost step by step during various desugaring and elaboration routines. The most obvious and naïve way to remedy this of course would be to add flags to rustc_middle data types to track this origin. I don't think this change would get accepted though as those data types generally represent abstract concepts (generally not tracking Spans for example) and it would likely impact perf quite heavily.

This is hard.
@rustbot label E-hard

@rustbot rustbot added the E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. label Jun 24, 2023
@fmease fmease removed the E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. label Feb 15, 2024
@fmease fmease self-assigned this Feb 15, 2024
@fmease
Copy link
Member

fmease commented Feb 15, 2024

Rethinking this, I think there might be a way to fix this fully without much effort, although it's probably detrimental to performance. I haven't tried this out yet but this recently popped into my head:

All clauses returned by tcx.explicit_predicates_of and tcx.predicates_of carry a Span. If we compare these spans to the definition span (def_ident_span / def_span) of the current item (function, trait, associated type, etc.) we can figure out if the respective predicate/clause/bound originally came from the list of generic params — the Span is smaller than the def ident span — or from the where clause — the Span is greater.

I don't think impls have a def ident span though, only a def span which would be to coarse-grained I'm pretty sure. I will figure out if there's a way. Still, this is a huge step forward imo since it solves this issue for at least all non-impl items. I will at some point open an experimental PR for this.


Edit: As an update, def_ident_span doesn't make any sense in this context and it's obvious in hindsight. def_span is much closer to what we want but unfortunately it's useless for impls and free & assoc fns, for every other kind of generic item (ADT, ty alias & assoc ty, free & assoc const, trait, trait alias), it works perfectly: We can check if the predicate span is contained in the def span of the overarching item. If so, the predicate came from the list of generic params, otherwise from the where-clause.
Unfortunately, the def span of impls & fns includes the where clause for cosmetic purposes.

Originally I assumed that the predicate span spanned the entire predicate (e.g., T: Copy) including the self ty (here, T) but that's not the case, it only contains the RHS (here, Copy). If it spanned the entire predicate we could've checked if the def_ident_span of the generic param was contained in the pred span.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate A-traits Area: Trait system C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants