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
pull/17322/head
Jakob Buchgraber 8 months ago committed by Copybara-Service
parent 0d6e9794d1
commit 08da921314
  1. 31
      rust/cpp.rs
  2. 16
      rust/string.rs
  3. 18
      rust/upb.rs
  4. 67
      src/google/protobuf/compiler/rust/accessors/singular_string.cc

@ -71,7 +71,7 @@ mod _opaque_pointees {
/// 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 RawRepeatedField = NonNull<_opaque_pointees::RawRepeatedFieldData>;
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 @@ impl InnerProtoString {
// 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 @@ impl SerializedData {
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 @@ impl<'msg> MutatorMessageRef<'msg> {
}
}
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 @@ impl CppTypeConversions for ProtoString {
}
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 @@ impl CppTypeConversions for ProtoBytes {
}
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 ptrlen_to_str<'msg>(val: PtrAndLen) -> &'msg ProtoStr {
}
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

@ -29,6 +29,15 @@ pub struct ProtoBytes {
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 @@ impl ProtoString {
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 {

@ -128,13 +128,6 @@ impl<'msg> MutatorMessageRef<'msg> {
}
}
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 @@ impl InnerProtoString {
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 @@ impl UpbTypeConversions for ProtoBytes {
) -> 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> {

@ -66,28 +66,46 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field,
}
)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 @@ void SingularString::InExternC(Context& ctx,
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 @@ void SingularString::InExternC(Context& ctx,
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 @@ void SingularString::InThunkCc(Context& ctx,
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");
}

Loading…
Cancel
Save