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: show inner enum and struct in type definition for concrete type #114855

Merged
merged 8 commits into from Sep 7, 2023

Conversation

Urgau
Copy link
Contributor

@Urgau Urgau commented Aug 15, 2023

This PR implements the Display enum variants for generic enum in type def page #rustdoc/zulip proposal.

This proposal comes from looking at TyKind typedef from the compiler. On that page, the documentation is able to show the layout for each variant, but not the variants themselves. This proposal suggests showing the fields and variants for those "concrete type". This would mean that instead of having many unresolved generics, like in IrTyKind:

    Array(I::Ty, I::Const),
    Slice(I::Ty),
    RawPtr(I::TypeAndMut),
    Ref(I::Region, I::Ty, I::Mutability),
    FnDef(I::DefId, I::GenericArgsRef),

those would be resolved with direct links to the proper types in the TyKind typedef page:

    Array(Ty<'tcx>, Const<'tcx>),
    Slice(Ty<'tcx>),
    RawPtr(TypeAndMut<'tcx>),
    Ref(Region<'tcx>, Ty<'tcx>, Mutability<'tcx>),
    FnDef(DefId<'tcx>, GenericArgsRef<'tcx>),

Saving both time and confusion.


Old description

I've chosen to add the enums and structs under the "Show Aliased Type" details, as well as showing the variants and fields under the usual "Variants" and "Fields" sections. under new the Inner Variants and Inner Fields sections (except for their names, they are identical to the one found in the enum, struct and union pages). Those sections are complementary and do not replace anything else.

This PR proposes the following condition for showing the aliased type (basically, has the aliased type some generics that are all of them resolved):

  • the typedef does NOT have any generics (modulo lifetimes)
  • AND the aliased type has some generics

Examples

pub enum IrTyKind<'a, I: Interner> {
    /// Doc comment for AdtKind
    AdtKind(&'a I::Adt),
    /// and another one for TyKind
    TyKind(I::Adt, I::Ty),
    // no comment
    StructKind { a: I::Adt, },
}

pub type TyKind<'a> = IrTyKind<'a, TyCtxt>;

TyKind

Old

image

TyKind

TyKind

pub struct One<T> {
    pub val: T,
    #[doc(hidden)]
    pub inner_tag: u64,
    __hidden: T,
}

/// `One` with `u64` as payload
pub type OneU64 = One<u64>;

OneU64

Old

image

OneU64

OneU64

r? @GuillaumeGomez

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 15, 2023
@rustbot
Copy link
Collaborator

rustbot commented Aug 15, 2023

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

@GuillaumeGomez
Copy link
Member

Just having "inner fields" isn't great. Maybe we could show the aliased type definition in a "show aliased type" thing like we have for "show all" when there are too any fields/variants?

@rustbot
Copy link
Collaborator

rustbot commented Aug 16, 2023

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

@Urgau
Copy link
Contributor Author

Urgau commented Aug 16, 2023

Sure. I've pushed a commit that does this. (I'm not sure about the CSS changes, I would appreciate some guidance).

Closed:
closed
Opened:
Opened

EDIT: This layout is out of date. Please refer to the description for up to date ones.

@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the rustdoc-typedef-inner-variants branch 2 times, most recently from 8b31409 to 4255f87 Compare August 16, 2023 11:04
@Urgau
Copy link
Contributor Author

Urgau commented Aug 16, 2023

I've discussed with @GuillaumeGomez on Zulip about the UI layout and we concluded that it should also display the enum/struct code definition. So I've added it.

image

@Urgau Urgau changed the title rustdoc: show inner variants and fields in type def for concrete type rustdoc: show inner enum and struct in type definition for concrete type Aug 16, 2023
@GuillaumeGomez
Copy link
Member

Looks really nice! I'm a big fan of this change considering how often we encounter it in rustc's docs. Once we're done with this review round, I'll start an FCP as it is a UI change.

@Urgau Urgau force-pushed the rustdoc-typedef-inner-variants branch from 4255f87 to 7c93554 Compare August 16, 2023 13:32
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small style suggestions. I haven't looked at the changes from a semantic standpoint yet, I'm gonna do so earliest tomorrow.

Edit: Thanks a lot for working on this, this will help tremendously when browsing the rustc docs!

src/librustdoc/clean/types.rs Outdated Show resolved Hide resolved
src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved
src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved
@Urgau Urgau force-pushed the rustdoc-typedef-inner-variants branch from 7c93554 to e77e3fa Compare August 16, 2023 14:16
@bors
Copy link
Contributor

bors commented Aug 16, 2023

☔ The latest upstream changes (presumably #114905) made this pull request unmergeable. Please resolve the merge conflicts.

@Urgau Urgau force-pushed the rustdoc-typedef-inner-variants branch from 06e1dc7 to ad71ae2 Compare August 17, 2023 08:08
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked if it handles #[doc(hidden)] correctly (with and without --document-hidden-items -Zunstable-options)? On fields and on variants?

Edit: I've just checked myself, it doesn't.

src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Aug 28, 2023
@rfcbot
Copy link

rfcbot commented Aug 28, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Sep 7, 2023
@rfcbot
Copy link

rfcbot commented Sep 7, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@GuillaumeGomez
Copy link
Member

Thanks everyone!

@bors r+

@bors
Copy link
Contributor

bors commented Sep 7, 2023

📌 Commit 85e4b89 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 7, 2023
@bors
Copy link
Contributor

bors commented Sep 7, 2023

⌛ Testing commit 85e4b89 with merge 70c7e4d...

@bors
Copy link
Contributor

bors commented Sep 7, 2023

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 70c7e4d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 7, 2023
@bors bors merged commit 70c7e4d into rust-lang:master Sep 7, 2023
12 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Sep 7, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (70c7e4d): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.5% [0.5%, 0.5%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.5% [0.5%, 0.5%] 1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.3% [1.3%, 1.3%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.6% [-3.6%, -3.6%] 1
All ❌✅ (primary) 1.3% [1.3%, 1.3%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 629.026s -> 628.819s (-0.03%)
Artifact size: 318.02 MiB -> 318.09 MiB (0.02%)

@Urgau Urgau deleted the rustdoc-typedef-inner-variants branch September 7, 2023 20:01
@notriddle
Copy link
Contributor

Of course there's a regression. It's emitting more HTML.

@jplatte
Copy link
Contributor

jplatte commented Sep 8, 2023

Wow, this is amazing! Thanks to everybody involved in landing this 🤩
Could it be extended to also show applicable methods on type aliases? 🙏🏼

@Urgau
Copy link
Contributor Author

Urgau commented Sep 8, 2023

Could it be extended to also show applicable methods on type aliases? 🙏🏼

Currently working on it! 😄

@GuillaumeGomez
Copy link
Member

There was #112429 which we needed to revert. But luckily we now have #115201 which should be merged soon.

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Sep 21, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 8, 2024
Pkgsrc changes:

 * Remove NetBSD-8 support (embedded LLVm requires newer C++
   than what is in -8; it's conceivable that this could still
   build with an external LLVM)
 * undo powerpc 9.0 file naming tweak, since we no longer support -8.
 * Remove patch to LLVM for powerpc now included by upstream.
 * Minor adjustments, checksum changes etc.


Upstream changes:

Version 1.74.1 (2023-12-07)
===========================

- [Resolved spurious STATUS_ACCESS_VIOLATIONs in LLVM]
  (rust-lang/rust#118464)
- [Clarify guarantees for std::mem::discriminant]
  (rust-lang/rust#118006)
- [Fix some subtyping-related regressions]
  (rust-lang/rust#116415)

Version 1.74.0 (2023-11-16)
==========================

Language
--------

- [Codify that `std::mem::Discriminant<T>` does not depend on any
  lifetimes in T]
  (rust-lang/rust#104299)
- [Replace `private_in_public` lint with `private_interfaces` and
  `private_bounds` per RFC 2145]
  (rust-lang/rust#113126)
  Read more in
  [RFC 2145](https://rust-lang.github.io/rfcs/2145-type-privacy.html).
- [Allow explicit `#[repr(Rust)]`]
  (rust-lang/rust#114201)
- [closure field capturing: don't depend on alignment of packed fields]
  (rust-lang/rust#115315)
- [Enable MIR-based drop-tracking for `async` blocks]
  (rust-lang/rust#107421)

Compiler
--------

- [stabilize combining +bundle and +whole-archive link modifiers]
  (rust-lang/rust#113301)
- [Stabilize `PATH` option for `--print KIND=PATH`]
  (rust-lang/rust#114183)
- [Enable ASAN/LSAN/TSAN for `*-apple-ios-macabi`]
  (rust-lang/rust#115644)
- [Promote loongarch64-unknown-none* to Tier 2]
  (rust-lang/rust#115368)
- [Add `i686-pc-windows-gnullvm` as a tier 3 target]
  (rust-lang/rust#115687)

Libraries
---------

- [Implement `From<OwnedFd/Handle>` for ChildStdin/out/err]
  (rust-lang/rust#98704)
- [Implement `From<{&,&mut} [T; N]>` for `Vec<T>` where `T: Clone`]
  (rust-lang/rust#111278)
- [impl Step for IP addresses]
  (rust-lang/rust#113748)
- [Implement `From<[T; N]>` for `Rc<[T]>` and `Arc<[T]>`]
  (rust-lang/rust#114041)
- [`impl TryFrom<char> for u16`]
  (rust-lang/rust#114065)
- [Stabilize `io_error_other` feature]
  (rust-lang/rust#115453)
- [Stabilize the `Saturating` type]
  (rust-lang/rust#115477)
- [Stabilize const_transmute_copy]
  (rust-lang/rust#115520)

Stabilized APIs
---------------

- [`core::num::Saturating`]
  (https://doc.rust-lang.org/stable/std/num/struct.Saturating.html)
- [`impl From<io::Stdout> for std::process::Stdio`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStdout%3E-for-Stdio)
- [`impl From<io::Stderr> for std::process::Stdio`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`impl From<OwnedHandle> for std::process::Child{Stdin, Stdout, Stderr}`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`impl From<OwnedFd> for std::process::Child{Stdin, Stdout, Stderr}`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`std::ffi::OsString::from_encoded_bytes_unchecked`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.from_encoded_bytes_unchecked)
- [`std::ffi::OsString::into_encoded_bytes`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.into_encoded_bytes)
- [`std::ffi::OsStr::from_encoded_bytes_unchecked`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.from_encoded_bytes_unchecked)
- [`std::ffi::OsStr::as_encoded_bytes`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.as_encoded_bytes)
- [`std::io::Error::other`]
  (https://doc.rust-lang.org/stable/std/io/struct.Error.html#method.other)
- [`impl TryFrom<char> for u16`]
  (https://doc.rust-lang.org/stable/std/primitive.u16.html#impl-TryFrom%3Cchar%3E-for-u16)
- [`impl<T: Clone, const N: usize> From<&[T; N]> for Vec<T>`]
  (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E)
- [`impl<T: Clone, const N: usize> From<&mut [T; N]> for Vec<T>`]
  (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26mut+%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E)
- [`impl<T, const N: usize> From<[T; N]> for Arc<[T]>`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#impl-From%3C%5BT;+N%5D%3E-for-Arc%3C%5BT%5D,+Global%3E)
- [`impl<T, const N: usize> From<[T; N]> for Rc<[T]>`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#impl-From%3C%5BT;+N%5D%3E-for-Rc%3C%5BT%5D,+Global%3E)

These APIs are now stable in const contexts:

- [`core::mem::transmute_copy`]
  (https://doc.rust-lang.org/beta/std/mem/fn.transmute_copy.html)
- [`str::is_ascii`]
  (https://doc.rust-lang.org/beta/std/primitive.str.html#method.is_ascii)
- [`[u8]::is_ascii`]
  (https://doc.rust-lang.org/beta/std/primitive.slice.html#method.is_ascii)

Cargo
-----

- [fix: Set MSRV for internal packages]
  (rust-lang/cargo#12381)
- [config: merge lists in precedence order]
  (rust-lang/cargo#12515)
- [fix(update): Clarify meaning of --aggressive as --recursive]
  (rust-lang/cargo#12544)
- [fix(update): Make `-p` more convenient by being positional]
  (rust-lang/cargo#12545)
- [feat(help): Add styling to help output ]
  (rust-lang/cargo#12578)
- [feat(pkgid): Allow incomplete versions when unambigious]
  (rust-lang/cargo#12614)
- [feat: stabilize credential-process and registry-auth]
  (rust-lang/cargo#12649)
- [feat(cli): Add '-n' to dry-run]
  (rust-lang/cargo#12660)
- [Add support for `target.'cfg(..)'.linker`]
  (rust-lang/cargo#12535)
- [Stabilize `--keep-going`]
  (rust-lang/cargo#12568)
- [feat: Stabilize lints]
  (rust-lang/cargo#12648)

Rustdoc
-------

- [Add warning block support in rustdoc]
  (rust-lang/rust#106561)
- [Accept additional user-defined syntax classes in fenced code blocks]
  (rust-lang/rust#110800)
- [rustdoc-search: add support for type parameters]
  (rust-lang/rust#112725)
- [rustdoc: show inner enum and struct in type definition for concrete type]
  (rust-lang/rust#114855)

Compatibility Notes
-------------------

- [Raise minimum supported Apple OS versions]
  (rust-lang/rust#104385)
- [make Cell::swap panic if the Cells partially overlap]
  (rust-lang/rust#114795)
- [Reject invalid crate names in `--extern`]
  (rust-lang/rust#116001)
- [Don't resolve generic impls that may be shadowed by dyn built-in impls]
  (rust-lang/rust#114941)

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

None this cycle.
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 A-rustdoc-ui Area: rustdoc UI (generated HTML) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet