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-json-types: Rename and document Impl::synthetic #125177

Open
aDotInTheVoid opened this issue May 16, 2024 · 3 comments
Open

rustdoc-json-types: Rename and document Impl::synthetic #125177

aDotInTheVoid opened this issue May 16, 2024 · 3 comments
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@aDotInTheVoid
Copy link
Member

"synthetic" impls arn't a really a term that are used, especially in user-facing documentation. It's quite confusing to users that we use this to refer to impl's of auto trait. It's lead to users beliving (and understandably so) that the JSON output doesn't contain wether an impl is auto or not. (eg, CC @zjp-CN).

We should rename this field to something more descriptive. An ititial proposal would be is_auto, but I'm not sure this is quite right, as it's the trait that's auto, not the impl. I'm open to suggestions for the field name.

While we're at it, we should document it and add tests.

@aDotInTheVoid aDotInTheVoid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-json Area: Rustdoc JSON backend labels May 16, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 16, 2024
@aDotInTheVoid aDotInTheVoid removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 16, 2024
@fmease
Copy link
Member

fmease commented May 16, 2024

is_auto_trait_impl?

Note that in the middle end of rustdoc, synthetic impls not only refer to (synthesized) auto trait impls but also to (synthesized) blanket impls.

I haven't checked rustdoc json but it's therefore possible that synthetic not only refers to auto trait impls but also blanket impls. If so, the field should be called is_blanket_or_auto_trait_impl.

@fmease
Copy link
Member

fmease commented May 16, 2024

Also note that – at least in the middle end of rustdoc (but likely also in rustdoc json) –, we have yet another use of the word synthetic, namely in the context of type parameters: APITs (impl-Trait in argument position) get lowered to synthetic type parameters by rustc and rustdoc keeps up this naming scheme (rightly so). Might be worth renaming the field synthetic of type parameters to is_impl_trait (or is_apit to rules out any potential confusion with opaque types) in rustdoc json.

@fmease fmease added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-enhancement Category: An issue proposing an enhancement or a PR with one. 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