Rust: Tracking issue for `Cow::is_borrowed` and `Cow::is_owned`

Created on 6 Oct 2019  路  8Comments  路  Source: rust-lang/rust

I just tried to use these in my own code and was kind of shocked they didn't exist.

Justification: this seems like a common Rust pattern. We have is_some and is_none for Option, is_ok and is_err for Result, etc., so, it seems pretty fair to have is_borrowed and is_owned for Cow.

Having as_borrowed and as_owned wouldn't really make much sense, as a simple & and &mut/to_mut cover those use cases. But, these check functions are pretty useful on their own.

B-unstable C-tracking-issue Libs-Tracked T-libs

Most helpful comment

Procedural: I鈥檝e edited the title and labels to reflect that this is not implemented.

My opinion: I鈥檓 not sure that these method carry their weight, since if let Cow::Borrowed(_) = foo or matches!(foo, Cow::Borrowed(_)) can be used. Neither of these existed when Option::is_some was added, and Option is much more common than Cow.

All 8 comments

I'm no expert on how backwards compatibility is handled. But Cow implements Deref, which means these new methods wouldn't be backwards compatible as they'd conflict with methods of the same name on the inner object. To fix this you'll have to change them to take cow: &Self, similar to what most of the methods on Box/Rc etc. take.

Procedural: I鈥檝e edited the title and labels to reflect that this is not implemented.

My opinion: I鈥檓 not sure that these method carry their weight, since if let Cow::Borrowed(_) = foo or matches!(foo, Cow::Borrowed(_)) can be used. Neither of these existed when Option::is_some was added, and Option is much more common than Cow.

@petertodd That's fair; that said, though, I think that having methods named is_borrowed or is_owned on a type which implements ToOwned would be extremely unusual, because to_owned would not affect said methods.

@SimonSapin I agree that Option is much more common than Cow, but I feel that these are still useful enough to carry their own weight. This is partially because I was writing code and was honestly surprised they didn't exist, so, I'm already pre-biased to assume that they should exist.

Mostly, I figured that adding these methods would be worthwhile because they're unlikely to clash with existing ones, and can incubate in nightly for a while to see if they do carry their weight. If they don't, they can be removed.

It appears that this is now merged.

@SimonSapin it would make sense to revert the changes to the title and tags, or discuss here whether the change should be reverted.

Done.

Sorry for the somewhat-pointless churn, I鈥檇 gotten here from https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3AC-tracking-issue+label%3AT-libs and hadn鈥檛 realized there was an implementation PR already.

All good! Thanks for updating things :)

Was this page helpful?
0 / 5 - 0 ratings