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.

Sign up for GitHub

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

Jump to bottom

Rustdoc should indicate variance on lifetime/type parameters #22108

Open
tomjakubowski opened this issue Feb 9, 2015 · 9 comments · May be fixed by #123220
Open

Rustdoc should indicate variance on lifetime/type parameters #22108

tomjakubowski opened this issue Feb 9, 2015 · 9 comments · May be fixed by #123220
Assignees
@fmease
Labels
A-variance Area: Variance (https://doc.rust-lang.org/nomicon/subtyping.html) C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@tomjakubowski
Copy link
Contributor

tomjakubowski commented Feb 9, 2015 •
edited by rustbot
Loading

This has come up a few times in IRC and I think it's a nice idea. It's already important for lifetime parameters, where variance is important today, and could be useful for type parameters as well if rust-lang/rfcs#738 is accepted.
@sanxiyn sanxiyn added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Feb 9, 2015
@tomjakubowski tomjakubowski mentioned this issue Feb 11, 2015
Merged
@steveklabnik
Copy link
Member

Triage: no changes here

@steveklabnik steveklabnik added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 18, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 22, 2017
@QuietMisdreavus QuietMisdreavus mentioned this issue Aug 14, 2017
Open
@steveklabnik
Copy link
Member

@rust-lang/rustdoc is this something we intend to pursue?

@QuietMisdreavus
Copy link
Member

I'm not too sure how we could display this without cluttering up the display way more than we already have. Moreover, how do we collect this information? I don't have a good grasp of variance myself, much less whether we can just ask the compiler about a type/lifetime param to get an easy answer. I'm inclined to say "no", but the question of variance tends to crop up regularly enough that it may be worth leaving open for an intrepid compiler hacker.

@sunshowers
Copy link
Contributor

This would be super useful to build tooling around even if only exposed in JSON output (#76578). Variance changes are one of the more subtle kinds of API breakage, and anything we can do to make them easier to understand would be great.

@jyn514
Copy link
Member

jyn514 commented Dec 16, 2020

@sunshowers I would be willing to take a PR for this (especially if it's only in the JSON) but I have no idea how to implement it myself.

@jyn514 jyn514 closed this as completed Dec 16, 2020
@jyn514 jyn514 reopened this Dec 16, 2020
@danielhenrymantilla
Copy link
Contributor

Same here, will have to learn a bit more about the compiler representation of variance, but language-wise, it is a topic I am well acquainted with and would like to push for it 🙂 (will probably team up with @jyn514 💪).

@rustbot claim

@ehuss ehuss removed the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label Jan 18, 2022
@GuillaumeGomez GuillaumeGomez mentioned this issue Feb 12, 2024
Closed
@fmease fmease added the A-variance Area: Variance (https://doc.rust-lang.org/nomicon/subtyping.html) label Feb 12, 2024
@fmease fmease self-assigned this Feb 12, 2024
@fmease
Copy link
Member

fmease commented Feb 12, 2024

We've discussed this today in a rustdoc meeting.

The rustdoc team is generally in favor of providing this information but it's still unclear how exactly we want to present it in the UI. It's currently contested if we want to show it inline (i.e., inside code blocks & code snippets) or in a separate paragraph similar to trait object safety.

On the other hand, providing this information in Rustdoc JSON should be pretty straightforward and the team members present in the meeting unanimously agreed that this would be useful to have especially for the semver checking tools out there.

@obi1kenobi
Copy link
Contributor

On the other hand, providing this information in Rustdoc JSON should be pretty straightforward and the team members present in the meeting unanimously agreed that this would be useful to have especially for the semver checking tools out there.

👋 cargo-semver-checks maintainer here, I'd be thrilled to plug this data into cargo-semver-checks and I'm happy to be a sounding board for the changes to the rustdoc JSON format needed to represent this data. Because of cargo-semver-checks' unique query-based architecture, almost all changes to the format are just a minor inconvenience instead of a big deal — so don't worry too much about breaking the tooling I maintain :)

Thanks so much for working on this!

This was referenced Mar 30, 2024
Draft
Draft
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 30, 2024
@bors
Auto merge of rust-lang#123219 - fmease:rustdoc-variances-json, r=<try>
77b18b0 
[WIP] rustdoc (base+json): add support for variances

I had this patch lying around for more than a month without much progress. Submitting this now to move things forward hopefully.

Addresses part of rust-lang#22108.

---

For transparency, I've stalled work on this because I haven't had the time to find ways to obtain or represent the “reason” / “provenance” of variances — a feature kindly requested by `@obi1kenobi,` maintainer of cargo-semver-checks. With “reason” we're referring to the set of constituent types that have had an influence on the variance of a given generic parameter.

This information could then be used in the diagnostics of cargo-semver-checks à la “variance of param `T` of ty `S` changed from covariant to contravariant because you changed the type of field `f` from `T` to `fn(T)”. On second thought, this seems like a major add-on and I fear that it's infeasible to implement this (smh. exposing the necessary info from rustc). On the other hand, this feature probably has a large impact on the design of the final API (i.e., how we represent variances in rustdoc JSON). I definitely don't want to hand-roll variance computation inside rustdoc.

---

TODOs:

* clean up the implementation (see individual FIXMEs)
* write docs for `rustdoc-json-types`
* add rustdoc JSON tests

r? ghost
@aDotInTheVoid
Copy link
Member

This was discussed in todays T-rustdoc meeting

TLDR:

  • @fmease working on a JSON only implementation
  • Got delayed investigating adding "cause of variance" information
    • I'm opposed to adding this to JSON
    • but it would be useful for cargo-semver-checks to have (it'd need to calulate these anyway to show a great error)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fmease fmease

Labels
A-variance Area: Variance (https://doc.rust-lang.org/nomicon/subtyping.html) C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[blocked on design] [PoC]: rustdoc (html): add support for variances fmease/rust