Skip to content
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.

Sign up for GitHub

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

Implement conversion from cow to std cow #478

Merged
merged 2 commits into from
May 5, 2024
Merged

Implement conversion from cow to std cow #478

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e30c4e7
Extract borrow from cow, implement conversion to std cow
Dav1dde May 1, 2024
0950704
remove as_borrowed
Dav1dde May 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion metrics/src/cow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ pub struct Cow<'a, T: Cowable + ?Sized + 'a> {
_lifetime: PhantomData<&'a T>,
}

impl<T> Cow<'_, T>
impl<'a, T> Cow<'a, T>
where
T: Cowable + ?Sized,
{
Expand Down Expand Up @@ -178,6 +178,35 @@ where

T::owned_from_parts(cow.ptr, &cow.metadata)
}

/// Extracts the borrowed data.
///
/// Returns `None` if the data is either shared or owned.
///
/// # Examples
///
/// ```
/// use metrics::SharedString;
///
/// let s = SharedString::from_borrowed("foobar");
/// assert_eq!(s.as_borrowed(), Some("foobar"));
///
/// let s = SharedString::from_owned("foobar".to_owned());
/// assert_eq!(s.as_borrowed(), None);
///
/// let s = SharedString::from_shared("foobar".to_owned().into());
/// assert_eq!(s.as_borrowed(), None);
/// ```
pub fn as_borrowed(&self) -> Option<&'a T> {
match self.metadata.kind() {
Kind::Owned | Kind::Shared => None,
Kind::Borrowed => {
// SAFETY: We know the contained data is borrowed from 'a, we're simply
// restoring the original immutable reference and returning a copy of it.
Some(unsafe { &*T::borrowed_from_parts(self.ptr, &self.metadata) })
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless there's a compelling reason to have this be a standalone method, we should just move the logic down into impl From<Cow<'a, T>> for std::borrow::Cow<'a, T>.

Copy link
Contributor Author

@Dav1dde Dav1dde May 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with that is, the From impl always requires a Clone (or taking ownership, but the way the SharedString is exposed it is usually through a reference), the as_borrowed() allows for a decision. In the future this can be supplemented with as_shared().

That being said, we can reduce the API surface and just impl the From (would be enough for my usecase) and see if more needs come up.

}

impl<'a, T> Cow<'a, T>
Expand Down Expand Up @@ -324,6 +353,17 @@ impl<'a> From<std::borrow::Cow<'a, str>> for Cow<'a, str> {
}
}

impl<'a, T: Cowable> From<Cow<'a, T>> for std::borrow::Cow<'a, T> {
#[inline]
fn from(value: Cow<'a, T>) -> Self {
if let Some(borrowed) = value.as_borrowed() {
Self::Borrowed(borrowed)
} else {
Self::Owned(value.into_owned())
}
}
}

impl From<String> for Cow<'_, str> {
#[inline]
fn from(s: String) -> Self {
Expand Down
Loading