Skip to content

Commit

Permalink
Temporarily use AsRef<{ProtoStr, [u8]}> for string/bytes accessors
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@buchgr @copybara-github
buchgr authored and copybara-github committed Apr 24, 2024
1 parent 1a7ce61 commit 849b975
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 109 deletions.
16 changes: 7 additions & 9 deletions rust/cpp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
103 changes: 17 additions & 86 deletions rust/string.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<const N: usize> SettableValue<[u8]> for &'_ [u8; N] {
// forward to `self[..]`
impl_forwarding_settable_value!([u8], self => &self[..]);
}

impl SettableValue<[u8]> for Vec<u8> {
// 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<H: Hasher>(&self, state: &mut H) {
self.deref().hash(state)
Expand Down Expand Up @@ -701,50 +654,28 @@ impl<'msg> MutProxy<'msg> for ProtoStrMut<'msg> {
}
}

impl SettableValue<ProtoStr> 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: <ProtoStr as ProxiedWithPresence>::AbsentMutData<'_>,
) -> <ProtoStr as ProxiedWithPresence>::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: <ProtoStr as ProxiedWithPresence>::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<ProtoStr> for String {
fn as_ref(&self) -> &ProtoStr {
ProtoStr::from_str(self.as_str())
}
}

impl SettableValue<ProtoStr> for &'_ str {
impl_forwarding_settable_value!(ProtoStr, self => ProtoStr::from_str(self));
}

impl SettableValue<ProtoStr> 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<ProtoStr> for &str {
fn as_ref(&self) -> &ProtoStr {
ProtoStr::from_str(self)
}
}

impl SettableValue<ProtoStr> 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<ProtoStr> for &ProtoStr {
fn as_ref(&self) -> &ProtoStr {
self
}
}

impl Hash for ProtoStrMut<'_> {
Expand Down
16 changes: 7 additions & 9 deletions rust/upb.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
20 changes: 15 additions & 5 deletions src/google/protobuf/compiler/rust/accessors/singular_string.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
[&] {
Expand Down

0 comments on commit 849b975

Please sign in to comment.