-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
rewrite documentation for unimplemented! to clarify use #64726
Conversation
r? @rkruppe (rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Returning |
@qnighy I meant regarding the example given, we could easily get it to type check without this macro. I did miss that it still allows it to compile if you try returning non-unit types. I'll edit my pull request to demonstrate this. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
r? @hellow554 |
Ping from triage. @rkruppe @andrewbanchich |
Oops, this slipped under my radar, sorry! I don't think @hellow554 has power over @bors so although their review is much appreciated they ultimately can't merge the PR. I'll take a look now. |
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 think it's great to be more explicit about what the macro does exactly and to mention how it accepts extra arguments. Thanks for the PR and sorry again for not responding earlier.
But I'm a bit conflicted about other parts of this diff which seem neutral or negative to me (see notes below). On the other hand, some of that is subjective and I certainly don't want a bikeshed out of it. I guess I'll make some suggestions and if you agree you can incorporate them and if not I'll just shrug and merge anyway (for the subjective stuff, there is one small thing I do care strongly about).
Thanks! @bors r+ rollup |
📌 Commit b7091e4 has been approved by |
@rkruppe I think it still needs you to accept the changes in your review. |
…uppe rewrite documentation for unimplemented! to clarify use The current docs for `unimplemented!` seem to miss the point of this macro. > This can be useful if you are prototyping and are just looking to have your code type-check, or if you're implementing a trait that requires multiple methods, and you're only planning on using one of them. You could also return a `()` if you just want your code to type-check. I think `unimplemented!` is useful for when you want your program to exit when it reaches an unimplemented area. I rewrote the explanation and gave examples of both forms of this macro that I think clarify its use a little better.
Rollup of 8 pull requests Successful merges: - #64726 (rewrite documentation for unimplemented! to clarify use) - #65040 (syntax: more cleanups in item and function signature parsing) - #65046 (Make `Cell::new` method come first in documentation) - #65098 (Add long error explanation for E0561) - #65150 (Suggest dereferencing boolean reference when used in 'if' or 'while') - #65154 (Fix const generic arguments not displaying in types mismatch diagnostic) - #65181 (fix bug in folding for constants) - #65187 (use 'invalid argument' for vxWorks) Failed merges: - #65179 (Add long error explanation for E0567) r? @ghost
The current docs for
unimplemented!
seem to miss the point of this macro.You could also return a
()
if you just want your code to type-check.I think
unimplemented!
is useful for when you want your program to exit when it reaches an unimplemented area.I rewrote the explanation and gave examples of both forms of this macro that I think clarify its use a little better.