-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Beef up Copy documentation #21261
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
Beef up Copy documentation #21261
Conversation
r? @pcwalton (rust_highfive has picked a reviewer for you, use r? to override) |
/// | ||
/// ``` | ||
/// struct MyStruct; | ||
/// impl Copy for MyStruct; |
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 this impl
will need braces after it
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.
The tests do pass...
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.
Weird...
struct Foo;
impl Copy for Foo;
foo.rs:3:18: 3:19 error: expected one of `(`, `+`, `::`, or `{`, found `;`
foo.rs:3 impl Copy for Foo;
^
r=me with doc fix |
/// | ||
/// ``` | ||
/// struct MyStruct; | ||
/// impl Copy for MyStruct {}; |
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 this semicolon will still need to be removed :(
I fixed the semicolon. @kmcallister what do you think is the right full explanation for |
Okay, I think this is good now. One more review? |
Looks great! |
/// ## When should my type be `Copy`? | ||
/// | ||
/// Generally speaking, if your type _can_ implement `Copy`, it should. There's one important thing | ||
/// to consider though: if you think your type may _not_ be able to implement `Copy` in the future, |
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.
Might also want to mention that it is sometimes useful to have types with no Drop impl but which shouldn't be Copy because they represent some kind of unique logical token or otherwise require more than just a bitwise copy to semantically copy them.
I've found success phrasing copy vs. move in terms of using values by-value, and "byte copies" and "semantic copies" and generally trying to divorce the two uses of the word "copy" because this overloading confuses people a lot.
|
Beef up Copy documentation Reviewed-by: alexcrichton
Beef up Copy documentation Reviewed-by: alexcrichton
Doing this manually as part of #21300 |
Fixes #21249
Fixes #11540
/cc @alexcrichton , @kmcallister