-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
impl Clone for Cow #19359
impl Clone for Cow #19359
Conversation
Just want to add that due to the bad interaction of struct bounds and the This compiles: use std::vec::CowVec;
#[deriving(Clone)]
struct Foo<'a, T: 'a> where T: Clone {
bar: CowVec<'a, T>,
}
fn main() {} But this does not: use std::vec::CowVec;
#[deriving(Clone)]
struct Foo<'a, T: 'a + Clone> {
bar: CowVec<'a, T>,
}
fn main() {} |
Borrowed(b) => Borrowed(b), | ||
Owned(ref o) => { | ||
let b: &B = BorrowFrom::borrow_from(o); | ||
Owned(b.to_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 must confess that it feels weird to me to always into_owned a Clone of an owned Cow. I imagine that always producing a Borrow would have unfortunate consequences? Can you provide an example?
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.
If Self = SendStr = CowString<'static>
. Cloning that can't return a Borrowed
value, since that would need an inner &'static str
, and you can't produce &'static str
from String
.
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.
Should we explicitly document the behaviour, since it's subtle and might be unexpected? Or do you think it can be considered an implementation detail?
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.
Another option would be to require T: Clone
instead of B: BorrowFrom<T>
so you could just call .clone()
on the owned variant (which may be more expected)
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.
require T: Clone instead of B: BorrowFrom
Note that the Cow
struct enforces the B: BorrowFrom<T>
bound, so it can't be removed. I wrote the impl
in this way to reduce the numbers of bounds on the input parameters.
Now we can use
#[deriving(Clone)]
on structs that containCow
.r? @aturon or anyone else