From 849b975e5fce3d181aea692b412826f48c38e087 Mon Sep 17 00:00:00 2001 From: Jakob Buchgraber Date: Wed, 24 Apr 2024 08:20:01 -0700 Subject: [PATCH] Temporarily use AsRef<{ProtoStr, [u8]}> for string/bytes accessors This change removes the only remaining instance of SettableValue in a generated accessor. The principled fix is to implement IntoProxied for ProtoStr/[u8], but this will have to wait until we have agreed on & implemented the 1.0 string types. So we'll use AsRef in the meantime in order to not break any user code while allowing us to make progress on IntoProxied and Proxied changes. PiperOrigin-RevId: 627735404 --- rust/cpp.rs | 16 ++- rust/string.rs | 103 +++--------------- rust/upb.rs | 16 ++- .../rust/accessors/singular_string.cc | 20 +++- 4 files changed, 46 insertions(+), 109 deletions(-) diff --git a/rust/cpp.rs b/rust/cpp.rs index 225c97e519..9e78c09336 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -171,6 +171,13 @@ impl Deref for SerializedData { } } +// TODO: remove after IntoProxied has been implemented for bytes. +impl AsRef<[u8]> for SerializedData { + fn as_ref(&self) -> &[u8] { + self + } +} + impl Drop for SerializedData { fn drop(&mut self) { // SAFETY: `data` was allocated by the Rust global allocator with a @@ -185,15 +192,6 @@ impl fmt::Debug for SerializedData { } } -impl SettableValue<[u8]> for SerializedData { - fn set_on<'msg>(self, _private: Private, mut mutator: Mut<'msg, [u8]>) - where - [u8]: 'msg, - { - mutator.set(self.as_ref()) - } -} - /// A type to transfer an owned Rust string across the FFI boundary: /// * This struct is ABI-compatible with the equivalent C struct. /// * It owns its data but does not drop it. Immediately turn it into a diff --git a/rust/string.rs b/rust/string.rs index 9377a53514..e54ec44bc0 100644 --- a/rust/string.rs +++ b/rust/string.rs @@ -185,53 +185,6 @@ impl<'msg> MutProxy<'msg> for BytesMut<'msg> { } } -impl SettableValue<[u8]> for &'_ [u8] { - fn set_on<'msg>(self, _private: Private, mutator: Mut<'msg, [u8]>) - where - [u8]: 'msg, - { - // SAFETY: this is a `bytes` field with no restriction on UTF-8. - unsafe { mutator.inner.set(self) } - } - - fn set_on_absent( - self, - _private: Private, - absent_mutator: <[u8] as ProxiedWithPresence>::AbsentMutData<'_>, - ) -> <[u8] as ProxiedWithPresence>::PresentMutData<'_> { - // SAFETY: this is a `bytes` field with no restriction on UTF-8. - unsafe { absent_mutator.set(self) } - } - - fn set_on_present( - self, - _private: Private, - present_mutator: <[u8] as ProxiedWithPresence>::PresentMutData<'_>, - ) { - // SAFETY: this is a `bytes` field with no restriction on UTF-8. - unsafe { - present_mutator.set(self); - } - } -} - -impl SettableValue<[u8]> for &'_ [u8; N] { - // forward to `self[..]` - impl_forwarding_settable_value!([u8], self => &self[..]); -} - -impl SettableValue<[u8]> for Vec { - // TODO: Investigate taking ownership of this when allowed by the - // runtime. - impl_forwarding_settable_value!([u8], self => &self[..]); -} - -impl SettableValue<[u8]> for Cow<'_, [u8]> { - // TODO: Investigate taking ownership of this when allowed by the - // runtime. - impl_forwarding_settable_value!([u8], self => &self[..]); -} - impl Hash for BytesMut<'_> { fn hash(&self, state: &mut H) { self.deref().hash(state) @@ -701,50 +654,28 @@ impl<'msg> MutProxy<'msg> for ProtoStrMut<'msg> { } } -impl SettableValue for &'_ ProtoStr { - fn set_on<'b>(self, _private: Private, mutator: Mut<'b, ProtoStr>) - where - ProtoStr: 'b, - { - // SAFETY: A `ProtoStr` has the same UTF-8 validity requirement as the runtime. - unsafe { mutator.bytes.inner.set(self.as_bytes()) } - } - - fn set_on_absent( - self, - _private: Private, - absent_mutator: ::AbsentMutData<'_>, - ) -> ::PresentMutData<'_> { - // SAFETY: A `ProtoStr` has the same UTF-8 validity requirement as the runtime. - StrPresentMutData(unsafe { absent_mutator.0.set(self.as_bytes()) }) - } - - fn set_on_present( - self, - _private: Private, - present_mutator: ::PresentMutData<'_>, - ) { - // SAFETY: A `ProtoStr` has the same UTF-8 validity requirement as the runtime. - unsafe { - present_mutator.0.set(self.as_bytes()); - } +// TODO: remove after IntoProxied has been implemented for +// ProtoStr. +impl AsRef for String { + fn as_ref(&self) -> &ProtoStr { + ProtoStr::from_str(self.as_str()) } } -impl SettableValue for &'_ str { - impl_forwarding_settable_value!(ProtoStr, self => ProtoStr::from_str(self)); -} - -impl SettableValue for String { - // TODO: Investigate taking ownership of this when allowed by the - // runtime. - impl_forwarding_settable_value!(ProtoStr, self => ProtoStr::from_str(&self)); +// TODO: remove after IntoProxied has been implemented for +// ProtoStr. +impl AsRef for &str { + fn as_ref(&self) -> &ProtoStr { + ProtoStr::from_str(self) + } } -impl SettableValue for Cow<'_, str> { - // TODO: Investigate taking ownership of this when allowed by the - // runtime. - impl_forwarding_settable_value!(ProtoStr, self => ProtoStr::from_str(&self)); +// TODO: remove after IntoProxied has been implemented for +// ProtoStr. +impl AsRef for &ProtoStr { + fn as_ref(&self) -> &ProtoStr { + self + } } impl Hash for ProtoStrMut<'_> { diff --git a/rust/upb.rs b/rust/upb.rs index 5a0c07e471..3b9a69ba44 100644 --- a/rust/upb.rs +++ b/rust/upb.rs @@ -97,18 +97,16 @@ impl Deref for SerializedData { } } -impl fmt::Debug for SerializedData { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Debug::fmt(self.deref(), f) +// TODO: remove after IntoProxied has been implemented for bytes. +impl AsRef<[u8]> for SerializedData { + fn as_ref(&self) -> &[u8] { + self } } -impl SettableValue<[u8]> for SerializedData { - fn set_on<'msg>(self, _private: Private, mut mutator: Mut<'msg, [u8]>) - where - [u8]: 'msg, - { - mutator.set(self.as_ref()) +impl fmt::Debug for SerializedData { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Debug::fmt(self.deref(), f) } } diff --git a/src/google/protobuf/compiler/rust/accessors/singular_string.cc b/src/google/protobuf/compiler/rust/accessors/singular_string.cc index afb3592e75..a9a8b27940 100644 --- a/src/google/protobuf/compiler/rust/accessors/singular_string.cc +++ b/src/google/protobuf/compiler/rust/accessors/singular_string.cc @@ -80,12 +80,22 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field, [&] { if (accessor_case == AccessorCase::VIEW) return; ctx.Emit(R"rs( - pub fn set_$raw_field_name$(&mut self, val: impl $pb$::SettableValue<$proxied_type$>) { - //~ TODO: Optimize this to not go through the - //~ FieldEntry. - self.$raw_field_name$_mut().set(val); + // TODO: Use IntoProxied once string/bytes types support it. + pub fn set_$raw_field_name$(&mut self, val: impl std::convert::AsRef<$proxied_type$>) { + let string_view: $pbr$::PtrAndLen = + $pbr$::copy_bytes_in_arena_if_needed_by_runtime( + self.as_mutator_message_ref(), + val.as_ref().into() + ).into(); + + unsafe { + $setter_thunk$( + self.as_mutator_message_ref().msg(), + string_view + ); } - )rs"); + } + )rs"); }}, {"hazzer", [&] {