-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Rename MaybeUninit
to MaybeUninitialized
#56138
Conversation
r? @RalfJung |
I know this is racing your other PR, so you get to review! |
This comment has been minimized.
This comment has been minimized.
Fine for me. @rust-lang/libs @rust-lang/lang any objections? |
What's the motivation for this change? |
Wrds r btr thn abbrs. |
For full context, if you start reading the comments here, there's an overwhelming consensus to add more words into the method names of this type to better indicate their unsafety and usage concerns. Since the point of all of those words is to force the user to think more about careful usage of this type, making sure that the type's name itself is unambiguous is a good start. |
What could |
The specific example there is |
Ok |
There is also a precedent in the standard library to avoid sometimes-well-established abbreviations: |
I actually think we should do this and would propose reopening the PR. We usually avoid abbreviations unless they are extremely widely used (like pretty much all of @withoutboats' examples). Another example is |
This comment has been minimized.
This comment has been minimized.
dca115d
to
4523174
Compare
I think some of the examples like |
Note that this replaces |
|
I certainly don't want to see this used often, but at the same time, this seems like unnecessary syntactic salt. |
My main gripe is with I don't think a lot is gained by lengthening the name of the type here but at the same time I don't feel the need for unsafe code to be overly ergonomic to write. So my reaction to this PR is pretty neutral. I'm not for nor against it -- if you can convince everyone else then I'm sold. :) |
“maybe uninitialized" is pretty commonly used term in system programming: https://stackoverflow.com/questions/14132898/gcc-wuninitialized-wmaybe-uninitialized-issues System programmers that are used to the term will make more effort to recognize "MaybeUninit" than the more commonly used fully spelled out version. It is also easier to relate "MaybeUninitialized" to the original "mem::uninitialized" method. |
☔ The latest upstream changes (presumably #54668) made this pull request unmergeable. Please resolve the merge conflicts. |
This comment has been minimized.
This comment has been minimized.
@rfcbot concern i-am-un-maybe-init @Centril claimed that I'm speaking about the benefit for human authors and code reviewers, especially those who are perhaps looking over something quickly and might not catch an occurrence of "Maybe" (even when next to "Init") but would catch "Uninit". I don't think there is a net win from renaming replacing (I don't have an opinion about the original proposal to lengthen the name to |
|
@pnkfelix I agree entirely, and I have the same objection. This not only doesn't seem worth the change, it seems less clear. |
@Centril proposal cancelled. |
I propose that we finalize the name of the type as @rfcbot merge (EDIT: this is a bit weird.. we're "merging" but this means that the PR itself will be closed...) |
Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
I don't know enough HCI to refute this claim, but it seems odd to expect people to read the middle of a word but not the beginning. Similarly, it seems odd that people are more likely to catch "Uninit" than they would be "Uninitialized". |
While I checked my name, I would like to note for the record that I prefer the name |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
I am not a native English speaker, and have not used much |
I find the mixture of abbreviations with long spellings in the API weird: Without consistency, unless I use this API all-day every-day I pretty much have to guess whenever I need it. I wish the standard library would have clear guidelines about this instead of each API doing its own thing, it would save us a lot of time (bikeshedding, context-switching to the docs because rustc does not suggest the right thing, etc.). |
The final comment period, with a disposition to merge, as per the review above, is now complete. |
So, uh, I guess we just close this PR now? |
Indeed we should; Thank you @shepmaster for your work & proposal. |
Proposal to finalize the name as
MaybeUninit
#56138 (comment) -- as in, the FCP is to close this PR.rfcbot comment is here
Changed via:
Checked via: