From 491aec8e5b514aa373ccef400fd5636da77bc936 Mon Sep 17 00:00:00 2001 From: Zachary Harrold Date: Tue, 20 Aug 2024 09:41:46 +1000 Subject: [PATCH] Generalized `Into` and `Into` Implementations over Lifetime (#10823) # Objective - Fixes #10478 ## Solution Generalised `From/Into` implementations over `&str` and `Option<&str>` for `AssetSourceId` and `AssetPath` across all lifetimes, not just static. To maintain access to the `'static`-only specialisation, these types (and `CowArc`) now include an `as_static` method which will apply the specialisation. ```rust // Snipped from `AssetApp` fn register_asset_source( &mut self, id: impl Into>, // ^^^^^^^ // | as_static is only available for 'static lifetimes source: AssetSourceBuilder, ) -> &mut Self { let id = id.into().as_static(); // ^^^^^^ ^^^^^^^^^ // | | Specialized (internally storing CowArc::Static) // | Generic Into (internally storing CowArc::Borrowed) // ... } ``` This post-fix specialisation is available here because the actual specialisation performed is only a marker for if/when modification or ownership is required, making the transform a very cheap operation. For cleanliness, I've also added `from_static`, which wraps this behaviour in a clean shorthand designed to replace `from` calls. --- ## Changelog - Generalised the following implementations over a generic lifetime: - `From<&'static str> for AssetSourceId<'static>` - `From> for AssetSourceId<'static>` - `From<&'static str> for AssetPath<'static>` - `From<&'static Path> for AssetPath<'static>` - Added `as_static` specialisation to: - `CowArc` - `AssetSourceId` - `AssetPath` - Added `from_static` specialised constructor to: - `AssetSourceId` - `AssetPath` ## Migration Guide In areas where these implementations where being used, you can now add `from_static` in order to get the original specialised implementation which avoids creating an `Arc` internally. ```rust // Before let asset_path = AssetPath::from("my/path/to/an/asset.ext"); // After let asset_path = AssetPath::from_static("my/path/to/an/asset.ext"); ``` To be clear, this is only required if you wish to maintain the performance benefit that came with the specialisation. Existing code is _not_ broken by this change. --- crates/bevy_asset/src/io/source.rs | 31 ++++++++++++++++----- crates/bevy_asset/src/lib.rs | 2 +- crates/bevy_asset/src/path.rs | 43 +++++++++++++++++++++++++----- crates/bevy_utils/src/cow_arc.rs | 12 +++++++++ 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/crates/bevy_asset/src/io/source.rs b/crates/bevy_asset/src/io/source.rs index 1ac6b65c22dd0..a979a3327791e 100644 --- a/crates/bevy_asset/src/io/source.rs +++ b/crates/bevy_asset/src/io/source.rs @@ -70,9 +70,26 @@ impl<'a> AssetSourceId<'a> { } } -impl From<&'static str> for AssetSourceId<'static> { - fn from(value: &'static str) -> Self { - AssetSourceId::Name(value.into()) +impl AssetSourceId<'static> { + /// Indicates this [`AssetSourceId`] should have a static lifetime. + #[inline] + pub fn as_static(self) -> Self { + match self { + Self::Default => Self::Default, + Self::Name(value) => Self::Name(value.as_static()), + } + } + + /// Constructs an [`AssetSourceId`] with a static lifetime. + #[inline] + pub fn from_static(value: impl Into) -> Self { + value.into().as_static() + } +} + +impl<'a> From<&'a str> for AssetSourceId<'a> { + fn from(value: &'a str) -> Self { + AssetSourceId::Name(CowArc::Borrowed(value)) } } @@ -82,10 +99,10 @@ impl<'a, 'b> From<&'a AssetSourceId<'b>> for AssetSourceId<'b> { } } -impl From> for AssetSourceId<'static> { - fn from(value: Option<&'static str>) -> Self { +impl<'a> From> for AssetSourceId<'a> { + fn from(value: Option<&'a str>) -> Self { match value { - Some(value) => AssetSourceId::Name(value.into()), + Some(value) => AssetSourceId::Name(CowArc::Borrowed(value)), None => AssetSourceId::Default, } } @@ -302,7 +319,7 @@ pub struct AssetSourceBuilders { impl AssetSourceBuilders { /// Inserts a new builder with the given `id` pub fn insert(&mut self, id: impl Into>, source: AssetSourceBuilder) { - match id.into() { + match AssetSourceId::from_static(id) { AssetSourceId::Default => { self.default = Some(source); } diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 5a1c4c7756d5d..e04a14e06bd0b 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -341,7 +341,7 @@ impl AssetApp for App { id: impl Into>, source: AssetSourceBuilder, ) -> &mut Self { - let id = id.into(); + let id = AssetSourceId::from_static(id); if self.world().get_resource::().is_some() { error!("{} must be registered before `AssetPlugin` (typically added as part of `DefaultPlugins`)", id); } diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 3c23fff5ad712..dc7719a25f9ad 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -475,14 +475,43 @@ impl<'a> AssetPath<'a> { } } -impl From<&'static str> for AssetPath<'static> { +impl AssetPath<'static> { + /// Indicates this [`AssetPath`] should have a static lifetime. #[inline] - fn from(asset_path: &'static str) -> Self { + pub fn as_static(self) -> Self { + let Self { + source, + path, + label, + } = self; + + let source = source.as_static(); + let path = path.as_static(); + let label = label.map(CowArc::as_static); + + Self { + source, + path, + label, + } + } + + /// Constructs an [`AssetPath`] with a static lifetime. + #[inline] + pub fn from_static(value: impl Into) -> Self { + value.into().as_static() + } +} + +impl<'a> From<&'a str> for AssetPath<'a> { + #[inline] + fn from(asset_path: &'a str) -> Self { let (source, path, label) = Self::parse_internal(asset_path).unwrap(); + AssetPath { source: source.into(), - path: CowArc::Static(path), - label: label.map(CowArc::Static), + path: CowArc::Borrowed(path), + label: label.map(CowArc::Borrowed), } } } @@ -501,12 +530,12 @@ impl From for AssetPath<'static> { } } -impl From<&'static Path> for AssetPath<'static> { +impl<'a> From<&'a Path> for AssetPath<'a> { #[inline] - fn from(path: &'static Path) -> Self { + fn from(path: &'a Path) -> Self { Self { source: AssetSourceId::Default, - path: CowArc::Static(path), + path: CowArc::Borrowed(path), label: None, } } diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index 47eb7c05429b5..635d31a583ef6 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -26,6 +26,18 @@ pub enum CowArc<'a, T: ?Sized + 'static> { Owned(Arc), } +impl CowArc<'static, T> { + /// Indicates this [`CowArc`] should have a static lifetime. + /// This ensures if this was created with a value `Borrowed(&'static T)`, it is replaced with `Static(&'static T)`. + #[inline] + pub fn as_static(self) -> Self { + match self { + Self::Borrowed(value) | Self::Static(value) => Self::Static(value), + Self::Owned(value) => Self::Owned(value), + } + } +} + impl<'a, T: ?Sized> Deref for CowArc<'a, T> { type Target = T;