docs(marker/copy): provide example for `&T` being `Copy` #75049

janriemer · 2020-08-02T12:59:51Z

Edited 2020-08-16 (most recent)

In the current documentation about the Copy marker trait, there is a section
with examples of structs that can implement Copy. Currently there is no example for
showing that shared references (&T) are also Copy.
It is worth to have a dedicated example for &T being Copy, because shared
references are an integral part of the language and it being Copy is not as
intuitive as other types that share this behaviour like i32 or bool.

The example picks up on the previous non-Copy struct and shows that
structs can be Copy, even when they hold a shared reference to a non-Copy type.

Edited 2020-08-02, 3:28 p.m.

I've just realized that it says "in addition to the implementors listed below", which makes this PR kind of "wrong", because &T is indeed in the "implementors listed below".
Maybe we can instead show an example with &T in the When can my type be Copy section.

What I really want to achieve is that it becomes more obvious that &T is also Copy, because, I think, it is very valuable to know and it wasn't obvious for me, until I read something about it in a forum post.

What do you think? I would create another PR for that.
Please feel free to close this PR.

Original post

In the current documentation about the Copy marker trait, there is a section
about "additional implementors", which list additional implementors of the Copy trait.
The fact that shared references are also Copy is mixed with another point,
which makes it hard to recognize and make it seem not as important.

This clarifies the fact that shared references are also Copy, by mentioning it as a
separate item in the list of "additional implementors".

In the current documentation about the `Copy` marker trait, there is a section about "additional implementors", which list additional implementors of the `Copy` trait. The fact that shared references are also `Copy` is mixed with another point, which makes it hard to recognize and make it seem not as important. This clarifies the fact that shared references are also `Copy`, by mentioning it as a separate item in the list of "additional implementors".

rust-highfive · 2020-08-02T12:59:54Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

jyn514 · 2020-08-08T03:11:55Z

Feel free to update this PR instead of making a new one, I think it would be nice to keep the discussion in one place.

In the current documentation about the `Copy` marker trait, there is a section with examples of structs that can implement `Copy`. Currently there is no example for showing that shared references (`&T`) are also `Copy`. It is worth to have a dedicated example for `&T` being `Copy`, because shared references are an integral part of the language and it being `Copy` is not as intuitive as other types that share this behaviour like `i32` or `bool`. The example picks up on the previous non-`Copy` struct and shows that structs can be `Copy`, even when they hold a shared reference to a non-`Copy` type.

library/core/src/marker.rs

Co-authored-by: Joshua Nelson <joshua@yottadb.com>

library/core/src/marker.rs

Co-authored-by: Joshua Nelson <joshua@yottadb.com>

janriemer · 2020-08-16T18:52:36Z

So ironic that the ghost emoji has disappeared. 😆 But, it is now back! 😃
@jyn514 Thank you for your fast responses. 😘

jyn514 · 2020-08-16T18:56:20Z

No problem :)

Maybe @poliorcetics is interested in reviewing the doc change?

poliorcetics · 2020-08-16T18:57:43Z

I'll have a look later. :)

library/core/src/marker.rs

Co-authored-by: Poliorcetics <poliorcetics@users.noreply.github.com>

Code blocks in doc comments are compiled and run, so we show `Copy` works in this example. Co-authored-by: Poliorcetics <poliorcetics@users.noreply.github.com>

poliorcetics · 2020-08-16T21:02:55Z

Looks good to me, thanks !

I don't have the right to r+ for bors though, you'll have to wait for someone who does. 😄

janriemer · 2020-08-16T21:14:15Z

@poliorcetics Thank you for your swift review! I really appreciate it! ❤️
My comment on the derive might have been hidden from you, because the conversation has been marked as "resolved" (I now have "unresolved" it), so I repeat it here. 😉

Do you think we should simultaneously add the same derive to the Point struct in line 294 for consistency?

poliorcetics · 2020-08-16T21:30:38Z

Oh yeah didn't see it sorry. Adding it above too is a good idea !

This adds another `derive` for a `Copy`able struct, so that we are consistent with `derive` annotations.

janriemer · 2020-08-18T16:49:41Z

@poliorcetics cc @jyn514 Sorry, for the late response. All the necessary changes from your review are now present.

jyn514 · 2020-08-18T16:53:19Z

@bors r=poliorcetics rollup

bors · 2020-08-18T16:53:20Z

📌 Commit 522d177 has been approved by poliorcetics

docs(marker/copy): provide example for `&T` being `Copy` ### Edited 2020-08-16 (most recent) In the current documentation about the `Copy` marker trait, there is a section with examples of structs that can implement `Copy`. Currently there is no example for showing that shared references (`&T`) are also `Copy`. It is worth to have a dedicated example for `&T` being `Copy`, because shared references are an integral part of the language and it being `Copy` is not as intuitive as other types that share this behaviour like `i32` or `bool`. The example picks up on the previous non-`Copy` struct and shows that structs can be `Copy`, even when they hold a shared reference to a non-`Copy` type. ----------------------------------------- ### Edited 2020-08-02, 3:28 p.m. I've just realized that it says "in addition to the **implementors listed below**", which makes this PR kind of "wrong", because `&T` is indeed in the "implementors listed below". Maybe we can instead show an example with `&T` in the [When can my type be Copy](https://doc.rust-lang.org/std/marker/trait.Copy.html#when-can-my-type-be-copy) section. What I really want to achieve is that it becomes more obvious that `&T` is also `Copy`, because, I think, it is very valuable to know and it wasn't obvious for me, until I read something about it in a forum post. What do you think? I would create another PR for that. **Please feel free to close this PR.** ----------------------------------- ### Original post In the current documentation about the `Copy` marker trait, there is a section about "additional implementors", which list additional implementors of the `Copy` trait. The fact that shared references are also `Copy` is mixed with another point, which makes it hard to recognize and make it seem not as important. This clarifies the fact that shared references are also `Copy`, by mentioning it as a separate item in the list of "additional implementors".

@ghost

Rollup of 9 pull requests Successful merges: - rust-lang#75038 (See also X-Link mem::{swap, take, replace}) - rust-lang#75049 (docs(marker/copy): provide example for `&T` being `Copy`) - rust-lang#75499 (Fix documentation error) - rust-lang#75554 (Fix clashing_extern_declarations stack overflow for recursive types.) - rust-lang#75646 (Move to intra doc links for keyword documentation) - rust-lang#75652 (Resolve true and false as booleans) - rust-lang#75658 (Don't emit "is not a logical operator" error outside of associative expressions) - rust-lang#75665 (Add doc examples coverage) - rust-lang#75685 (Switch to intra-doc links in /src/sys/unix/ext/*.rs) Failed merges: r? @ghost

janriemer · 2020-08-19T17:30:26Z

@poliorcetics @jyn514 Thank you for your swift responses and this fast merge!❤ It was my first contribution to Rust and I've felt very welcome. I hope, I can contribute some more in the future.🤗
You are all doing a great job for Rust and the open source community! 👍

jyn514 · 2020-08-19T17:44:22Z

No problem! Thanks for your contribution :)

poliorcetics · 2020-08-19T19:39:15Z

Thanks for the contribution :) if you continue to do docs PR you'll probably see us often !

Welcome to the open source world !

rust-highfive assigned kennytm Aug 2, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 2, 2020

janriemer changed the title ~~docs(marker/copy): clarify that &T is also Copy~~ docs(marker/copy): provide example for &T being Copy Aug 16, 2020

jyn514 reviewed Aug 16, 2020

View reviewed changes

library/core/src/marker.rs Outdated Show resolved Hide resolved

library/core/src/marker.rs Outdated Show resolved Hide resolved

rephrase: struct -> type

43dec0e

Co-authored-by: Joshua Nelson <joshua@yottadb.com>

jyn514 reviewed Aug 16, 2020

View reviewed changes

library/core/src/marker.rs Outdated Show resolved Hide resolved

add back emojis that have been removed accidentally

dce8644

Co-authored-by: Joshua Nelson <joshua@yottadb.com>

poliorcetics reviewed Aug 16, 2020

View reviewed changes

library/core/src/marker.rs Show resolved Hide resolved

library/core/src/marker.rs Show resolved Hide resolved

janriemer and others added 2 commits August 16, 2020 22:15

add empty line above code block

9061da2

Co-authored-by: Poliorcetics <poliorcetics@users.noreply.github.com>

docs: add derive for struct

56daf63

Code blocks in doc comments are compiled and run, so we show `Copy` works in this example. Co-authored-by: Poliorcetics <poliorcetics@users.noreply.github.com>

docs: add another derive for Copyable struct

522d177

This adds another `derive` for a `Copy`able struct, so that we are consistent with `derive` annotations.

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 18, 2020

tmandry mentioned this pull request Aug 18, 2020

Rollup of 10 pull requests #75683

Closed

tmandry mentioned this pull request Aug 18, 2020

Rollup of 10 pull requests #75684

Closed

tmandry mentioned this pull request Aug 19, 2020

Rollup of 11 pull requests #75688

Closed

tmandry mentioned this pull request Aug 19, 2020

Rollup of 10 pull requests #75689

Closed

tmandry mentioned this pull request Aug 19, 2020

Rollup of 11 pull requests #75690

Closed

JohnTitor mentioned this pull request Aug 19, 2020

Rollup of 9 pull requests #75692

Merged

bors merged commit 1f4f317 into rust-lang:master Aug 19, 2020

janriemer deleted the patch-1 branch August 19, 2020 17:21

janriemer mentioned this pull request Jan 15, 2022

Leave the Rustdoc team rust-lang/team#689

Merged

cuviper added this to the 1.47.0 milestone May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(marker/copy): provide example for `&T` being `Copy` #75049

docs(marker/copy): provide example for `&T` being `Copy` #75049

janriemer commented Aug 2, 2020 •

edited

Loading

rust-highfive commented Aug 2, 2020

jyn514 commented Aug 8, 2020

janriemer commented Aug 16, 2020

jyn514 commented Aug 16, 2020

poliorcetics commented Aug 16, 2020

poliorcetics commented Aug 16, 2020

janriemer commented Aug 16, 2020

poliorcetics commented Aug 16, 2020

janriemer commented Aug 18, 2020

jyn514 commented Aug 18, 2020

bors commented Aug 18, 2020

janriemer commented Aug 19, 2020

jyn514 commented Aug 19, 2020

poliorcetics commented Aug 19, 2020

docs(marker/copy): provide example for &T being Copy #75049

docs(marker/copy): provide example for &T being Copy #75049

Conversation

janriemer commented Aug 2, 2020 • edited Loading

Edited 2020-08-16 (most recent)

Edited 2020-08-02, 3:28 p.m.

Original post

rust-highfive commented Aug 2, 2020

jyn514 commented Aug 8, 2020

janriemer commented Aug 16, 2020

jyn514 commented Aug 16, 2020

poliorcetics commented Aug 16, 2020

poliorcetics commented Aug 16, 2020

janriemer commented Aug 16, 2020

poliorcetics commented Aug 16, 2020

janriemer commented Aug 18, 2020

jyn514 commented Aug 18, 2020

bors commented Aug 18, 2020

janriemer commented Aug 19, 2020

jyn514 commented Aug 19, 2020

poliorcetics commented Aug 19, 2020

docs(marker/copy): provide example for `&T` being `Copy` #75049

docs(marker/copy): provide example for `&T` being `Copy` #75049

janriemer commented Aug 2, 2020 •

edited

Loading