From 0d6e9794d1091e622ae96c586e1b33632feb5841 Mon Sep 17 00:00:00 2001 From: Jakob Buchgraber Date: Tue, 9 Jul 2024 04:44:21 -0700 Subject: [PATCH] Migrate Repeated::{push, set} and Map::insert to use the IntoProxied trait. * The public Repeated::{push, set} and Map::insert methods now accept any value that implements IntoProxied, allowing us to move owned values instead of copying them. * This change also updates the FFI layer for strings/bytes in the repeated and maps thunks to accept a std::string* that can be moved rather than a PtrAndLen type that needs to be copied. * Tests are updated to no longer .as_view() when setting a message / string on a repeated / map field. The IntoProxied trait makes calling .as_view() obsolete. PiperOrigin-RevId: 650580788 --- rust/cpp.rs | 77 ++++++--- rust/cpp_kernel/map.cc | 31 ++-- rust/cpp_kernel/map.h | 163 ++++++++++--------- rust/cpp_kernel/repeated.cc | 79 ++++----- rust/map.rs | 24 ++- rust/primitive.rs | 9 +- rust/repeated.rs | 97 ++++++----- rust/string.rs | 6 + rust/test/shared/accessors_map_test.rs | 8 +- rust/test/shared/accessors_repeated_test.rs | 19 +-- rust/test/shared/edition2023_test.rs | 2 +- rust/test/upb/string_ctypes_test.rs | 4 +- rust/upb.rs | 54 +++--- src/google/protobuf/compiler/rust/enum.cc | 30 +++- src/google/protobuf/compiler/rust/message.cc | 79 ++++----- 15 files changed, 360 insertions(+), 322 deletions(-) diff --git a/rust/cpp.rs b/rust/cpp.rs index c2a362aedd..c33e71d528 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -9,7 +9,7 @@ use crate::__internal::{Enum, Private}; use crate::{ - Map, MapIter, Mut, ProtoBytes, ProtoStr, ProtoString, Proxied, ProxiedInMapValue, + IntoProxied, Map, MapIter, Mut, ProtoBytes, ProtoStr, ProtoString, Proxied, ProxiedInMapValue, ProxiedInRepeated, Repeated, RepeatedMut, RepeatedView, View, }; use core::fmt::Debug; @@ -18,7 +18,7 @@ use std::convert::identity; use std::ffi::{c_int, c_void}; use std::fmt; use std::marker::PhantomData; -use std::mem::MaybeUninit; +use std::mem::{ManuallyDrop, MaybeUninit}; use std::ops::Deref; use std::ptr::{self, NonNull}; use std::slice; @@ -393,20 +393,27 @@ impl<'msg> InnerRepeatedMut<'msg> { } trait CppTypeConversions: Proxied { + type InsertElemType; type ElemType; fn elem_to_view<'msg>(v: Self::ElemType) -> View<'msg, Self>; + fn into_insertelem(v: Self) -> Self::InsertElemType; } macro_rules! impl_cpp_type_conversions_for_scalars { ($($t:ty),* $(,)?) => { $( impl CppTypeConversions for $t { + type InsertElemType = Self; type ElemType = Self; fn elem_to_view<'msg>(v: Self) -> View<'msg, Self> { v } + + fn into_insertelem(v: Self) -> Self { + v + } } )* } @@ -415,19 +422,31 @@ macro_rules! impl_cpp_type_conversions_for_scalars { impl_cpp_type_conversions_for_scalars!(i32, u32, i64, u64, f32, f64, bool); impl CppTypeConversions for ProtoString { + type InsertElemType = CppStdString; type ElemType = PtrAndLen; fn elem_to_view<'msg>(v: PtrAndLen) -> View<'msg, ProtoString> { ptrlen_to_str(v) } + + fn into_insertelem(v: Self) -> CppStdString { + let v = ManuallyDrop::new(v); + v.inner.owned_ptr + } } impl CppTypeConversions for ProtoBytes { + type InsertElemType = CppStdString; type ElemType = PtrAndLen; fn elem_to_view<'msg>(v: Self::ElemType) -> View<'msg, Self> { ptrlen_to_bytes(v) } + + fn into_insertelem(v: Self) -> CppStdString { + let v = ManuallyDrop::new(v); + v.inner.owned_ptr + } } macro_rules! impl_repeated_primitives { @@ -446,7 +465,7 @@ macro_rules! impl_repeated_primitives { extern "C" { fn $new_thunk() -> RawRepeatedField; fn $free_thunk(f: RawRepeatedField); - fn $add_thunk(f: RawRepeatedField, v: <$t as CppTypeConversions>::ElemType); + fn $add_thunk(f: RawRepeatedField, v: <$t as CppTypeConversions>::InsertElemType); fn $size_thunk(f: RawRepeatedField) -> usize; fn $get_thunk( f: RawRepeatedField, @@ -454,7 +473,7 @@ macro_rules! impl_repeated_primitives { fn $set_thunk( f: RawRepeatedField, i: usize, - v: <$t as CppTypeConversions>::ElemType); + v: <$t as CppTypeConversions>::InsertElemType); fn $clear_thunk(f: RawRepeatedField); fn $copy_from_thunk(src: RawRepeatedField, dst: RawRepeatedField); fn $reserve_thunk( @@ -480,8 +499,8 @@ macro_rules! impl_repeated_primitives { unsafe { $size_thunk(f.as_raw(Private)) } } #[inline] - fn repeated_push(mut f: Mut>, v: View<$t>) { - unsafe { $add_thunk(f.as_raw(Private), v.into()) } + fn repeated_push(mut f: Mut>, v: impl IntoProxied<$t>) { + unsafe { $add_thunk(f.as_raw(Private), <$t as CppTypeConversions>::into_insertelem(v.into_proxied(Private))) } } #[inline] fn repeated_clear(mut f: Mut>) { @@ -493,8 +512,8 @@ macro_rules! impl_repeated_primitives { unsafe { $get_thunk(f.as_raw(Private), i) }) } #[inline] - unsafe fn repeated_set_unchecked(mut f: Mut>, i: usize, v: View<$t>) { - unsafe { $set_thunk(f.as_raw(Private), i, v.into()) } + unsafe fn repeated_set_unchecked(mut f: Mut>, i: usize, v: impl IntoProxied<$t>) { + unsafe { $set_thunk(f.as_raw(Private), i, <$t as CppTypeConversions>::into_insertelem(v.into_proxied(Private))) } } #[inline] fn repeated_copy_from(src: View>, mut dest: Mut>) { @@ -686,18 +705,18 @@ extern "C" { } macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types { - ($key_t:ty, $ffi_key_t:ty, $to_ffi_key:expr, $from_ffi_key:expr, for $($t:ty, $ffi_t:ty, $to_ffi_value:expr, $from_ffi_value:expr;)*) => { + ($key_t:ty, $ffi_key_t:ty, $to_ffi_key:expr, $from_ffi_key:expr, for $($t:ty, $ffi_view_t:ty, $ffi_value_t:ty, $to_ffi_value:expr, $from_ffi_value:expr;)*) => { paste! { $( extern "C" { fn [< proto2_rust_thunk_Map_ $key_t _ $t _new >]() -> RawMap; fn [< proto2_rust_thunk_Map_ $key_t _ $t _free >](m: RawMap); fn [< proto2_rust_thunk_Map_ $key_t _ $t _clear >](m: RawMap); fn [< proto2_rust_thunk_Map_ $key_t _ $t _size >](m: RawMap) -> usize; - fn [< proto2_rust_thunk_Map_ $key_t _ $t _insert >](m: RawMap, key: $ffi_key_t, value: $ffi_t) -> bool; - fn [< proto2_rust_thunk_Map_ $key_t _ $t _get >](m: RawMap, key: $ffi_key_t, value: *mut $ffi_t) -> bool; + fn [< proto2_rust_thunk_Map_ $key_t _ $t _insert >](m: RawMap, key: $ffi_key_t, value: $ffi_value_t) -> bool; + fn [< proto2_rust_thunk_Map_ $key_t _ $t _get >](m: RawMap, key: $ffi_key_t, value: *mut $ffi_view_t) -> bool; fn [< proto2_rust_thunk_Map_ $key_t _ $t _iter >](m: RawMap) -> UntypedMapIterator; - fn [< proto2_rust_thunk_Map_ $key_t _ $t _iter_get >](iter: &mut UntypedMapIterator, key: *mut $ffi_key_t, value: *mut $ffi_t); - fn [< proto2_rust_thunk_Map_ $key_t _ $t _remove >](m: RawMap, key: $ffi_key_t, value: *mut $ffi_t) -> bool; + fn [< proto2_rust_thunk_Map_ $key_t _ $t _iter_get >](iter: &mut UntypedMapIterator, key: *mut $ffi_key_t, value: *mut $ffi_view_t); + fn [< proto2_rust_thunk_Map_ $key_t _ $t _remove >](m: RawMap, key: $ffi_key_t, value: *mut $ffi_view_t) -> bool; } impl ProxiedInMapValue<$key_t> for $t { @@ -728,9 +747,9 @@ macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types { unsafe { [< proto2_rust_thunk_Map_ $key_t _ $t _size >](map.as_raw(Private)) } } - fn map_insert(mut map: Mut<'_, Map<$key_t, Self>>, key: View<'_, $key_t>, value: View<'_, Self>) -> bool { + fn map_insert(mut map: Mut<'_, Map<$key_t, Self>>, key: View<'_, $key_t>, value: impl IntoProxied) -> bool { let ffi_key = $to_ffi_key(key); - let ffi_value = $to_ffi_value(value); + let ffi_value = $to_ffi_value(value.into_proxied(Private)); unsafe { [< proto2_rust_thunk_Map_ $key_t _ $t _insert >](map.as_raw(Private), ffi_key, ffi_value) } } @@ -797,8 +816,14 @@ fn ptrlen_to_str<'msg>(val: PtrAndLen) -> &'msg ProtoStr { unsafe { ProtoStr::from_utf8_unchecked(val.as_ref()) } } -fn bytes_to_ptrlen(val: &[u8]) -> PtrAndLen { - val.into() +fn protostr_into_cppstdstring(val: ProtoString) -> CppStdString { + let m = ManuallyDrop::new(val); + m.inner.owned_ptr +} + +fn protobytes_into_cppstdstring(val: ProtoBytes) -> CppStdString { + let m = ManuallyDrop::new(val); + m.inner.owned_ptr } // Warning: this function is unsound on its own! `val.as_ref()` must be safe to @@ -813,15 +838,15 @@ macro_rules! impl_ProxiedInMapValue_for_key_types { $( impl_ProxiedInMapValue_for_non_generated_value_types!( $t, $ffi_t, $to_ffi_key, $from_ffi_key, for - f32, f32, identity, identity; - f64, f64, identity, identity; - i32, i32, identity, identity; - u32, u32, identity, identity; - i64, i64, identity, identity; - u64, u64, identity, identity; - bool, bool, identity, identity; - ProtoString, PtrAndLen, str_to_ptrlen, ptrlen_to_str; - ProtoBytes, PtrAndLen, bytes_to_ptrlen, ptrlen_to_bytes; + f32, f32, f32, identity, identity; + f64, f64, f64, identity, identity; + i32, i32, i32, identity, identity; + u32, u32, u32, identity, identity; + i64, i64, i64, identity, identity; + u64, u64, u64, identity, identity; + bool, bool, bool, identity, identity; + ProtoString, PtrAndLen, CppStdString, protostr_into_cppstdstring, ptrlen_to_str; + ProtoBytes, PtrAndLen, CppStdString, protobytes_into_cppstdstring, ptrlen_to_bytes; ); )* } diff --git a/rust/cpp_kernel/map.cc b/rust/cpp_kernel/map.cc index 08b8078926..2418c7274c 100644 --- a/rust/cpp_kernel/map.cc +++ b/rust/cpp_kernel/map.cc @@ -2,6 +2,7 @@ #include #include +#include #include "google/protobuf/map.h" #include "rust/cpp_kernel/strings.h" @@ -13,27 +14,27 @@ void proto2_rust_thunk_UntypedMapIterator_increment( iter->PlusPlus(); } -__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(int32_t, i32, int32_t, value, - cpp_value); +__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(int32_t, i32, int32_t, + int32_t, value, cpp_value); __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(uint32_t, u32, uint32_t, + uint32_t, value, cpp_value); +__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(float, f32, float, float, value, cpp_value); -__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(float, f32, float, value, - cpp_value); -__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(double, f64, double, value, - cpp_value); -__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(bool, bool, bool, value, - cpp_value); -__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(uint64_t, u64, uint64_t, +__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(double, f64, double, double, + value, cpp_value); +__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(bool, bool, bool, bool, value, cpp_value); -__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(int64_t, i64, int64_t, value, - cpp_value); +__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(uint64_t, u64, uint64_t, + uint64_t, value, cpp_value); +__PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(int64_t, i64, int64_t, + int64_t, value, cpp_value); __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE( - std::string, ProtoBytes, google::protobuf::rust::PtrAndLen, - std::string(value.ptr, value.len), + std::string, ProtoBytes, google::protobuf::rust::PtrAndLen, std::string*, + std::move(*value), google::protobuf::rust::PtrAndLen(cpp_value.data(), cpp_value.size())); __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE( - std::string, ProtoString, google::protobuf::rust::PtrAndLen, - std::string(value.ptr, value.len), + std::string, ProtoString, google::protobuf::rust::PtrAndLen, std::string*, + std::move(*value), google::protobuf::rust::PtrAndLen(cpp_value.data(), cpp_value.size())); } // extern "C" diff --git a/rust/cpp_kernel/map.h b/rust/cpp_kernel/map.h index 4d90ab322a..2f0a8a82ab 100644 --- a/rust/cpp_kernel/map.h +++ b/rust/cpp_kernel/map.h @@ -2,89 +2,90 @@ #define GOOGLE_PROTOBUF_RUST_CPP_KERNEL_MAP_H__ // Defines concrete thunks to access typed map methods from Rust. -#define __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ - key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, to_ffi_key, value_ty, \ - rust_value_ty, ffi_value_ty, to_cpp_value, to_ffi_value) \ - google::protobuf::Map* \ - proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_new() { \ - return new google::protobuf::Map(); \ - } \ - void proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_free( \ - google::protobuf::Map* m) { \ - delete m; \ - } \ - void proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_clear( \ - google::protobuf::Map* m) { \ - m->clear(); \ - } \ - size_t proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_size( \ - const google::protobuf::Map* m) { \ - return m->size(); \ - } \ - bool proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_insert( \ - google::protobuf::Map* m, ffi_key_ty key, ffi_value_ty value) { \ - auto iter_and_inserted = m->try_emplace(to_cpp_key, to_cpp_value); \ - if (!iter_and_inserted.second) { \ - iter_and_inserted.first->second = to_cpp_value; \ - } \ - return iter_and_inserted.second; \ - } \ - bool proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_get( \ - const google::protobuf::Map* m, ffi_key_ty key, \ - ffi_value_ty* value) { \ - auto cpp_key = to_cpp_key; \ - auto it = m->find(cpp_key); \ - if (it == m->end()) { \ - return false; \ - } \ - auto& cpp_value = it->second; \ - *value = to_ffi_value; \ - return true; \ - } \ - google::protobuf::internal::UntypedMapIterator \ - proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_iter( \ - const google::protobuf::Map* m) { \ - return google::protobuf::internal::UntypedMapIterator::FromTyped(m->cbegin()); \ - } \ - void proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_iter_get( \ - const google::protobuf::internal::UntypedMapIterator* iter, ffi_key_ty* key, \ - ffi_value_ty* value) { \ - auto typed_iter = \ - iter->ToTyped::const_iterator>(); \ - const auto& cpp_key = typed_iter->first; \ - const auto& cpp_value = typed_iter->second; \ - *key = to_ffi_key; \ - *value = to_ffi_value; \ - } \ - bool proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_remove( \ - google::protobuf::Map* m, ffi_key_ty key, ffi_value_ty* value) { \ - auto cpp_key = to_cpp_key; \ - auto num_removed = m->erase(cpp_key); \ - return num_removed > 0; \ +#define __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ + key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, to_ffi_key, value_ty, \ + rust_value_ty, ffi_view_ty, ffi_value_ty, to_cpp_value, to_ffi_value) \ + google::protobuf::Map* \ + proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_new() { \ + return new google::protobuf::Map(); \ + } \ + void proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_free( \ + google::protobuf::Map* m) { \ + delete m; \ + } \ + void proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_clear( \ + google::protobuf::Map* m) { \ + m->clear(); \ + } \ + size_t proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_size( \ + const google::protobuf::Map* m) { \ + return m->size(); \ + } \ + bool proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_insert( \ + google::protobuf::Map* m, ffi_key_ty key, ffi_value_ty value) { \ + auto iter_and_inserted = m->try_emplace(to_cpp_key, to_cpp_value); \ + if (!iter_and_inserted.second) { \ + iter_and_inserted.first->second = to_cpp_value; \ + } \ + return iter_and_inserted.second; \ + } \ + bool proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_get( \ + const google::protobuf::Map* m, ffi_key_ty key, \ + ffi_view_ty* value) { \ + auto cpp_key = to_cpp_key; \ + auto it = m->find(cpp_key); \ + if (it == m->end()) { \ + return false; \ + } \ + auto& cpp_value = it->second; \ + *value = to_ffi_value; \ + return true; \ + } \ + google::protobuf::internal::UntypedMapIterator \ + proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_iter( \ + const google::protobuf::Map* m) { \ + return google::protobuf::internal::UntypedMapIterator::FromTyped(m->cbegin()); \ + } \ + void proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_iter_get( \ + const google::protobuf::internal::UntypedMapIterator* iter, ffi_key_ty* key, \ + ffi_view_ty* value) { \ + auto typed_iter = \ + iter->ToTyped::const_iterator>(); \ + const auto& cpp_key = typed_iter->first; \ + const auto& cpp_value = typed_iter->second; \ + *key = to_ffi_key; \ + *value = to_ffi_value; \ + } \ + bool proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_remove( \ + google::protobuf::Map* m, ffi_key_ty key, ffi_view_ty* value) { \ + auto cpp_key = to_cpp_key; \ + auto num_removed = m->erase(cpp_key); \ + return num_removed > 0; \ } // Defines the map thunks for all supported key types for a given value type. -#define __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE( \ - value_ty, rust_value_ty, ffi_value_ty, to_cpp_value, to_ffi_value) \ - __PB_RUST_EXPOSE_SCALAR_MAP_METHODS(int32_t, i32, int32_t, key, cpp_key, \ - value_ty, rust_value_ty, ffi_value_ty, \ - to_cpp_value, to_ffi_value); \ - __PB_RUST_EXPOSE_SCALAR_MAP_METHODS(uint32_t, u32, uint32_t, key, cpp_key, \ - value_ty, rust_value_ty, ffi_value_ty, \ - to_cpp_value, to_ffi_value); \ - __PB_RUST_EXPOSE_SCALAR_MAP_METHODS(bool, bool, bool, key, cpp_key, \ - value_ty, rust_value_ty, ffi_value_ty, \ - to_cpp_value, to_ffi_value); \ - __PB_RUST_EXPOSE_SCALAR_MAP_METHODS(uint64_t, u64, uint64_t, key, cpp_key, \ - value_ty, rust_value_ty, ffi_value_ty, \ - to_cpp_value, to_ffi_value); \ - __PB_RUST_EXPOSE_SCALAR_MAP_METHODS(int64_t, i64, int64_t, key, cpp_key, \ - value_ty, rust_value_ty, ffi_value_ty, \ - to_cpp_value, to_ffi_value); \ - __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ - std::string, ProtoString, google::protobuf::rust::PtrAndLen, \ - std::string(key.ptr, key.len), \ - google::protobuf::rust::PtrAndLen(cpp_key.data(), cpp_key.size()), value_ty, \ - rust_value_ty, ffi_value_ty, to_cpp_value, to_ffi_value); +#define __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE( \ + value_ty, rust_value_ty, ffi_view_ty, ffi_value_ty, to_cpp_value, \ + to_ffi_value) \ + __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ + int32_t, i32, int32_t, key, cpp_key, value_ty, rust_value_ty, \ + ffi_view_ty, ffi_value_ty, to_cpp_value, to_ffi_value); \ + __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ + uint32_t, u32, uint32_t, key, cpp_key, value_ty, rust_value_ty, \ + ffi_view_ty, ffi_value_ty, to_cpp_value, to_ffi_value); \ + __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ + bool, bool, bool, key, cpp_key, value_ty, rust_value_ty, ffi_view_ty, \ + ffi_value_ty, to_cpp_value, to_ffi_value); \ + __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ + uint64_t, u64, uint64_t, key, cpp_key, value_ty, rust_value_ty, \ + ffi_view_ty, ffi_value_ty, to_cpp_value, to_ffi_value); \ + __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ + int64_t, i64, int64_t, key, cpp_key, value_ty, rust_value_ty, \ + ffi_view_ty, ffi_value_ty, to_cpp_value, to_ffi_value); \ + __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ + std::string, ProtoString, google::protobuf::rust::PtrAndLen, \ + std::string(key.ptr, key.len), \ + google::protobuf::rust::PtrAndLen(cpp_key.data(), cpp_key.size()), value_ty, \ + rust_value_ty, ffi_view_ty, ffi_value_ty, to_cpp_value, to_ffi_value); #endif // GOOGLE_PROTOBUF_RUST_CPP_KERNEL_MAP_H__ diff --git a/rust/cpp_kernel/repeated.cc b/rust/cpp_kernel/repeated.cc index ce2d6c911b..79654fbad2 100644 --- a/rust/cpp_kernel/repeated.cc +++ b/rust/cpp_kernel/repeated.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include "google/protobuf/message.h" #include "google/protobuf/message_lite.h" @@ -56,45 +57,45 @@ expose_repeated_field_methods(uint64_t, u64); expose_repeated_field_methods(int64_t, i64); #undef expose_repeated_field_methods -#define expose_repeated_ptr_field_methods(ty) \ - google::protobuf::RepeatedPtrField* \ - proto2_rust_RepeatedField_##ty##_new() { \ - return new google::protobuf::RepeatedPtrField(); \ - } \ - void proto2_rust_RepeatedField_##ty##_free( \ - google::protobuf::RepeatedPtrField* r) { \ - delete r; \ - } \ - void proto2_rust_RepeatedField_##ty##_add( \ - google::protobuf::RepeatedPtrField* r, google::protobuf::rust::PtrAndLen val) { \ - r->Add(std::string(val.ptr, val.len)); \ - } \ - size_t proto2_rust_RepeatedField_##ty##_size( \ - google::protobuf::RepeatedPtrField* r) { \ - return r->size(); \ - } \ - google::protobuf::rust::PtrAndLen proto2_rust_RepeatedField_##ty##_get( \ - google::protobuf::RepeatedPtrField* r, size_t index) { \ - const std::string& s = r->Get(index); \ - return google::protobuf::rust::PtrAndLen(s.data(), s.size()); \ - } \ - void proto2_rust_RepeatedField_##ty##_set( \ - google::protobuf::RepeatedPtrField* r, size_t index, \ - google::protobuf::rust::PtrAndLen val) { \ - *r->Mutable(index) = std::string(val.ptr, val.len); \ - } \ - void proto2_rust_RepeatedField_##ty##_copy_from( \ - const google::protobuf::RepeatedPtrField* src, \ - google::protobuf::RepeatedPtrField* dst) { \ - dst->CopyFrom(*src); \ - } \ - void proto2_rust_RepeatedField_##ty##_clear( \ - google::protobuf::RepeatedPtrField* r) { \ - r->Clear(); \ - } \ - void proto2_rust_RepeatedField_##ty##_reserve( \ - google::protobuf::RepeatedPtrField* r, size_t additional) { \ - r->Reserve(r->size() + additional); \ +#define expose_repeated_ptr_field_methods(ty) \ + google::protobuf::RepeatedPtrField* \ + proto2_rust_RepeatedField_##ty##_new() { \ + return new google::protobuf::RepeatedPtrField(); \ + } \ + void proto2_rust_RepeatedField_##ty##_free( \ + google::protobuf::RepeatedPtrField* r) { \ + delete r; \ + } \ + void proto2_rust_RepeatedField_##ty##_add( \ + google::protobuf::RepeatedPtrField* r, std::string* val) { \ + r->AddAllocated(val); \ + } \ + size_t proto2_rust_RepeatedField_##ty##_size( \ + google::protobuf::RepeatedPtrField* r) { \ + return r->size(); \ + } \ + google::protobuf::rust::PtrAndLen proto2_rust_RepeatedField_##ty##_get( \ + google::protobuf::RepeatedPtrField* r, size_t index) { \ + const std::string& s = r->Get(index); \ + return google::protobuf::rust::PtrAndLen(s.data(), s.size()); \ + } \ + void proto2_rust_RepeatedField_##ty##_set( \ + google::protobuf::RepeatedPtrField* r, size_t index, \ + std::string* val) { \ + *r->Mutable(index) = std::move(*val); \ + } \ + void proto2_rust_RepeatedField_##ty##_copy_from( \ + const google::protobuf::RepeatedPtrField* src, \ + google::protobuf::RepeatedPtrField* dst) { \ + dst->CopyFrom(*src); \ + } \ + void proto2_rust_RepeatedField_##ty##_clear( \ + google::protobuf::RepeatedPtrField* r) { \ + r->Clear(); \ + } \ + void proto2_rust_RepeatedField_##ty##_reserve( \ + google::protobuf::RepeatedPtrField* r, size_t additional) { \ + r->Reserve(r->size() + additional); \ } expose_repeated_ptr_field_methods(ProtoString); diff --git a/rust/map.rs b/rust/map.rs index dc852408c2..bbb5bfc677 100644 --- a/rust/map.rs +++ b/rust/map.rs @@ -6,7 +6,7 @@ // https://developers.google.com/open-source/licenses/bsd use crate::{ - Mut, MutProxied, MutProxy, Proxied, View, ViewProxy, + IntoProxied, Mut, MutProxied, MutProxy, Proxied, View, ViewProxy, __internal::Private, __runtime::{InnerMap, InnerMapMut, RawMap, RawMapIter}, }; @@ -92,7 +92,11 @@ where fn map_clear(map: Mut<'_, Map>); fn map_len(map: View<'_, Map>) -> usize; - fn map_insert(map: Mut<'_, Map>, key: View<'_, K>, value: View<'_, Self>) -> bool; + fn map_insert( + map: Mut<'_, Map>, + key: View<'_, K>, + value: impl IntoProxied, + ) -> bool; fn map_get<'a>(map: View<'a, Map>, key: View<'_, K>) -> Option>; fn map_remove(map: Mut<'_, Map>, key: View<'_, K>) -> bool; @@ -283,16 +287,11 @@ where /// Adds a key-value pair to the map. /// /// Returns `true` if the entry was newly inserted. - pub fn insert<'a, 'b>( - &mut self, - key: impl Into>, - value: impl Into>, - ) -> bool + pub fn insert<'a>(&mut self, key: impl Into>, value: impl IntoProxied) -> bool where K: 'a, - V: 'b, { - V::map_insert(self.as_mut(), key.into(), value.into()) + V::map_insert(self.as_mut(), key.into(), value) } pub fn remove<'a>(&mut self, key: impl Into>) -> bool @@ -313,12 +312,11 @@ where V::map_get(self.as_view(), key.into()) } - pub fn copy_from<'a, 'b>( + pub fn copy_from<'a>( &mut self, - src: impl IntoIterator>, impl Into>)>, + src: impl IntoIterator>, impl IntoProxied)>, ) where K: 'a, - V: 'b, { //TODO self.clear(); @@ -446,7 +444,7 @@ where K: Proxied + ?Sized + 'msg + 'k, V: ProxiedInMapValue + ?Sized + 'msg + 'v, KView: Into>, - VView: Into>, + VView: IntoProxied, { fn extend>(&mut self, iter: T) { for (k, v) in iter.into_iter() { diff --git a/rust/primitive.rs b/rust/primitive.rs index 75665eca2b..b5f4b38108 100644 --- a/rust/primitive.rs +++ b/rust/primitive.rs @@ -4,7 +4,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file or at // https://developers.google.com/open-source/licenses/bsd -use crate::{Proxied, View, ViewProxy}; +use crate::__internal::Private; +use crate::{IntoProxied, Proxied, View, ViewProxy}; macro_rules! impl_singular_primitives { ($($t:ty),*) => { @@ -25,6 +26,12 @@ macro_rules! impl_singular_primitives { } } + impl IntoProxied<$t> for $t { + fn into_proxied(self, _private: Private) -> $t { + self + } + } + // ProxiedInRepeated is implemented in {cpp,upb}.rs )* } diff --git a/rust/repeated.rs b/rust/repeated.rs index 436933d705..5d455922f6 100644 --- a/rust/repeated.rs +++ b/rust/repeated.rs @@ -22,44 +22,44 @@ use crate::{ /// Views the elements in a `repeated` field of `T`. #[repr(transparent)] -pub struct RepeatedView<'msg, T: ?Sized> { +pub struct RepeatedView<'msg, T> { // This does not need to carry an arena in upb, so it can be just the raw repeated field raw: RawRepeatedField, _phantom: PhantomData<&'msg T>, } -impl<'msg, T: ?Sized> Copy for RepeatedView<'msg, T> {} -impl<'msg, T: ?Sized> Clone for RepeatedView<'msg, T> { +impl<'msg, T> Copy for RepeatedView<'msg, T> {} +impl<'msg, T> Clone for RepeatedView<'msg, T> { fn clone(&self) -> Self { *self } } -unsafe impl<'msg, T: ?Sized> Sync for RepeatedView<'msg, T> {} -unsafe impl<'msg, T: ?Sized> Send for RepeatedView<'msg, T> {} +unsafe impl<'msg, T> Sync for RepeatedView<'msg, T> {} +unsafe impl<'msg, T> Send for RepeatedView<'msg, T> {} -impl<'msg, T: ?Sized> Debug for RepeatedView<'msg, T> { +impl<'msg, T> Debug for RepeatedView<'msg, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("RepeatedView").field("raw", &self.raw).finish() } } /// Mutates the elements in a `repeated` field of `T`. -pub struct RepeatedMut<'msg, T: ?Sized> { +pub struct RepeatedMut<'msg, T> { pub(crate) inner: InnerRepeatedMut<'msg>, _phantom: PhantomData<&'msg mut T>, } -unsafe impl<'msg, T: ?Sized> Sync for RepeatedMut<'msg, T> {} +unsafe impl<'msg, T> Sync for RepeatedMut<'msg, T> {} -impl<'msg, T: ?Sized> Debug for RepeatedMut<'msg, T> { +impl<'msg, T> Debug for RepeatedMut<'msg, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("RepeatedMut").field("raw", &self.inner.raw).finish() } } #[doc(hidden)] -impl<'msg, T: ?Sized> RepeatedView<'msg, T> { +impl<'msg, T> RepeatedView<'msg, T> { #[doc(hidden)] #[inline] pub fn as_raw(&self, _private: Private) -> RawRepeatedField { @@ -77,7 +77,7 @@ impl<'msg, T: ?Sized> RepeatedView<'msg, T> { impl<'msg, T> RepeatedView<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'msg, + T: ProxiedInRepeated + 'msg, { /// Gets the length of the repeated field. #[inline] @@ -120,7 +120,7 @@ where } #[doc(hidden)] -impl<'msg, T: ?Sized> RepeatedMut<'msg, T> { +impl<'msg, T> RepeatedMut<'msg, T> { /// # Safety /// - `inner` must be valid to read and write from for `'msg` /// - There must be no aliasing references or mutations on the same @@ -140,7 +140,7 @@ impl<'msg, T: ?Sized> RepeatedMut<'msg, T> { impl<'msg, T> RepeatedMut<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'msg, + T: ProxiedInRepeated + 'msg, { /// Gets the length of the repeated field. #[inline] @@ -174,8 +174,7 @@ where /// Appends `val` to the end of the repeated field. #[inline] - pub fn push(&mut self, val: View) { - // TODO: b/320936046 - Use SettableValue instead of View for added ergonomics. + pub fn push(&mut self, val: impl IntoProxied) { T::repeated_push(self.as_mut(), val); } @@ -184,13 +183,11 @@ where /// # Panics /// Panics if `index >= len` #[inline] - pub fn set(&mut self, index: usize, val: View) { + pub fn set(&mut self, index: usize, val: impl IntoProxied) { let len = self.len(); if index >= len { panic!("index {index} >= repeated len {len}"); } - // TODO: b/320936046 - Use SettableValue instead of View for added ergonomics. - // SAFETY: `index` has been checked to be in-bounds. unsafe { self.set_unchecked(index, val) } } @@ -199,9 +196,7 @@ where /// # Safety /// Undefined behavior if `index >= len` #[inline] - pub unsafe fn set_unchecked(&mut self, index: usize, val: View) { - // TODO: b/320936046 - Use SettableValue instead of View for added ergonomics. - // SAFETY: `index` is in-bounds as promised by the caller. + pub unsafe fn set_unchecked(&mut self, index: usize, val: impl IntoProxied) { unsafe { T::repeated_set_unchecked(self.as_mut(), index, val) } } @@ -225,7 +220,7 @@ where impl Repeated where - T: ?Sized + ProxiedInRepeated, + T: ProxiedInRepeated, { pub fn as_view(&self) -> View> { RepeatedView { raw: self.inner.raw(), _phantom: PhantomData } @@ -239,7 +234,7 @@ where impl IntoProxied> for Repeated where - T: ?Sized + ProxiedInRepeated, + T: ProxiedInRepeated, { fn into_proxied(self, _private: Private) -> Repeated { self @@ -248,7 +243,7 @@ where impl<'msg, T> IntoProxied> for RepeatedView<'msg, T> where - T: 'msg + ?Sized + ProxiedInRepeated, + T: 'msg + ProxiedInRepeated, { fn into_proxied(self, _private: Private) -> Repeated { let mut repeated: Repeated = Repeated::new(); @@ -259,7 +254,7 @@ where impl<'msg, T> IntoProxied> for RepeatedMut<'msg, T> where - T: 'msg + ?Sized + ProxiedInRepeated, + T: 'msg + ProxiedInRepeated, { fn into_proxied(self, _private: Private) -> Repeated { IntoProxied::into_proxied(self.as_view(), _private) @@ -296,7 +291,7 @@ pub unsafe trait ProxiedInRepeated: Proxied { fn repeated_len(repeated: View>) -> usize; /// Appends a new element to the end of the repeated field. - fn repeated_push(repeated: Mut>, val: View); + fn repeated_push(repeated: Mut>, val: impl IntoProxied); /// Clears the repeated field of elements. fn repeated_clear(repeated: Mut>); @@ -307,7 +302,11 @@ pub unsafe trait ProxiedInRepeated: Proxied { /// # Safety /// `index` must be less than `Self::repeated_len(repeated)` - unsafe fn repeated_set_unchecked(repeated: Mut>, index: usize, val: View); + unsafe fn repeated_set_unchecked( + repeated: Mut>, + index: usize, + val: impl IntoProxied, + ); /// Copies the values in the `src` repeated field into `dest`. fn repeated_copy_from(src: View>, dest: Mut>); @@ -318,12 +317,12 @@ pub unsafe trait ProxiedInRepeated: Proxied { } /// An iterator over the values inside of a [`View>`](RepeatedView). -pub struct RepeatedIter<'msg, T: ?Sized> { +pub struct RepeatedIter<'msg, T> { view: RepeatedView<'msg, T>, current_index: usize, } -impl<'msg, T: ?Sized> Debug for RepeatedIter<'msg, T> { +impl<'msg, T> Debug for RepeatedIter<'msg, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("RepeatedIter") .field("view", &self.view) @@ -336,19 +335,19 @@ impl<'msg, T: ?Sized> Debug for RepeatedIter<'msg, T> { /// /// Users will generally write [`View>`](RepeatedView) or /// [`Mut>`](RepeatedMut) to access the repeated elements -pub struct Repeated { +pub struct Repeated { pub(crate) inner: InnerRepeated, _phantom: PhantomData, } // SAFETY: `Repeated` is Sync because it does not implement interior mutability. -unsafe impl Sync for Repeated {} +unsafe impl Sync for Repeated {} // SAFETY: `Repeated` is Send because it's not bound to a specific thread e.g. // it does not use thread-local data or similar. -unsafe impl Send for Repeated {} +unsafe impl Send for Repeated {} -impl Repeated { +impl Repeated { pub fn new() -> Self { T::repeated_new(Private) } @@ -362,13 +361,13 @@ impl Repeated { } } -impl Default for Repeated { +impl Default for Repeated { fn default() -> Self { Repeated::new() } } -impl Drop for Repeated { +impl Drop for Repeated { fn drop(&mut self) { // SAFETY: only called once unsafe { T::repeated_free(Private, self) } @@ -377,21 +376,21 @@ impl Drop for Repeated { impl Proxied for Repeated where - T: ProxiedInRepeated + ?Sized, + T: ProxiedInRepeated, { type View<'msg> = RepeatedView<'msg, T> where Repeated: 'msg; } impl MutProxied for Repeated where - T: ProxiedInRepeated + ?Sized, + T: ProxiedInRepeated, { type Mut<'msg> = RepeatedMut<'msg, T> where Repeated: 'msg; } impl<'msg, T> ViewProxy<'msg> for RepeatedView<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'msg, + T: ProxiedInRepeated + 'msg, { type Proxied = Repeated; @@ -411,7 +410,7 @@ where impl<'msg, T> ViewProxy<'msg> for RepeatedMut<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'msg, + T: ProxiedInRepeated + 'msg, { type Proxied = Repeated; @@ -431,7 +430,7 @@ where impl<'msg, T> MutProxy<'msg> for RepeatedMut<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'msg, + T: ProxiedInRepeated + 'msg, { #[inline] fn as_mut(&mut self) -> Mut<'_, Self::Proxied> { @@ -449,7 +448,7 @@ where impl<'msg, T> iter::Iterator for RepeatedIter<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'msg, + T: ProxiedInRepeated + 'msg, { type Item = View<'msg, T>; @@ -468,18 +467,18 @@ where } } -impl<'msg, T: ?Sized + ProxiedInRepeated> ExactSizeIterator for RepeatedIter<'msg, T> { +impl<'msg, T: ProxiedInRepeated> ExactSizeIterator for RepeatedIter<'msg, T> { fn len(&self) -> usize { self.view.len() - self.current_index } } // TODO: impl DoubleEndedIterator -impl<'msg, T: ?Sized + ProxiedInRepeated> FusedIterator for RepeatedIter<'msg, T> {} +impl<'msg, T: ProxiedInRepeated> FusedIterator for RepeatedIter<'msg, T> {} impl<'msg, T> iter::IntoIterator for RepeatedView<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'msg, + T: ProxiedInRepeated + 'msg, { type Item = View<'msg, T>; type IntoIter = RepeatedIter<'msg, T>; @@ -491,7 +490,7 @@ where impl<'msg, T> iter::IntoIterator for &'_ RepeatedView<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'msg, + T: ProxiedInRepeated + 'msg, { type Item = View<'msg, T>; type IntoIter = RepeatedIter<'msg, T>; @@ -503,7 +502,7 @@ where impl<'borrow, T> iter::IntoIterator for &'borrow RepeatedMut<'_, T> where - T: ProxiedInRepeated + ?Sized + 'borrow, + T: ProxiedInRepeated + 'borrow, { type Item = View<'borrow, T>; type IntoIter = RepeatedIter<'borrow, T>; @@ -515,14 +514,14 @@ where impl<'msg, 'view, T, ViewT> Extend for RepeatedMut<'msg, T> where - T: ProxiedInRepeated + ?Sized + 'view, - ViewT: Into>, + T: ProxiedInRepeated + 'view, + ViewT: IntoProxied, { fn extend>(&mut self, iter: I) { let iter = iter.into_iter(); T::repeated_reserve(self.as_mut(), iter.size_hint().0); for item in iter { - self.push(item.into()); + self.push(item); } } } diff --git a/rust/string.rs b/rust/string.rs index 589721c42e..3f82e14b67 100644 --- a/rust/string.rs +++ b/rust/string.rs @@ -164,6 +164,12 @@ impl ProtoString { } } +impl From for ProtoBytes { + fn from(v: ProtoString) -> Self { + ProtoBytes { inner: v.inner } + } +} + impl From<&str> for ProtoString { fn from(v: &str) -> Self { Self::from(v.as_bytes()) diff --git a/rust/test/shared/accessors_map_test.rs b/rust/test/shared/accessors_map_test.rs index b743180642..26a309de28 100644 --- a/rust/test/shared/accessors_map_test.rs +++ b/rust/test/shared/accessors_map_test.rs @@ -142,7 +142,7 @@ fn test_bytes_and_string_copied() { // Ensure val is dropped after inserting into the map. let mut key = String::from("hello"); let mut val = String::from("world"); - msg.map_string_string_mut().insert(key.as_str(), val.as_str()); + msg.map_string_string_mut().insert(key.as_str(), &val); msg.map_int32_bytes_mut().insert(1, val.as_bytes()); // Validate that map keys are copied by mutating the originals. key.replace_range(.., "ayo"); @@ -233,7 +233,7 @@ macro_rules! generate_map_with_msg_values_tests { assert_that!( - msg.[< map_ $k_field _all_types_mut >]().insert($k_nonzero, TestAllTypes::new().as_view()), + msg.[< map_ $k_field _all_types_mut >]().insert($k_nonzero, TestAllTypes::new()), eq(true)); assert_that!( msg.[< map_ $k_field _all_types_mut >]().remove($k_nonzero), @@ -264,7 +264,7 @@ macro_rules! generate_map_with_msg_values_tests { // single element iter assert_that!( - msg.[< map_ $k_field _all_types_mut >]().insert($k_nonzero, TestAllTypes::new().as_view()), + msg.[< map_ $k_field _all_types_mut >]().insert($k_nonzero, TestAllTypes::new()), eq(true)); // assert_that!( // msg.[< map_ $k_field _all_types >]().iter().collect::>(), @@ -285,7 +285,7 @@ macro_rules! generate_map_with_msg_values_tests { assert_that!( msg .[< map_ $k_field _all_types_mut >]() - .insert($k_other, TestAllTypes::new().as_view()), + .insert($k_other, TestAllTypes::new()), eq(true)); assert_that!( diff --git a/rust/test/shared/accessors_repeated_test.rs b/rust/test/shared/accessors_repeated_test.rs index b93b12325b..cf3b9922ae 100644 --- a/rust/test/shared/accessors_repeated_test.rs +++ b/rust/test/shared/accessors_repeated_test.rs @@ -193,7 +193,7 @@ fn test_repeated_message() { assert_that!(msg.repeated_nested_message().len(), eq(0)); let mut nested = NestedMessage::new(); nested.set_bb(1); - msg.repeated_nested_message_mut().push(nested.as_view()); + msg.repeated_nested_message_mut().push(nested); assert_that!(msg.repeated_nested_message().get(0).unwrap().bb(), eq(1)); let mut msg2 = TestAllTypes::new(); @@ -203,8 +203,7 @@ fn test_repeated_message() { let mut nested2 = NestedMessage::new(); nested2.set_bb(2); - // TODO: b/320936046 - Test SettableValue once available - msg.repeated_nested_message_mut().set(0, nested2.as_view()); + msg.repeated_nested_message_mut().set(0, nested2); assert_that!(msg.repeated_nested_message().get(0).unwrap().bb(), eq(2)); assert_that!( @@ -227,14 +226,12 @@ fn test_repeated_strings() { assert_that!(msg.repeated_string(), empty()); { let s = String::from("set from Mut"); - // TODO: b/320936046 - Test SettableValue once available - msg.repeated_string_mut().push(s.as_str().into()); + msg.repeated_string_mut().push(s); } - msg.repeated_string_mut().push("second str".into()); + msg.repeated_string_mut().push("second str"); { let s2 = String::from("set second str"); - // TODO: b/320936046 - Test SettableValue once available - msg.repeated_string_mut().set(1, s2.as_str().into()); + msg.repeated_string_mut().set(1, s2); } assert_that!(msg.repeated_string().len(), eq(2)); assert_that!(msg.repeated_string().get(0).unwrap(), eq("set from Mut")); @@ -264,14 +261,12 @@ fn test_repeated_bytes() { assert_that!(msg.repeated_bytes(), empty()); { let s = Vec::from(b"set from Mut"); - // TODO: b/320936046 - Test SettableValue once available - msg.repeated_bytes_mut().push(&s[..]); + msg.repeated_bytes_mut().push(s); } msg.repeated_bytes_mut().push(b"second bytes"); { let s2 = Vec::from(b"set second bytes"); - // TODO: b/320936046 - Test SettableValue once available - msg.repeated_bytes_mut().set(1, &s2[..]); + msg.repeated_bytes_mut().set(1, s2); } assert_that!(msg.repeated_bytes().len(), eq(2)); assert_that!(msg.repeated_bytes().get(0).unwrap(), eq(b"set from Mut")); diff --git a/rust/test/shared/edition2023_test.rs b/rust/test/shared/edition2023_test.rs index 9c5aeba1fe..824e33b0be 100644 --- a/rust/test/shared/edition2023_test.rs +++ b/rust/test/shared/edition2023_test.rs @@ -32,7 +32,7 @@ fn string_view_works() { fn repeated_string_view_works() { let mut msg = edition2023_rust_proto::EditionsMessage::new(); assert_that!(msg.repeated_str_view().len(), eq(0)); - msg.repeated_str_view_mut().push("first".into()); + msg.repeated_str_view_mut().push("first"); assert_that!(msg.repeated_str_view().len(), eq(1)); assert_that!(msg.repeated_str_view().get(0), some(eq("first"))); } diff --git a/rust/test/upb/string_ctypes_test.rs b/rust/test/upb/string_ctypes_test.rs index 58f25b207c..6bbea67a21 100644 --- a/rust/test/upb/string_ctypes_test.rs +++ b/rust/test/upb/string_ctypes_test.rs @@ -12,7 +12,7 @@ use unittest_proto3_rust_proto::TestAllTypes; fn test_stringpiece_repeated() { let mut msg = TestAllTypes::new(); assert_that!(msg.repeated_string_piece().len(), eq(0)); - msg.repeated_string_piece_mut().push("hello".into()); + msg.repeated_string_piece_mut().push("hello"); assert_that!(msg.repeated_string_piece().len(), eq(1)); assert_that!(msg.repeated_string_piece().get(0), some(eq("hello"))); } @@ -29,7 +29,7 @@ fn test_cord() { fn test_cord_repeated() { let mut msg = TestAllTypes::new(); assert_that!(msg.repeated_cord().len(), eq(0)); - msg.repeated_cord_mut().push("hello".into()); + msg.repeated_cord_mut().push("hello"); assert_that!(msg.repeated_cord().len(), eq(1)); assert_that!(msg.repeated_cord().get(0), some(eq("hello"))); } diff --git a/rust/upb.rs b/rust/upb.rs index 4b1ab8b0c5..e53e0aa98f 100644 --- a/rust/upb.rs +++ b/rust/upb.rs @@ -226,12 +226,15 @@ macro_rules! impl_repeated_base { unsafe { upb_Array_Size(f.as_raw(Private)) } } #[inline] - fn repeated_push(mut f: Mut>, v: View<$t>) { + fn repeated_push(mut f: Mut>, v: impl IntoProxied<$t>) { let arena = f.raw_arena(Private); unsafe { assert!(upb_Array_Append( f.as_raw(Private), - <$t as UpbTypeConversions>::to_message_value_copy_if_required(arena, v), + <$t as UpbTypeConversions>::into_message_value_fuse_if_required( + arena, + v.into_proxied(Private) + ), arena, )); } @@ -249,13 +252,20 @@ macro_rules! impl_repeated_base { } } #[inline] - unsafe fn repeated_set_unchecked(mut f: Mut>, i: usize, v: View<$t>) { + unsafe fn repeated_set_unchecked( + mut f: Mut>, + i: usize, + v: impl IntoProxied<$t>, + ) { let arena = f.raw_arena(Private); unsafe { upb_Array_Set( f.as_raw(Private), i, - <$t as UpbTypeConversions>::to_message_value_copy_if_required(arena, v.into()), + <$t as UpbTypeConversions>::into_message_value_fuse_if_required( + arena, + v.into_proxied(Private), + ), ) } } @@ -340,7 +350,7 @@ macro_rules! impl_repeated_bytes { } } -impl<'msg, T: ?Sized> RepeatedMut<'msg, T> { +impl<'msg, T> RepeatedMut<'msg, T> { // Returns a `RawArena` which is live for at least `'msg` #[doc(hidden)] pub fn raw_arena(&mut self, _private: Private) -> RawArena { @@ -538,13 +548,14 @@ impl<'msg> InnerMapMut<'msg> { pub trait UpbTypeConversions: Proxied { fn upb_type() -> upb::CType; + fn to_message_value(val: View<'_, Self>) -> upb_MessageValue; /// # Safety /// - `raw_arena` must point to a valid upb arena. - unsafe fn to_message_value_copy_if_required( + unsafe fn into_message_value_fuse_if_required( raw_arena: RawArena, - val: View<'_, Self>, + val: Self, ) -> upb_MessageValue; /// # Safety @@ -568,7 +579,7 @@ macro_rules! impl_upb_type_conversions_for_scalars { } #[inline(always)] - unsafe fn to_message_value_copy_if_required(_: RawArena, val: View<'_, $t>) -> upb_MessageValue { + unsafe fn into_message_value_fuse_if_required(_: RawArena, val: $t) -> upb_MessageValue { Self::to_message_value(val) } @@ -600,14 +611,17 @@ impl UpbTypeConversions for ProtoBytes { upb_MessageValue { str_val: val.into() } } - unsafe fn to_message_value_copy_if_required( - raw_arena: RawArena, - val: View<'_, ProtoBytes>, + unsafe fn into_message_value_fuse_if_required( + raw_parent_arena: RawArena, + val: ProtoBytes, ) -> upb_MessageValue { // SAFETY: The arena memory is not freed due to `ManuallyDrop`. - let arena = ManuallyDrop::new(unsafe { Arena::from_raw(raw_arena) }); - let copied = copy_bytes_in_arena(&arena, val); - let msg_val = Self::to_message_value(copied); + let parent_arena = ManuallyDrop::new(unsafe { Arena::from_raw(raw_parent_arena) }); + let (data, arena) = val.inner.0.into_parts(); + + parent_arena.fuse(&arena); + + let msg_val = Self::to_message_value(unsafe { data.as_ref() }); msg_val } @@ -625,15 +639,15 @@ impl UpbTypeConversions for ProtoString { upb_MessageValue { str_val: val.as_bytes().into() } } - unsafe fn to_message_value_copy_if_required( + unsafe fn into_message_value_fuse_if_required( raw_arena: RawArena, - val: View<'_, ProtoString>, + val: ProtoString, ) -> upb_MessageValue { // SAFETY: `raw_arena` is valid as promised by the caller unsafe { - ::to_message_value_copy_if_required( + ::into_message_value_fuse_if_required( raw_arena, - val.as_bytes(), + val.into(), ) } } @@ -701,13 +715,13 @@ macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types { } } - fn map_insert(mut map: Mut<'_, Map<$key_t, Self>>, key: View<'_, $key_t>, value: View<'_, Self>) -> bool { + fn map_insert(mut map: Mut<'_, Map<$key_t, Self>>, key: View<'_, $key_t>, value: impl IntoProxied) -> bool { let arena = map.raw_arena(Private); unsafe { upb_Map_InsertAndReturnIfInserted( map.as_raw(Private), <$key_t as UpbTypeConversions>::to_message_value(key), - <$t as UpbTypeConversions>::to_message_value_copy_if_required(arena, value), + <$t as UpbTypeConversions>::into_message_value_fuse_if_required(arena, value.into_proxied(Private)), arena ) } diff --git a/src/google/protobuf/compiler/rust/enum.cc b/src/google/protobuf/compiler/rust/enum.cc index fc3cc16423..6c2117928d 100644 --- a/src/google/protobuf/compiler/rust/enum.cc +++ b/src/google/protobuf/compiler/rust/enum.cc @@ -105,8 +105,8 @@ void EnumProxiedInMapValue(Context& ctx, const EnumDescriptor& desc) { unsafe { $map_size_thunk$(map.as_raw($pbi$::Private)) } } - fn map_insert(mut map: $pb$::Mut<'_, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>, value: $pb$::View<'_, Self>) -> bool { - unsafe { $map_insert_thunk$(map.as_raw($pbi$::Private), $to_ffi_key_expr$, value) } + fn map_insert(mut map: $pb$::Mut<'_, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>, value: impl $pb$::IntoProxied) -> bool { + unsafe { $map_insert_thunk$(map.as_raw($pbi$::Private), $to_ffi_key_expr$, value.into_proxied($pbi$::Private)) } } fn map_get<'a>(map: $pb$::View<'a, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>) -> Option<$pb$::View<'a, Self>> { @@ -193,13 +193,13 @@ void EnumProxiedInMapValue(Context& ctx, const EnumDescriptor& desc) { } } - fn map_insert(mut map: $pb$::Mut<'_, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>, value: $pb$::View<'_, Self>) -> bool { + fn map_insert(mut map: $pb$::Mut<'_, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>, value: impl $pb$::IntoProxied) -> bool { let arena = map.inner($pbi$::Private).raw_arena($pbi$::Private); unsafe { $pbr$::upb_Map_InsertAndReturnIfInserted( map.as_raw($pbi$::Private), <$key_t$ as $pbr$::UpbTypeConversions>::to_message_value(key), - $pbr$::upb_MessageValue { int32_val: value.0 }, + $pbr$::upb_MessageValue { int32_val: value.into_proxied($pbi$::Private).0 }, arena ) } @@ -389,6 +389,18 @@ void GenerateEnumDefinition(Context& ctx, const EnumDescriptor& desc) { } } + impl $pb$::IntoProxied<$name$> for $name$ { + fn into_proxied(self, _: $pbi$::Private) -> Self { + self + } + } + + impl $pb$::IntoProxied for $name$ { + fn into_proxied(self, _: $pbi$::Private) -> i32 { + self.0 + } + } + impl $pb$::Proxied for $name$ { type View<'a> = $name$; } @@ -410,8 +422,8 @@ void GenerateEnumDefinition(Context& ctx, const EnumDescriptor& desc) { $pbr$::cast_enum_repeated_view($pbi$::Private, r).len() } - fn repeated_push(r: $pb$::Mut<$pb$::Repeated>, val: $name$) { - $pbr$::cast_enum_repeated_mut($pbi$::Private, r).push(val.into()) + fn repeated_push(r: $pb$::Mut<$pb$::Repeated>, val: impl $pb$::IntoProxied<$name$>) { + $pbr$::cast_enum_repeated_mut($pbi$::Private, r).push(val.into_proxied($pbi$::Private)) } fn repeated_clear(r: $pb$::Mut<$pb$::Repeated>) { @@ -434,12 +446,12 @@ void GenerateEnumDefinition(Context& ctx, const EnumDescriptor& desc) { unsafe fn repeated_set_unchecked( r: $pb$::Mut<$pb$::Repeated>, index: usize, - val: $name$, + val: impl $pb$::IntoProxied<$name$>, ) { // SAFETY: In-bounds as promised by the caller. unsafe { $pbr$::cast_enum_repeated_mut($pbi$::Private, r) - .set_unchecked(index, val.into()) + .set_unchecked(index, val.into_proxied($pbi$::Private)) } } @@ -484,7 +496,7 @@ void GenerateEnumThunksCc(Context& ctx, const EnumDescriptor& desc) { R"cc( extern $abi$ { __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE( - $cpp_t$, $rs_t$, $cpp_t$, value, cpp_value) + $cpp_t$, $rs_t$, $cpp_t$, $cpp_t$, value, cpp_value) } )cc"); } diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc index 512d64d5ee..9eac1897cc 100644 --- a/src/google/protobuf/compiler/rust/message.cc +++ b/src/google/protobuf/compiler/rust/message.cc @@ -374,7 +374,7 @@ void MessageProxiedInRepeated(Context& ctx, const Descriptor& msg) { unsafe fn repeated_set_unchecked( mut f: $pb$::Mut<$pb$::Repeated>, i: usize, - v: $pb$::View, + v: impl $pb$::IntoProxied, ) { // SAFETY: // - `f.as_raw()` is a valid `RepeatedPtrField*`. @@ -383,7 +383,7 @@ void MessageProxiedInRepeated(Context& ctx, const Descriptor& msg) { unsafe { $copy_from_thunk$( $repeated_get_mut_thunk$(f.as_raw($pbi$::Private), i), - v.raw_msg(), + v.into_proxied($pbi$::Private).raw_msg(), ); } } @@ -404,13 +404,13 @@ void MessageProxiedInRepeated(Context& ctx, const Descriptor& msg) { unsafe { $repeated_clear_thunk$(f.as_raw($pbi$::Private)) }; } - fn repeated_push(mut f: $pb$::Mut<$pb$::Repeated>, v: $pb$::View) { + fn repeated_push(mut f: $pb$::Mut<$pb$::Repeated>, v: impl $pb$::IntoProxied) { // SAFETY: // - `f.as_raw()` is a valid `RepeatedPtrField*`. // - `v.raw_msg()` is a valid `const Message&`. unsafe { let new_elem = $repeated_add_thunk$(f.as_raw($pbi$::Private)); - $copy_from_thunk$(new_elem, v.raw_msg()); + $copy_from_thunk$(new_elem, v.into_proxied($pbi$::Private).raw_msg()); } } @@ -452,26 +452,18 @@ void MessageProxiedInRepeated(Context& ctx, const Descriptor& msg) { unsafe fn repeated_set_unchecked( mut f: $pb$::Mut<$pb$::Repeated>, i: usize, - v: $pb$::View, + v: impl $pb$::IntoProxied, ) { - // SAFETY: - // - `f.as_raw()` is a valid `upb_Array*`. - // - `i < len(f)` is promised by the caller. - let dest_msg = unsafe { - $pbr$::upb_Array_GetMutable(f.as_raw($pbi$::Private), i).msg - }.expect("upb_Array* element should not be NULL"); - - // SAFETY: - // - `dest_msg` is a valid `upb_Message*`. - // - `v.raw_msg()` and `dest_msg` both have message minitable `$minitable$`. unsafe { - $pbr$::upb_Message_DeepCopy( - dest_msg, - v.raw_msg(), - $std$::ptr::addr_of!($minitable$), - f.raw_arena($pbi$::Private), - ) - }; + $pbr$::upb_Array_Set( + f.as_raw($pbi$::Private), + i, + ::into_message_value_fuse_if_required( + f.raw_arena($pbi$::Private), + v.into_proxied($pbi$::Private), + ), + ) + } } unsafe fn repeated_get_unchecked( @@ -493,26 +485,15 @@ void MessageProxiedInRepeated(Context& ctx, const Descriptor& msg) { $pbr$::upb_Array_Resize(f.as_raw($pbi$::Private), 0, f.raw_arena($pbi$::Private)) }; } - fn repeated_push(mut f: $pb$::Mut<$pb$::Repeated>, v: $pb$::View) { - // SAFETY: - // - `v.raw_msg()` is a valid `const upb_Message*` with minitable `$minitable$`. - let msg_ptr = unsafe { - $pbr$::upb_Message_DeepClone( - v.raw_msg(), - std::ptr::addr_of!($minitable$), - f.raw_arena($pbi$::Private), - ) - }.expect("upb_Message_DeepClone failed."); - - // Append new default message to array. + fn repeated_push(mut f: $pb$::Mut<$pb$::Repeated>, v: impl $pb$::IntoProxied) { // SAFETY: // - `f.as_raw()` is a valid `upb_Array*`. // - `msg_ptr` is a valid `upb_Message*`. unsafe { $pbr$::upb_Array_Append( f.as_raw($pbi$::Private), - $pbr$::upb_MessageValue{msg_val: Some(msg_ptr)}, - f.raw_arena($pbi$::Private), + ::into_message_value_fuse_if_required(f.raw_arena($pbi$::Private), v.into_proxied($pbi$::Private)), + f.raw_arena($pbi$::Private) ); }; } @@ -605,8 +586,8 @@ void MessageProxiedInMapValue(Context& ctx, const Descriptor& msg) { unsafe { $map_size_thunk$(map.as_raw($pbi$::Private)) } } - fn map_insert(mut map: $pb$::Mut<'_, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>, value: $pb$::View<'_, Self>) -> bool { - unsafe { $map_insert_thunk$(map.as_raw($pbi$::Private), $key_expr$, value.raw_msg()) } + fn map_insert(mut map: $pb$::Mut<'_, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>, value: impl $pb$::IntoProxied) -> bool { + unsafe { $map_insert_thunk$(map.as_raw($pbi$::Private), $key_expr$, value.into_proxied($pbi$::Private).raw_msg()) } } fn map_get<'a>(map: $pb$::View<'a, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>) -> Option<$pb$::View<'a, Self>> { @@ -675,17 +656,15 @@ void MessageProxiedInMapValue(Context& ctx, const Descriptor& msg) { $pbr$::upb_MessageValue { msg_val: Some(val.raw_msg()) } } - unsafe fn to_message_value_copy_if_required( - arena: $pbr$::RawArena, - val: $pb$::View<'_, Self>) -> $pbr$::upb_MessageValue { - // Self::to_message_value(val) + unsafe fn into_message_value_fuse_if_required( + raw_parent_arena: $pbr$::RawArena, + mut val: Self) -> $pbr$::upb_MessageValue { // SAFETY: The arena memory is not freed due to `ManuallyDrop`. - let cloned_msg = $pbr$::upb_Message_DeepClone( - val.raw_msg(), $std$::ptr::addr_of!($minitable$), arena) - .expect("upb_Message_DeepClone failed."); - Self::to_message_value( - $Msg$View::new($pbi$::Private, cloned_msg)) - } + let parent_arena = core::mem::ManuallyDrop::new(unsafe { $pbr$::Arena::from_raw(raw_parent_arena) }); + + parent_arena.fuse(val.as_mutator_message_ref($pbi$::Private).arena($pbi$::Private)); + $pbr$::upb_MessageValue { msg_val: Some(val.raw_msg()) } + } unsafe fn from_message_value<'msg>(msg: $pbr$::upb_MessageValue) -> $pb$::View<'msg, Self> { @@ -731,13 +710,13 @@ void MessageProxiedInMapValue(Context& ctx, const Descriptor& msg) { } } - fn map_insert(mut map: $pb$::Mut<'_, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>, value: $pb$::View<'_, Self>) -> bool { + fn map_insert(mut map: $pb$::Mut<'_, $pb$::Map<$key_t$, Self>>, key: $pb$::View<'_, $key_t$>, value: impl $pb$::IntoProxied) -> bool { let arena = map.inner($pbi$::Private).raw_arena($pbi$::Private); unsafe { $pbr$::upb_Map_InsertAndReturnIfInserted( map.as_raw($pbi$::Private), <$key_t$ as $pbr$::UpbTypeConversions>::to_message_value(key), - ::to_message_value_copy_if_required(arena, value), + ::into_message_value_fuse_if_required(arena, value.into_proxied($pbi$::Private)), arena ) }