-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Audit the option
and ptr
types for proper integer usage
#22294
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
Conversation
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
/// assert_eq!(x.is_some(), true); | ||
/// | ||
/// let x: Option<uint> = None; | ||
/// let x: Option<usize> = None; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to leave these as usize
rather than i32
? (for pedagogy against using usize randomly)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these can actually be inferred if we want i32, although I dunno if that would "mess up" the quality of the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured the explicit types were there for a reason (in particular, type inference is hard on the reader, and these docs are largely targeting total newbies). In that case, I don't know that it matters what type is there. But I guess I can change it to u32
or i32
.
Looks good, although there are a lot of examples using r=me all except that one are changed to |
@huonw updated the option examples and threw on a few more modules. |
@bors r+ 6171 rollup |
cc #22240