Add missing examples on core traits' method#76568
Conversation
rust-highfive
added
the
S-waiting-on-review
jyn514
added
the
A-docs
jyn514
left a comment
jyn514
left a comment
There was a problem hiding this comment.
LGTM with nits fixed for everything besides *Assign
GuillaumeGomez
force-pushed
the
add-missing-examples
branch
from
0933ae5 to
d7a9707
Compare
|
Updated! |
| /// let x: i32 = 12; | ||
| /// assert_eq!(-x, -12); |
There was a problem hiding this comment.
I was going to say something about assert_eq!(-12, -12) and then I realized that was silly lol. So this is fine.
There was a problem hiding this comment.
How about assert_eq!(0, -0)?
There was a problem hiding this comment.
These suggestions are about the meaning of the operations, you're showing off the edge cases. I don't think this is a good place to document edge cases, this is just showing which operations the trait corresponds to.
There was a problem hiding this comment.
In particular, all these suggestions only make sense for numbers and would be completely out of place if it used Duration or something instead.
There was a problem hiding this comment.
Well, edge cases are considered part of the meanings too, of course users wouldn't want surprises. Or maybe do you have a better idea where to put these edge cases?
There was a problem hiding this comment.
I'd document them on the implementations, not the traits themselves. Take a look at library/core/src/num, maybe. But they certainly don't belong here - this is for every type that implements the trait, not just numbers.
|
@bors r+ rollup |
|
📌 Commit d7a9707 has been approved by |
bors
added
S-waiting-on-bors
| /// # Example | ||
| /// | ||
| /// ``` | ||
| /// assert_eq!(12 / 2, 6); |
There was a problem hiding this comment.
?? 12/0 is a compile time error: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=750633198d7a3b2f68deffac99eed104
There was a problem hiding this comment.
Exactly, I think it would be good to have another example to show that it will fail to compile.
| /// | ||
| /// ``` | ||
| /// let mut x: u32 = 12; | ||
| /// x /= 2; |
| /// | ||
| /// ``` | ||
| /// assert_eq!(12 + 1, 13); | ||
| /// ``` |
There was a problem hiding this comment.
Should we mention somewhere what happens when add overflow on debug build and release build? IIRC it behaves differently.
Rollup of 11 pull requests Successful merges: - rust-lang#75857 (Syntactically permit unsafety on mods) - rust-lang#76289 (Add docs about crate level documentation support) - rust-lang#76514 (Add revisions to const generic issue UI tests.) - rust-lang#76524 (typeck: don't suggest inaccessible private fields) - rust-lang#76548 (Validate removal of AscribeUserType, FakeRead, and Shallow borrow) - rust-lang#76555 (Reword `trivial_casts` lint in rustc book to better explain what it does.) - rust-lang#76559 (add the `const_evaluatable_checked` feature) - rust-lang#76563 (small typo fix in rustc_parse docs) - rust-lang#76565 (take reference to Place directly instead of taking reference to Box<Place>) - rust-lang#76567 (use push(char) to add chars (single-char &strs) to strings instead of push_str(&str)) - rust-lang#76568 (Add missing examples on core traits' method) Failed merges: r? `@ghost`
6 participants
Linked to #76450.
r? @jyn514