Improve the tuple and unit trait docs #97842

notriddle · 2022-06-07T19:23:33Z

Reduce duplicate impls; show only the (T,) and include a sentence saying that there exists ones up to twelve of them.
Show Copy and Clone.
Show auto traits like Send and Sync, and blanket impls like Any.

Here's the new version:

rust-highfive · 2022-06-07T19:23:36Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

rust-highfive · 2022-06-07T19:23:37Z

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

notriddle · 2022-06-07T19:43:19Z

CC @rust-lang/rustdoc

GuillaumeGomez · 2022-06-07T19:47:23Z

I like the idea, but I'm not convinced by the approach here as I have no idea just from the reading the docs that it is also applied to (T, T1, T2,...).

Also it seems a bit strange to have only T11 in https://notriddle.com/notriddle-rustdoc-test/std/primitive.tuple.html#impl-Debug.

But I really think it's a good start!

notriddle · 2022-06-07T20:54:59Z

@GuillaumeGomez What do you imagine a good approach to this would look like?

GuillaumeGomez · 2022-06-07T21:23:56Z

I honestly have absolutely no idea. Providing this information unambigously and efficiently is gonna be the hard part I think.

cc @jsha @Manishearth

vacuus · 2022-06-07T22:24:41Z

Perhaps
impl<T0,...> Debug for (T0,...)
This trait is implemented for tuples up to twelve items long, where all items implement Debug. The above syntax is (currently) invalid.

Manishearth · 2022-06-08T00:06:45Z

I'm not sure either but yeah I do feel this is an area that could handle improvement.

I think ... is a good way of doing it. We could also do a lil * on the ... and have it show the "only implemented on tuples up till..." in a tooltip if we really want

notriddle · 2022-06-08T00:11:18Z

So, custom unstable attribute that changes the presentation of tuples?

jsha · 2022-06-08T16:42:21Z

I think impl Foo for (T, ...) is good. The (T, ...) part should link to a subheading of the tuple page's top-doc that says:

(T, ...)

In this documentation the shorthand (T, ...) is used to represent all tuples up to length twelve. When that is used, any trait bounds expressed on T applies to each field of the tuple independently. Note that this is a convenience notation to avoid repetitive documentation, not valid Rust syntax.

This commit adds a new unstable attribute, `#[doc(tuple_varadic)]`, that shows a 1-tuple as `(T, ...)` instead of just `(T,)`, and links to a section in the tuple primitive docs that talks about these.

Before: impl<T, U> UnwindSafe for (T, ...) where T: UnwindSafe, U: UnwindSafe, After: impl<T> UnwindSafe for (T, ...) where T: UnwindSafe,

notriddle · 2022-06-09T03:34:33Z

@jsha Okay, here’s a new version with that idea implemented. It does look quite a bit better.

vacuus · 2022-06-09T03:55:44Z

(I realize I'm being pedantic 😄 )

In the "Trait implementations" subheading:

In this documentation the shorthand (T, ...) is used to represent all tuples up to length twelve.

But in the impl Clone/impl Copy:

This trait is implemented on arbitrary-length tuples.

Instead, the subheading could say

In this documentation the shorthand (T, ...) is used to represent all tuples up to some length, depending on the trait.

or outright state that it's a length of twelve except for Clone/Copy

notriddle · 2022-06-09T04:25:20Z

In this documentation the shorthand (T, ...) is used to represent tuples of varying length. Any trait bounds expressed on T applies to each field of the tuple independently. Note that this is a convenience notation to avoid repetitive documentation, not valid Rust syntax.

Due to a temporary restriction in Rust’s type system, most traits are only implemented on tuples of arity 12 or less. In the future, this may change.

jsha

The display and the text are looking good to me. Just to check: are the demo links up to date?

A note on consistency: the current demo has:

impl<T: Clone> Clone for (T, ...)
impl<T11> Debug for (T11, ...)
impl<L> Default for (L, ...)
impl<A> Hash for (A, ...)

I haven't fully grokked the macros yet, so it's not clear to me where the T, T11, L, and A are coming from. It would be nice for them to be consistently T unless the other type variable names mean something.

compiler/rustc_span/src/symbol.rs

library/core/src/primitive_docs.rs

library/std/src/primitive_docs.rs

src/librustdoc/html/format.rs

Co-authored-by: Jacob Hoffman-Andrews <github@hoffman-andrews.com>

notriddle · 2022-06-14T18:19:09Z

I like the idea of using Unicode ellipses.

Maybe, to avoid being overly verbose, it could use subscripts in the tuple, but not elsewhere?

impl<T: Copy> Copy for (T₁, T₂, …, Tₙ)

m-ou-se · 2022-06-14T18:24:10Z

Yeah, I think that works well!

fbstj · 2022-06-14T18:33:49Z

impl<Tᵢ: Copy> Copy for (T₁, T₂, …, Tₙ) too?

m-ou-se · 2022-06-14T18:38:31Z

impl<Tᵢ: Copy> Copy for (T₁, T₂, …, Tₙ) too?

Hm, that might result in people wondering what the i represents. (And adding more notation with ∀ and i ∈ {1, 2, …, n} or something like that probably goes a bit too far. ^^)

But I don't have a strong opinion either way. It could also work nicely. :)

GuillaumeGomez · 2022-06-14T18:43:17Z

Mathematical notation overload. :D

jsha · 2022-06-14T19:17:09Z

Maybe, to avoid being overly verbose, it could use subscripts in the tuple, but not elsewhere?
impl<T: Copy> Copy for (T₁, T₂, …, Tₙ)

I like this version. And the detail that T's bound must apply to each Tₙ individually can be explained in the linked paragraph in the documentation.

notriddle · 2022-06-14T19:56:20Z

Okay, I've update the uploaded docs page to have the new unicode presentation: https://notriddle.com/notriddle-rustdoc-test/std/primitive.tuple.html#impl-Copy

GuillaumeGomez · 2022-06-14T20:02:35Z

Looks almost all good to me. Can you add a test for the notation as well please?

GuillaumeGomez · 2022-06-14T22:00:15Z

Looks good to me, thanks!

GuillaumeGomez · 2022-06-16T09:04:32Z

Let's go then. Thanks everyone for the feedback!

@bors r=jsha,GuillaumeGomez

bors · 2022-06-16T09:04:33Z

📌 Commit f1d24be has been approved by jsha,GuillaumeGomez

bors · 2022-06-16T11:13:33Z

⌛ Testing commit f1d24be with merge 6ec3993...

bors · 2022-06-16T13:54:17Z

☀️ Test successful - checks-actions
Approved by: jsha,GuillaumeGomez
Pushing 6ec3993 to master...

rust-timer · 2022-06-16T16:38:16Z

Finished benchmarking commit (6ec3993): comparison url.

Instruction count

Primary benchmarks: 😿 relevant regressions found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	0.4%	0.6%	2
Regressions 😿 (secondary)	0.3%	0.4%	3
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	0.4%	0.6%	2

Max RSS (memory usage)

Results

Primary benchmarks: 😿 relevant regression found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	0.6%	0.6%	1
Regressions 😿 (secondary)	3.0%	3.0%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	0.6%	0.6%	1

Cycles

Results

Primary benchmarks: 🎉 relevant improvement found
Secondary benchmarks: 🎉 relevant improvement found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-2.2%	-2.2%	1
Improvements 🎉 (secondary)	-3.1%	-3.1%	1
All 😿🎉 (primary)	-2.2%	-2.2%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

…rochenkov,GuillaumeGomez Improve the function pointer docs This is rust-lang#97842 but for function pointers instead of tuples. The concept is basically the same. * Reduce duplicate impls; show `fn (T₁, T₂, …, Tₙ)` and include a sentence saying that there exists up to twelve of them. * Show `Copy` and `Clone`. * Show auto traits like `Send` and `Sync`, and blanket impls like `Any`. https://notriddle.com/notriddle-rustdoc-test/std/primitive.fn.html

WaffleLapkin · 2022-08-19T12:48:16Z

library/core/src/tuple.rs

@@ -125,4 +156,4 @@ macro_rules! last_type {
    ($a:ident, $($rest_a:ident,)+) => { last_type!($($rest_a,)+) };
 }

-tuple_impls!(A B C D E F G H I J K L);
+tuple_impls!(E D C B A Z Y X W V U T);


Is there a particular reason for this change?

Yeah, it’s so that the tuple docs consistently show (T…) instead of (L…)

Wouldn't s/L/T suffice then? The letters seem random...

That would work, yes.

jsha · 2022-10-11T08:21:19Z

I think `impl Foo for (T, ...)` is good. The `(T, ...)` part should link to a subheading of the tuple page's top-doc that say, approximately, "In this documentation (T, ...) is used to represent all tuples up to length twelve. In such constructions, any trait bounds expressed on T apply to all fields of the tuple. Not that this is a shorthand, not valid Rust syntax."

notriddle added 3 commits June 7, 2022 11:24

docs: clean up trait docs for tuples

9940ed0

rustdoc: show auto/blanket docs for tuple and unit

1e6a857

docs: show Clone and Copy on () doc pages

7a93567

rust-highfive assigned m-ou-se Jun 7, 2022

rustbot added T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jun 7, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 7, 2022

notriddle added 3 commits June 8, 2022 19:26

rustdoc: show tuple impls as impl Trait for (T, ...)

6950f14

This commit adds a new unstable attribute, `#[doc(tuple_varadic)]`, that shows a 1-tuple as `(T, ...)` instead of just `(T,)`, and links to a section in the tuple primitive docs that talks about these.

rustdoc: fixed messed-up rustdoc auto trait impls

85b0c2f

Before: impl<T, U> UnwindSafe for (T, ...) where T: UnwindSafe, U: UnwindSafe, After: impl<T> UnwindSafe for (T, ...) where T: UnwindSafe,

Fix bootstrap attr

9f6dcce

jsha requested changes Jun 11, 2022

View reviewed changes

compiler/rustc_span/src/symbol.rs Outdated Show resolved Hide resolved

library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved

library/std/src/primitive_docs.rs Outdated Show resolved Hide resolved

src/librustdoc/html/format.rs Outdated Show resolved Hide resolved

notriddle force-pushed the notriddle/tuple-docs branch from 31c35c5 to e78950b Compare June 11, 2022 16:45

notriddle and others added 4 commits June 11, 2022 09:54

Fix incorrectly spelled "variadic"

9b31323

Use relative path for addressing things in rust-lang/rust

090c68b

Co-authored-by: Jacob Hoffman-Andrews <github@hoffman-andrews.com>

Re-add explicit list of traits to tuple docs, with limit notes

3fd1664

Add test case for #trait-implementations-1 link

c148755

notriddle force-pushed the notriddle/tuple-docs branch from e78950b to c148755 Compare June 11, 2022 16:54

rustdoc: change "variadic tuple" notation to look less like real syntax

2bbf44f

rustdoc: add test case for "variadic tuple" search notation

f1d24be

GuillaumeGomez approved these changes Jun 14, 2022

View reviewed changes

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 Jun 16, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 16, 2022

bors merged commit 6ec3993 into rust-lang:master Jun 16, 2022

rustbot added this to the 1.63.0 milestone Jun 16, 2022

notriddle deleted the notriddle/tuple-docs branch June 16, 2022 14:01

rustbot added the perf-regression Performance regression. label Jun 16, 2022

notriddle mentioned this pull request Jun 16, 2022

Improve the function pointer docs #98180

Merged

Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Jun 21, 2022

WaffleLapkin reviewed Aug 19, 2022

View reviewed changes

oli-obk mentioned this pull request Oct 26, 2023

Reorder check_item_type diagnostics so they occur next to the corresponding check_well_formed diagnostics #117213

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve the tuple and unit trait docs #97842

Improve the tuple and unit trait docs #97842

notriddle commented Jun 7, 2022

rust-highfive commented Jun 7, 2022

rust-highfive commented Jun 7, 2022

notriddle commented Jun 7, 2022

GuillaumeGomez commented Jun 7, 2022

notriddle commented Jun 7, 2022

GuillaumeGomez commented Jun 7, 2022

vacuus commented Jun 7, 2022

Manishearth commented Jun 8, 2022

notriddle commented Jun 8, 2022

jsha commented Jun 8, 2022

(T, ...)

notriddle commented Jun 9, 2022

vacuus commented Jun 9, 2022

notriddle commented Jun 9, 2022

jsha left a comment

notriddle commented Jun 14, 2022

m-ou-se commented Jun 14, 2022

fbstj commented Jun 14, 2022

m-ou-se commented Jun 14, 2022 •

edited

Loading

GuillaumeGomez commented Jun 14, 2022

jsha commented Jun 14, 2022

notriddle commented Jun 14, 2022

GuillaumeGomez commented Jun 14, 2022

GuillaumeGomez commented Jun 14, 2022

GuillaumeGomez commented Jun 16, 2022

bors commented Jun 16, 2022

bors commented Jun 16, 2022

bors commented Jun 16, 2022

rust-timer commented Jun 16, 2022

WaffleLapkin Aug 19, 2022

notriddle Aug 19, 2022

WaffleLapkin Aug 19, 2022

notriddle Aug 19, 2022

jsha commented Oct 11, 2022 via email

Improve the tuple and unit trait docs #97842

Improve the tuple and unit trait docs #97842

Conversation

notriddle commented Jun 7, 2022

rust-highfive commented Jun 7, 2022

rust-highfive commented Jun 7, 2022

notriddle commented Jun 7, 2022

GuillaumeGomez commented Jun 7, 2022

notriddle commented Jun 7, 2022

GuillaumeGomez commented Jun 7, 2022

vacuus commented Jun 7, 2022

Manishearth commented Jun 8, 2022

notriddle commented Jun 8, 2022

jsha commented Jun 8, 2022

(T, ...)

notriddle commented Jun 9, 2022

vacuus commented Jun 9, 2022

notriddle commented Jun 9, 2022

jsha left a comment

Choose a reason for hiding this comment

notriddle commented Jun 14, 2022

m-ou-se commented Jun 14, 2022

fbstj commented Jun 14, 2022

m-ou-se commented Jun 14, 2022 • edited Loading

GuillaumeGomez commented Jun 14, 2022

jsha commented Jun 14, 2022

notriddle commented Jun 14, 2022

GuillaumeGomez commented Jun 14, 2022

GuillaumeGomez commented Jun 14, 2022

GuillaumeGomez commented Jun 16, 2022

bors commented Jun 16, 2022

bors commented Jun 16, 2022

bors commented Jun 16, 2022

rust-timer commented Jun 16, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

WaffleLapkin Aug 19, 2022

Choose a reason for hiding this comment

notriddle Aug 19, 2022

Choose a reason for hiding this comment

WaffleLapkin Aug 19, 2022

Choose a reason for hiding this comment

notriddle Aug 19, 2022

Choose a reason for hiding this comment

jsha commented Oct 11, 2022 via email

m-ou-se commented Jun 14, 2022 •

edited

Loading