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

Fix various grammatical errors around ?Sized #992

Closed
wants to merge 2 commits into from

Conversation

mcclure
Copy link
Contributor

@mcclure mcclure commented Mar 25, 2021

  • Section 10.2 contains two sentences which are not sentences (I.E., they do not contain verbs). Patch makes the sentences grammatical and less ambugious, also links on the phrase ?Sized to the doc for ?Sized.
  • Section 10.6 contains the text '? is only used to declare that the Sized trait may not be implemented for a type parameter or associated type'. 'May not' makes it sound as if it is prohibited to implement the Sized trait, whereas in fact the text means that the type may at its option fail to implement the sized trait. Patch clarifies.

I am not completely certain about this text. I think my text is better (because the existing text is not valid English) but my text is definitely more ungainly. (I would love to find an excuse to remove the words "a non-trait item" in the first bullet point.) I also do not know for a fact that my replacement text is correct— it is my best guess from looking at the other documentation (in particular Sized in the stdlib). I am new at Rust and have yet to learn traits.

[EDIT: Removed note about ?sized link]

* Section 10.2 contains two sentences which are not sentences (I.E., they do not contain verbs). Patch makes the sentences grammatical and less ambugious, also links on the phrase ?Sized to the stdlib doc for Sized which is the clearest explanation of what ?Sized does I can find.
* Section 10.6 contains the text '? is only used to declare that the Sized trait may not be implemented for a type parameter or associated type'. 'May not' makes it sound as if it is prohibited to implement the Sized trait, whereas in fact the text means that the type may at its option fail to implement the sized trait. Patch clarifies.
@mcclure
Copy link
Contributor Author

mcclure commented Mar 25, 2021

Addresses #987

@mcclure
Copy link
Contributor Author

mcclure commented Mar 26, 2021

A note: The best explanation of ?Sized I've found is attached to Sized, in the stdlib.

In the current version of this patch, when I mention ?Sized I link to "?Sized" in the reference.

This means the current status of Sized/?Sized documentation is:

  • Stdlib— best documentation.
  • Second-least-clear documentation, now that I've added additional verbosity in this patch: In the dynamically sized type section, 10.2.
  • Third- and fourth- least clear documentation: "Sized" is defined in 11 "Special Types and Traits" and "?Sized" is defined in 10.6 "Trait and lifetime bounds". Neither 10.6 or 11 make it clear what the defaults are (IE: default ?sized for trait items, default sized for everything else) and the description of defaults in 10.6 is, to my read, actively misleading.

This all feels real backward. Getting the defaults wrong/misleading in the reference is especially concerning because this (defaults) is the exact material which is relevant for the reference rather than the stdlib. This PR currently constitutes a minimal change to make the text readable, but in my opinion a better way to structure this overall would be:

  • Put Sized and ?Sized together in the reference— maybe in 10.6, maybe in 11, but in only one of those places and put them next to each other, because they're clearest when read in conjunction with each other. In this single place, explain ?Sized is the default for trait type parameters and Sized is the default for all other type parameters.
  • Remove the verbosity from 10.2 and instead say something like:
    • DSTs can be provided as type arguments if the type parameter specifies ?Sized (see sized and [?sized])
    • Traits may be implemented for DSTs unless the trait specifies Sized (see sized and [?sized]).
  • Leave stdlib "Sized" doc unchanged, it's already ok

Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

Review addresses the PR specifically.

I don't see any problem with the prose. I agree it is an improvement over what is currently there. I do have a few changes, but they are just one (small but crucial) factual inconsistency and three grammar/style things.

@@ -11,10 +11,10 @@ types">DSTs</abbr>. Such types can only be used in certain cases:
* Pointers to slices also store the number of elements of the slice.
* Pointers to trait objects also store a pointer to a vtable.
* <abbr title="dynamically sized types">DSTs</abbr> can be provided as
type arguments when a bound of `?Sized`. By default any type parameter
has a `Sized` bound.
type arguments when the type parameter has the special lifetime bound [`?Sized`](trait-bounds.html#sized) specified. (In other words by default any type parameter on a non-trait item
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type arguments when the type parameter has the special lifetime bound [`?Sized`](trait-bounds.html#sized) specified. (In other words by default any type parameter on a non-trait item
type arguments when the type parameter has the special trait bound [`?Sized`](trait-bounds.html#sized) specified. (In other words by default any type parameter on a non-trait item

@@ -11,10 +11,10 @@ types">DSTs</abbr>. Such types can only be used in certain cases:
* Pointers to slices also store the number of elements of the slice.
* Pointers to trait objects also store a pointer to a vtable.
* <abbr title="dynamically sized types">DSTs</abbr> can be provided as
type arguments when a bound of `?Sized`. By default any type parameter
has a `Sized` bound.
type arguments when the type parameter has the special lifetime bound [`?Sized`](trait-bounds.html#sized) specified. (In other words by default any type parameter on a non-trait item
Copy link
Contributor

Choose a reason for hiding this comment

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

The parenthesis are not needed. The "In other words" already guards the sentence enough.

There's also needed commas after the "In other words" and "by default".

Also, as per the style guide, links in prose should just be the text and then have link refs at the bottom in alphabetical (well, UTF-8 codepoint) order.

@ehuss ehuss added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Apr 26, 2021
`?` is only used to declare that the [`Sized`] trait may not be
implemented for a type parameter or associated type. `?Sized` may
`?` is only used to declare that the [`Sized`] trait may be
unimplemented for a type parameter or associated type. `?Sized` may
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be better to say "is not required" instead of "may be unimplemented". Maybe add a parenthetical "(Type parameters and associated types have an implicit Sized bound.)" after this sentence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants