-
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
Clarify that Cow::into_owned
returns owned data
#94022
Conversation
r? @m-ou-se rust-highfive didn't tag this PR, so pinging you as library team lead. |
@rustbot label S-waiting-on-review |
library/alloc/src/borrow.rs
Outdated
@@ -307,7 +306,7 @@ impl<B: ?Sized + ToOwned> Cow<'_, B> { | |||
/// ); | |||
/// ``` | |||
/// | |||
/// Calling `into_owned` on a `Cow::Owned` is a no-op: | |||
/// Calling `into_owned` on a `Cow::Owned` returns the owned data: |
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 important to explicitly say that it's the original owned data and doesn't involve any new allocations/newly cloned data.
Maybe something like this:
/// Calling `into_owned` on a `Cow::Owned` returns the owned data: | |
/// Calling `into_owned` on a `Cow::Owned` returns the owned data unmodified, without cloning it: |
(I see that the example doesn't really show that. Maybe a better example would show that the pointer (cow.as_ptr()
) doesn't change before and after into_owned()
.)
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've updated the text to say that the value is moved and not cloned. I did not change the example, as the two examples demonstrate that we get the same outcome regardless of the Cow variant. Adding code to the Owned version to prove that the additional statement is true would obscure this.
667fda9
to
9f4934e
Compare
@rustbot label: +S-waiting-on-review -S-waiting-on-author |
@bors r+ rollup |
📌 Commit 9f4934e has been approved by |
…an-DPC Clarify that `Cow::into_owned` returns owned data Two sections of the `Cow::into_owned` docs imply that `into_owned` returns a `Cow`. Clarify that it returns the underlying owned object, either cloned or extracted from the `Cow`.
…an-DPC Clarify that `Cow::into_owned` returns owned data Two sections of the `Cow::into_owned` docs imply that `into_owned` returns a `Cow`. Clarify that it returns the underlying owned object, either cloned or extracted from the `Cow`.
…laumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#94022 (Clarify that `Cow::into_owned` returns owned data) - rust-lang#94703 (Fix codegen bug in "ptx-kernel" abi related to arg passing) - rust-lang#95949 (Implement Default for AssertUnwindSafe) - rust-lang#96361 (Switch JS code to ES6) - rust-lang#96372 (Suggest calling method on nested field when struct is missing method) - rust-lang#96386 (simplify `describe_field` func in borrowck's diagnostics part) - rust-lang#96400 (Correct documentation for `Rvalue::ShallowInitBox`) - rust-lang#96415 (Remove references to git.io) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Two sections of the
Cow::into_owned
docs imply thatinto_owned
returns aCow
. Clarify that it returns the underlying owned object, either cloned or extracted from theCow
.