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
Update destructuring 2.md #63
Conversation
I'm very new to Rust and the changes I proposed may either be incorrect or not fit in the flow of the document. Please feel free to suggest modifications that I can make or to reject altogether. Thanks.
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.
Thanks for the PR! I left a comment. If you're not keen on changing all of that, I'd be happy to take the PR with just a minor change to talk about implementing rather than (or as well as) deriving.
destructuring 2.md
Outdated
You can also choose to have copy semantics for structs defined by you. | ||
Use `#[derive(Copy)]` to implement the `Copy` trait and the struct would | ||
have Copy semantics. You may also need to derive the `Clone` trait needed | ||
by the `Copy` trait. |
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.
This is an improvement but it is not quite complete. So, I would say that to have copy semantics you have to implement Copy
and derive
is one way to do that. It is also worth noting that there restrictions on whether a type can have the Copy
trait implemented - in particular, the destructor thing (see https://doc.rust-lang.org/std/marker/trait.Copy.html#when-can-my-type-be-copy for details).
Add information about destructors and their effect on allowing `Copy` trait to be implemented by the struct in question.
Thanks you for taking the time to comment (and of course the very wonderfully written series). I (think I) have addressed your comment and put back some destructor related content from my previous deletion. Please let me know if any more changes are required. |
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.
Thanks for the changes. Could you also add a sentence that to implement Copy
, all fields of a type must also implement Copy
.
destructuring 2.md
Outdated
fields are examined and if any of those do then the whole object has move | ||
semantics. And so on down the object structure. If no destructors are found | ||
anywhere in an object, then it has copy semantics. | ||
You can also choose to have copy semantics for user-defined structs |
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.
s/structs/types
destructuring 2.md
Outdated
anywhere in an object, then it has copy semantics. | ||
You can also choose to have copy semantics for user-defined structs | ||
by implementing the `Copy` trait. One straightforward way to do that is | ||
to add `#[derive(Copy)]` before the definition of the struct. Not all |
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.
and here
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.
and below in a few places
destructuring 2.md
Outdated
those with a destructor. Destructors probably need a post of their own, | ||
but for now, an object in Rust has a destructor if it implements the `Drop` | ||
trait. Just like C++, the destructor is executed just before an object is | ||
destroyed. If an object has a destructor then it has move semantics. |
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 would drop this last sentence
Sorry about the delay, I was going to blame github but I checked and there indeed was an email notifying me about your review. I've addressed your comments: 1) Mention that all fields of a type must implement `Copy` for that type to implement `Copy` 2) Replace the word struct with types in a few places. (I've left it at places where it was a keyword) 3) Dropped the sentence- "if an object has a destructor then it has move semantics" Feel free to let me know if there's still things that I should improve- hopefully I'll see the email in time
I've addressed your comments, please let me know if any other changes are required. |
Thanks for the changes, looks good! |
I'm very new to Rust and the changes I proposed may either be incorrect or not fit in the flow of the document. Please feel free to suggest modifications that I can make or to reject altogether. Thanks.