std: clarify Clone trait documentation about duplication semantics #141215
+37
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
My role (T-compiler) does not involve approving libs api docs, sorry. r? libs-api |
rustbot
added
the
T-libs-api
fee1-dead
removed
the
T-libs
workingjubilee
requested changes
May 30, 2025
library/core/src/clone.rs
Outdated
| @@ -38,7 +38,25 @@ | |||
|
|
|||
| mod uninit; | |||
|
|
|||
| /// A common trait for the ability to explicitly duplicate an object. | |||
| /// A common trait that allows explicit creation of a "duplicate" value. | |||
There was a problem hiding this comment.
No need for scare quotes. The value is in fact duplicated.
Suggested change
| /// A common trait that allows explicit creation of a "duplicate" value. | |
| /// A common trait that allows explicit creation of a duplicate value. |
library/core/src/clone.rs
Outdated
Comment on lines
43
to
44
| /// ## Important Note on "Duplication" | ||
| /// |
There was a problem hiding this comment.
I don't think this heading adds much? At worst, it's a little confusing due to what else gets moved under it.
Suggested change
| /// ## Important Note on "Duplication" | |
| /// |
library/core/src/clone.rs
Outdated
Comment on lines
45
to
52
| /// What "duplication" means depends on the type implementing `Clone`: | ||
| /// | ||
| /// - For most types, calling [`clone`] creates a semantically independent copy with its own | ||
| /// unique identity and state. | ||
| /// - For smart pointers (like [`Arc`], [`Rc`]), [`clone`] increases the reference count but points | ||
| /// to the same underlying data. This means modifications to the underlying data through one | ||
| /// clone will be visible through other clones. | ||
| /// - For reference types (`&T`), [`clone`] just creates another reference to the same value. |
There was a problem hiding this comment.
I don't think that the behavior actually varies much. What is duplicated is the value.
However, some types indeed, as values, are instead references to other data. So you get a new reference. It might be more concise to simply state that you always get a new value, it's just not guaranteed that the new value does not contain references to other data that is not duplicated.
There was a problem hiding this comment.
Besides, this part of the text is just repeated on the docs for Clone::clone itself.
library/core/src/clone.rs
Outdated
Comment on lines
58
to
59
| /// [`Arc`]: ../../std/sync/struct.Arc.html | ||
| /// [`Rc`]: ../../std/rc/struct.Rc.html |
There was a problem hiding this comment.
After this we start talking about how this differs from Copy, you see, and that probably doesn't fit under the same heading.
|
This is very not an API change so anyone on libs can take it. yoink |
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
This commit improves the Clone trait documentation to address confusion around what "duplication" means for different types, especially for smart pointers like Arc<Mutex<T>>. Signed-off-by: xizheyin <[email protected]>
xizheyin
commented
May 31, 2025
Comment on lines
+43
to
+46
| /// Calling [`clone`] always produces a new value. | ||
| /// However, for types that are references to other data (such as smart pointers or references), | ||
| /// the new value may still point to the same underlying data, rather than duplicating it. | ||
| /// See [`Clone::clone`] for more details. |
There was a problem hiding this comment.
I don't categorize it here, I'm just reminding the user of the situation.
Comment on lines
+159
to
+168
| /// Returns a duplicate of the value. | ||
| /// | ||
| /// Note that what "duplicate" means varies by type: | ||
| /// - For most types, this creates a deep, independent copy | ||
| /// - For reference types like `&T`, this creates another reference to the same value | ||
| /// - For smart pointers like [`Arc`] or [`Rc`], this increments the reference count | ||
| /// but still points to the same underlying data | ||
| /// | ||
| /// [`Arc`]: ../../std/sync/struct.Arc.html | ||
| /// [`Rc`]: ../../std/rc/struct.Rc.html |
There was a problem hiding this comment.
Do I need to change it here, also similar to Trait Clone?
There was a problem hiding this comment.
Nope, I think the summary list actually fits better here. Thanks!
rustbot
added
S-waiting-on-review
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
May 31, 2025
…bilee std: clarify Clone trait documentation about duplication semantics Closes rust-lang#141138 The change explicitly explains that cloning behavior varies by type and clarifies that smart pointers (`Arc`, `Rc`) share the same underlying data. I've also added an example of cloning to Arc.
This was referenced May 31, 2025
bors
added a commit
that referenced
this pull request
Jun 1, 2025
Rollup of 6 pull requests Successful merges: - #141072 (Stabilize feature `result_flattening`) - #141215 (std: clarify Clone trait documentation about duplication semantics) - #141277 (Miri CI: test aarch64-apple-darwin in PRs instead of the x86_64 target) - #141521 (Add `const` support for float rounding methods) - #141812 (Fix "consider borrowing" for else-if) - #141832 (library: explain TOCTOU races in `fs::remove_dir_all`) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Jun 1, 2025
Rollup merge of #141215 - xizheyin:issue-141138, r=workingjubilee std: clarify Clone trait documentation about duplication semantics Closes #141138 The change explicitly explains that cloning behavior varies by type and clarifies that smart pointers (`Arc`, `Rc`) share the same underlying data. I've also added an example of cloning to Arc.
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Jun 1, 2025
Rollup of 6 pull requests Successful merges: - rust-lang/rust#141072 (Stabilize feature `result_flattening`) - rust-lang/rust#141215 (std: clarify Clone trait documentation about duplication semantics) - rust-lang/rust#141277 (Miri CI: test aarch64-apple-darwin in PRs instead of the x86_64 target) - rust-lang/rust#141521 (Add `const` support for float rounding methods) - rust-lang/rust#141812 (Fix "consider borrowing" for else-if) - rust-lang/rust#141832 (library: explain TOCTOU races in `fs::remove_dir_all`) r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #141138
The change explicitly explains that cloning behavior varies by type and clarifies that smart pointers (
Arc,Rc) share the same underlying data. I've also added an example of cloning to Arc.