Remove the defensive copy on singular string setter
Calling into_proxied() already does a copy and before this change we were doing a second one.
I am not using set_allocated_<field(std::string* s) because the method is not generated when [features.(pb.cpp).string_type = VIEW] is specified.
PiperOrigin-RevId: 650612909
diff --git a/rust/cpp.rs b/rust/cpp.rs
index c33e71d..c5257ba 100644
--- a/rust/cpp.rs
+++ b/rust/cpp.rs
@@ -71,7 +71,7 @@
/// It is only meant to provide type safety for raw pointers
/// which are manipulated behind FFI.
#[repr(C)]
- pub(super) struct CppStdStringData {
+ pub struct CppStdStringData {
_data: [u8; 0],
_marker: std::marker::PhantomData<(*mut u8, ::std::marker::PhantomPinned)>,
}
@@ -87,7 +87,7 @@
pub type RawMap = NonNull<_opaque_pointees::RawMapData>;
/// A raw pointer to a std::string.
-type CppStdString = NonNull<_opaque_pointees::CppStdStringData>;
+pub type CppStdString = NonNull<_opaque_pointees::CppStdStringData>;
/// Kernel-specific owned `string` and `bytes` field type.
#[derive(Debug)]
@@ -109,6 +109,11 @@
// SAFETY: `self.owned_ptr` points to a valid std::string object.
unsafe { proto2_rust_cpp_string_to_view(self.owned_ptr).as_ref() }
}
+
+ pub fn into_raw(self, _private: Private) -> CppStdString {
+ let s = ManuallyDrop::new(self);
+ s.owned_ptr
+ }
}
impl From<&[u8]> for InnerProtoString {
@@ -219,7 +224,7 @@
pub fn into_vec(self) -> Vec<u8> {
// We need to prevent self from being dropped, because we are going to transfer
// ownership of self.data to the Vec<u8>.
- let s = std::mem::ManuallyDrop::new(self);
+ let s = ManuallyDrop::new(self);
unsafe {
// SAFETY:
@@ -352,14 +357,6 @@
}
}
-pub fn copy_bytes_in_arena_if_needed_by_runtime<'msg>(
- _msg_ref: MutatorMessageRef<'msg>,
- val: &'msg [u8],
-) -> &'msg [u8] {
- // Nothing to do, the message manages its own string memory for C++.
- val
-}
-
/// The raw type-erased version of an owned `Repeated`.
#[derive(Debug)]
pub struct InnerRepeated {
@@ -430,8 +427,7 @@
}
fn into_insertelem(v: Self) -> CppStdString {
- let v = ManuallyDrop::new(v);
- v.inner.owned_ptr
+ v.into_inner(Private).into_raw(Private)
}
}
@@ -444,8 +440,7 @@
}
fn into_insertelem(v: Self) -> CppStdString {
- let v = ManuallyDrop::new(v);
- v.inner.owned_ptr
+ v.into_inner(Private).into_raw(Private)
}
}
@@ -817,13 +812,11 @@
}
fn protostr_into_cppstdstring(val: ProtoString) -> CppStdString {
- let m = ManuallyDrop::new(val);
- m.inner.owned_ptr
+ val.into_inner(Private).into_raw(Private)
}
fn protobytes_into_cppstdstring(val: ProtoBytes) -> CppStdString {
- let m = ManuallyDrop::new(val);
- m.inner.owned_ptr
+ val.into_inner(Private).into_raw(Private)
}
// Warning: this function is unsound on its own! `val.as_ref()` must be safe to
diff --git a/rust/string.rs b/rust/string.rs
index 3f82e14..a94f4ef 100644
--- a/rust/string.rs
+++ b/rust/string.rs
@@ -29,6 +29,15 @@
pub(crate) inner: InnerProtoString,
}
+impl ProtoBytes {
+ // Returns the kernel-specific container. This method is private in spirit and
+ // must not be called by a user.
+ #[doc(hidden)]
+ pub fn into_inner(self, _private: Private) -> InnerProtoString {
+ self.inner
+ }
+}
+
impl AsRef<[u8]> for ProtoBytes {
fn as_ref(&self) -> &[u8] {
self.inner.as_bytes()
@@ -162,6 +171,13 @@
pub fn as_bytes(&self) -> &[u8] {
self.inner.as_bytes()
}
+
+ // Returns the kernel-specific container. This method is private in spirit and
+ // must not be called by a user.
+ #[doc(hidden)]
+ pub fn into_inner(self, _private: Private) -> InnerProtoString {
+ self.inner
+ }
}
impl From<ProtoString> for ProtoBytes {
diff --git a/rust/upb.rs b/rust/upb.rs
index e53e0aa..5a8cdfa 100644
--- a/rust/upb.rs
+++ b/rust/upb.rs
@@ -128,13 +128,6 @@
}
}
-pub fn copy_bytes_in_arena_if_needed_by_runtime<'msg>(
- msg_ref: MutatorMessageRef<'msg>,
- val: &'msg [u8],
-) -> &'msg [u8] {
- copy_bytes_in_arena(msg_ref.arena, val)
-}
-
fn copy_bytes_in_arena<'msg>(arena: &'msg Arena, val: &'msg [u8]) -> &'msg [u8] {
// SAFETY: the alignment of `[u8]` is less than `UPB_MALLOC_ALIGN`.
let new_alloc = unsafe { arena.alloc(Layout::for_value(val)) };
@@ -157,6 +150,12 @@
pub(crate) fn as_bytes(&self) -> &[u8] {
&self.0
}
+
+ #[doc(hidden)]
+ pub fn into_raw_parts(self, _private: Private) -> (PtrAndLen, Arena) {
+ let (data_ptr, arena) = self.0.into_parts();
+ (unsafe { data_ptr.as_ref().into() }, arena)
+ }
}
impl From<&[u8]> for InnerProtoString {
@@ -617,12 +616,11 @@
) -> upb_MessageValue {
// SAFETY: The arena memory is not freed due to `ManuallyDrop`.
let parent_arena = ManuallyDrop::new(unsafe { Arena::from_raw(raw_parent_arena) });
- let (data, arena) = val.inner.0.into_parts();
+ let (view, arena) = val.inner.into_raw_parts(Private);
parent_arena.fuse(&arena);
- let msg_val = Self::to_message_value(unsafe { data.as_ref() });
- msg_val
+ upb_MessageValue { str_val: view }
}
unsafe fn from_message_value<'msg>(msg: upb_MessageValue) -> View<'msg, ProtoBytes> {
diff --git a/src/google/protobuf/compiler/rust/accessors/singular_string.cc b/src/google/protobuf/compiler/rust/accessors/singular_string.cc
index d37bb75..99f8b9f 100644
--- a/src/google/protobuf/compiler/rust/accessors/singular_string.cc
+++ b/src/google/protobuf/compiler/rust/accessors/singular_string.cc
@@ -66,28 +66,46 @@
}
)rs");
}},
- {"setter",
+ {"setter_impl",
[&] {
- if (accessor_case == AccessorCase::VIEW) return;
- ctx.Emit({{"as_ref_method",
- (field.type() == FieldDescriptor::TYPE_STRING
- ? "as_bytes()"
- : "as_ref()")}},
- R"rs(
- pub fn set_$raw_field_name$(&mut self, val: impl $pb$::IntoProxied<$proxied_type$>) {
- let into_proxied = val.into_proxied($pbi$::Private);
- let string_view: $pbr$::PtrAndLen =
- $pbr$::copy_bytes_in_arena_if_needed_by_runtime(
- self.as_mutator_message_ref($pbi$::Private),
- into_proxied.$as_ref_method$,
- ).into();
+ if (ctx.is_cpp()) {
+ ctx.Emit(R"rs(
+ let s = val.into_proxied($pbi$::Private);
+ unsafe {
+ $setter_thunk$(
+ self.as_mutator_message_ref($pbi$::Private).msg(),
+ s.into_inner($pbi$::Private).into_raw($pbi$::Private)
+ );
+ }
+ )rs");
+ } else {
+ ctx.Emit(R"rs(
+ let s = val.into_proxied($pbi$::Private);
+ let (view, arena) =
+ s.into_inner($pbi$::Private).into_raw_parts($pbi$::Private);
+
+ let mm_ref =
+ self.as_mutator_message_ref($pbi$::Private);
+ let parent_arena = mm_ref.arena($pbi$::Private);
+
+ parent_arena.fuse(&arena);
unsafe {
$setter_thunk$(
self.as_mutator_message_ref($pbi$::Private).msg(),
- string_view
+ view
);
}
+ )rs");
+ }
+ }},
+ {"setter",
+ [&] {
+ if (accessor_case == AccessorCase::VIEW) return;
+ ctx.Emit({},
+ R"rs(
+ pub fn set_$raw_field_name$(&mut self, val: impl $pb$::IntoProxied<$proxied_type$>) {
+ $setter_impl$
}
)rs");
}},
@@ -123,6 +141,18 @@
ctx.Emit({{"hazzer_thunk", ThunkName(ctx, field, "has")},
{"getter_thunk", ThunkName(ctx, field, "get")},
{"setter_thunk", ThunkName(ctx, field, "set")},
+ {"setter",
+ [&] {
+ if (ctx.is_cpp()) {
+ ctx.Emit(R"rs(
+ fn $setter_thunk$(raw_msg: $pbr$::RawMessage, val: $pbr$::CppStdString);
+ )rs");
+ } else {
+ ctx.Emit(R"rs(
+ fn $setter_thunk$(raw_msg: $pbr$::RawMessage, val: $pbr$::PtrAndLen);
+ )rs");
+ }
+ }},
{"clearer_thunk", ThunkName(ctx, field, "clear")},
{"with_presence_fields_thunks",
[&] {
@@ -136,7 +166,7 @@
R"rs(
$with_presence_fields_thunks$
fn $getter_thunk$(raw_msg: $pbr$::RawMessage) -> $pbr$::PtrAndLen;
- fn $setter_thunk$(raw_msg: $pbr$::RawMessage, val: $pbr$::PtrAndLen);
+ $setter$
)rs");
}
@@ -165,8 +195,9 @@
absl::string_view val = msg->$field$();
return ::google::protobuf::rust::PtrAndLen(val.data(), val.size());
}
- void $setter_thunk$($QualifiedMsg$* msg, ::google::protobuf::rust::PtrAndLen s) {
- msg->set_$field$(absl::string_view(s.ptr, s.len));
+ void $setter_thunk$($QualifiedMsg$* msg, std::string* s) {
+ msg->set_$field$(std::move(*s));
+ delete s;
}
)cc");
}
