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

Document default values for primitive types #43252

Merged
merged 2 commits into from Jul 16, 2017

Conversation

vbrandl
Copy link
Contributor

@vbrandl vbrandl commented Jul 15, 2017

All primitive types implement the Default trait but the documentation just says Returns the "default value" for a type. and doesn't give a hint about the actual default value. I think it would be good to document the default values in a proper way.
I changed the default_impl macro to accept a doc string as a third parameter and use this string to overwrite the documentation of default() for each primitive type.
The generated documentation now looks like this:
Documentation of default() on the bool primitive

@retep998
Copy link
Member

Bikeshedding alternative phrasing:
Returns the default value of 0

@vbrandl
Copy link
Contributor Author

vbrandl commented Jul 16, 2017

Shouldn't @rust-highfive assign a reviewer to this PR? Or do I have to do it manually or just wait?

Copy link
Member

@GuillaumeGomez GuillaumeGomez left a comment

Choose a reason for hiding this comment

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

A light rewording would be better I think.

default_impl! { char, '\x00' }
default_impl! { (), (), "Returns the default value of `()`" }
default_impl! { bool, false, "Returns the default value of `false`" }
default_impl! { char, '\x00', "Returns the default value of `\\x00`" }
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be "Returns the default value of char which is \\x00?"

default_impl! { bool, false }
default_impl! { char, '\x00' }
default_impl! { (), (), "Returns the default value of `()`" }
default_impl! { bool, false, "Returns the default value of `false`" }
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be "Returns the default value ofbool which is false?"

@GuillaumeGomez GuillaumeGomez self-assigned this Jul 16, 2017
@GuillaumeGomez
Copy link
Member

Hum, actually I think it's good as is. The wording is lighter. I'll go for this one then. Thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 16, 2017

📌 Commit caf125f has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Jul 16, 2017

⌛ Testing commit caf125f with merge be18613...

bors added a commit that referenced this pull request Jul 16, 2017
Document default values for primitive types

All primitive types implement the `Default` trait but the documentation just says `Returns the "default value" for a type.` and doesn't give a hint about the actual default value. I think it would be good to document the default values in a proper way.
I changed the `default_impl` macro to accept a doc string as a third parameter and use this string to overwrite the documentation of `default()` for each primitive type.
The generated documentation now looks like this:
![Documentation of default() on the bool primitive](https://i.imgur.com/nK6TApo.png)
@bors
Copy link
Contributor

bors commented Jul 16, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: GuillaumeGomez
Pushing be18613 to master...

@bors bors merged commit caf125f into rust-lang:master Jul 16, 2017
kennytm added a commit to kennytm/rust that referenced this pull request Jul 17, 2017
After rust-lang#43252 is merged, building stage0 libcore with -i (--incremental)
flag will cause 17 "Quasi-quoting might make incremental compilation very
inefficient: NtExpr(..)" warnings, as in rust-lang#40946.

Fixing the warning in rust-lang#40946 will take 12 weeks from now to make into the
next stage0, so it is quicker to workaround it in libcore instead.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 18, 2017
…n-rustbuild, r=alexcrichton

Workaround "Quasi-quoting is inefficient" warning in incremental rustbuild introduced in rust-lang#43252.

After rust-lang#43252 is merged, building stage0 libcore with `-i` (`--incremental`) flag will cause 17 "Quasi-quoting might make incremental compilation very inefficient: NtExpr(..)" warnings, as in rust-lang#40946.

```
warning: Quasi-quoting might make incremental compilation very inefficient: NtExpr(..)
   --> src/libcore/default.rs:133:21
    |
133 |             #[doc = $doc]
    |                     ^^^^
...
139 | default_impl! { (), (), "Returns the default value of `()`" }
    | ------------------------------------------------------------- in this macro invocation
(× 17)
```

True fix for rust-lang#40946 will take at least 12 weeks from now to make into the next stage0, so it is quicker to workaround it in libcore instead.

cc @vbrandl @jseyfried
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants