From 976029283f00b1b2d7a16209ef8bfc94eb82a3f1 Mon Sep 17 00:00:00 2001 From: Jakob Buchgraber <buchgr@google.com> Date: Thu, 7 Dec 2023 07:19:54 -0800 Subject: [PATCH] Implement Maps for strings This change implements maps with keys and values of type string e.g. Map<ProtoStr, i32> and Map<ProtoStr, ProtoStr>. Implementing the Map type for ProtoStr has been different from scalar types because ProtoStr is an unsized type i.e. its size is not known at compile time. The existing Map implementation assumed sized types in many places. To make unsized types fit into the existing code architecture I have added an associated type 'Value' to the MapWith*KeyOps traits. The associated type needs to be sized and is the type returned by the Map::get(self, key) method e.g. for aProtoStr, the `type Value = &ProtoStr`. PiperOrigin-RevId: 588783751 --- rust/cpp.rs | 255 ++++++++++------ rust/cpp_kernel/BUILD | 1 + rust/cpp_kernel/cpp_api.cc | 120 ++++---- rust/map.rs | 197 +++++++++---- rust/shared.rs | 2 +- rust/test/shared/accessors_map_test.rs | 12 + rust/upb.rs | 272 ++++++++++++------ .../compiler/rust/accessors/accessors.cc | 27 +- .../protobuf/compiler/rust/accessors/map.cc | 15 +- 9 files changed, 603 insertions(+), 298 deletions(-) diff --git a/rust/cpp.rs b/rust/cpp.rs index 61e0aee382..d5743f6ad4 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -7,11 +7,13 @@ // Rust Protobuf runtime using the C++ kernel. -use crate::__internal::{Private, RawArena, RawMap, RawMessage, RawRepeatedField}; +use crate::ProtoStr; +use crate::__internal::{Private, PtrAndLen, RawArena, RawMap, RawMessage, RawRepeatedField}; use core::fmt::Debug; use paste::paste; use std::alloc::Layout; use std::cell::UnsafeCell; +use std::convert::identity; use std::fmt; use std::marker::PhantomData; use std::mem::MaybeUninit; @@ -327,9 +329,6 @@ pub struct MapInner<'msg, K: ?Sized, V: ?Sized> { pub _phantom_value: PhantomData<&'msg mut V>, } -// These use manual impls instead of derives to avoid unnecessary bounds on `K` -// and `V`. This problem is referred to as "perfect derive". -// https://smallcultfollowing.com/babysteps/blog/2022/04/12/implied-bounds-and-perfect-derive/ impl<'msg, K: ?Sized, V: ?Sized> Copy for MapInner<'msg, K, V> {} impl<'msg, K: ?Sized, V: ?Sized> Clone for MapInner<'msg, K, V> { fn clone(&self) -> MapInner<'msg, K, V> { @@ -337,78 +336,22 @@ impl<'msg, K: ?Sized, V: ?Sized> Clone for MapInner<'msg, K, V> { } } -macro_rules! impl_scalar_map_values { - ($kt:ty, $trait:ident for $($t:ty),*) => { - paste! { $( - extern "C" { - fn [< __pb_rust_Map_ $kt _ $t _new >]() -> RawMap; - fn [< __pb_rust_Map_ $kt _ $t _clear >](m: RawMap); - fn [< __pb_rust_Map_ $kt _ $t _size >](m: RawMap) -> usize; - fn [< __pb_rust_Map_ $kt _ $t _insert >](m: RawMap, key: $kt, value: $t); - fn [< __pb_rust_Map_ $kt _ $t _get >](m: RawMap, key: $kt, value: *mut $t) -> bool; - fn [< __pb_rust_Map_ $kt _ $t _remove >](m: RawMap, key: $kt, value: *mut $t) -> bool; - } - impl $trait for $t { - fn new_map() -> RawMap { - unsafe { [< __pb_rust_Map_ $kt _ $t _new >]() } - } - - fn clear(m: RawMap) { - unsafe { [< __pb_rust_Map_ $kt _ $t _clear >](m) } - } - - fn size(m: RawMap) -> usize { - unsafe { [< __pb_rust_Map_ $kt _ $t _size >](m) } - } +macro_rules! generate_map_with_key_ops_traits { + ($($t:ty, $sized_t:ty;)*) => { + paste! { + $( + pub trait [< MapWith $t:camel KeyOps >] { + type Value<'a>: Sized; - fn insert(m: RawMap, key: $kt, value: $t) { - unsafe { [< __pb_rust_Map_ $kt _ $t _insert >](m, key, value) } - } - - fn get(m: RawMap, key: $kt) -> Option<$t> { - let mut val: $t = Default::default(); - let found = unsafe { [< __pb_rust_Map_ $kt _ $t _get >](m, key, &mut val) }; - if !found { - return None; - } - Some(val) - } - - fn remove(m: RawMap, key: $kt) -> Option<$t> { - let mut val: $t = Default::default(); - let removed = - unsafe { [< __pb_rust_Map_ $kt _ $t _remove >](m, key, &mut val) }; - if !removed { - return None; - } - Some(val) - } - } - )* } - } -} - -macro_rules! impl_scalar_maps { - ($($t:ty),*) => { - paste! { $( - pub trait [< MapWith $t:camel KeyOps >] : Sync + Send + Copy + Clone + Debug { fn new_map() -> RawMap; fn clear(m: RawMap); fn size(m: RawMap) -> usize; - fn insert(m: RawMap, key: $t, value: Self); - fn get(m: RawMap, key: $t) -> Option<Self> - where - Self: Sized; - fn remove(m: RawMap, key: $t) -> Option<Self> - where - Self: Sized; + fn insert(m: RawMap, key: $sized_t, value: Self::Value<'_>) -> bool; + fn get<'a>(m: RawMap, key: $sized_t) -> Option<Self::Value<'a>>; + fn remove<'a>(m: RawMap, key: $sized_t) -> bool; } - impl_scalar_map_values!( - $t, [< MapWith $t:camel KeyOps >] for i32, u32, f32, f64, bool, u64, i64 - ); - - impl<'msg, V: [< MapWith $t:camel KeyOps >]> Default for MapInner<'msg, $t, V> { + impl<'msg, V: [< MapWith $t:camel KeyOps >] + ?Sized> Default for MapInner<'msg, $t, V> { fn default() -> Self { MapInner { raw: V::new_map(), @@ -418,7 +361,7 @@ macro_rules! impl_scalar_maps { } } - impl<'msg, V: [< MapWith $t:camel KeyOps >]> MapInner<'msg, $t, V> { + impl<'msg, V: [< MapWith $t:camel KeyOps >] + ?Sized> MapInner<'msg, $t, V> { pub fn size(&self) -> usize { V::size(self.raw) } @@ -427,27 +370,129 @@ macro_rules! impl_scalar_maps { V::clear(self.raw) } - pub fn get(&self, key: $t) -> Option<V> { + pub fn get<'a>(&self, key: $sized_t) -> Option<V::Value<'a>> { V::get(self.raw, key) } - pub fn remove(&mut self, key: $t) -> Option<V> { + pub fn remove<'a>(&mut self, key: $sized_t) -> bool { V::remove(self.raw, key) } - pub fn insert(&mut self, key: $t, value: V) -> bool { + pub fn insert(&mut self, key: $sized_t, value: V::Value<'_>) -> bool { V::insert(self.raw, key, value); true } } - )* } + )* + } } } -impl_scalar_maps!(i32, u32, bool, u64, i64); +generate_map_with_key_ops_traits!( + i32, i32; + u32, u32; + i64, i64; + u64, u64; + bool, bool; + ProtoStr, &ProtoStr; +); + +macro_rules! impl_scalar_map_with_key_op_for_scalar_values { + ($key_t:ty, $sized_key_t:ty, $ffi_key_t:ty, $to_ffi_key:expr, $trait:ident for $($t:ty, $sized_t:ty, $ffi_t:ty, $to_ffi_value:expr, $from_ffi_value:expr, $zero_val:literal;)*) => { + paste! { $( + extern "C" { + fn [< __pb_rust_Map_ $key_t _ $t _new >]() -> RawMap; + fn [< __pb_rust_Map_ $key_t _ $t _clear >](m: RawMap); + fn [< __pb_rust_Map_ $key_t _ $t _size >](m: RawMap) -> usize; + fn [< __pb_rust_Map_ $key_t _ $t _insert >](m: RawMap, key: $ffi_key_t, value: $ffi_t); + fn [< __pb_rust_Map_ $key_t _ $t _get >](m: RawMap, key: $ffi_key_t, value: *mut $ffi_t) -> bool; + fn [< __pb_rust_Map_ $key_t _ $t _remove >](m: RawMap, key: $ffi_key_t, value: *mut $ffi_t) -> bool; + } + impl $trait for $t { + type Value<'a> = $sized_t; + + fn new_map() -> RawMap { + unsafe { [< __pb_rust_Map_ $key_t _ $t _new >]() } + } + + fn clear(m: RawMap) { + unsafe { [< __pb_rust_Map_ $key_t _ $t _clear >](m) } + } + + fn size(m: RawMap) -> usize { + unsafe { [< __pb_rust_Map_ $key_t _ $t _size >](m) } + } + + fn insert(m: RawMap, key: $sized_key_t, value: Self::Value<'_>) -> bool { + let ffi_key = $to_ffi_key(key); + let ffi_value = $to_ffi_value(value); + unsafe { [< __pb_rust_Map_ $key_t _ $t _insert >](m, ffi_key, ffi_value) } + true + } + + fn get<'a>(m: RawMap, key: $sized_key_t) -> Option<Self::Value<'a>> { + let ffi_key = $to_ffi_key(key); + let mut ffi_value = $to_ffi_value($zero_val); + let found = unsafe { [< __pb_rust_Map_ $key_t _ $t _get >](m, ffi_key, &mut ffi_value) }; + if !found { + return None; + } + Some($from_ffi_value(ffi_value)) + } + + fn remove<'a>(m: RawMap, key: $sized_key_t) -> bool { + let ffi_key = $to_ffi_key(key); + let mut ffi_value = $to_ffi_value($zero_val); + unsafe { [< __pb_rust_Map_ $key_t _ $t _remove >](m, ffi_key, &mut ffi_value) } + } + } + )* } + } +} + +fn str_to_ptrlen<'a>(val: impl Into<&'a ProtoStr>) -> PtrAndLen { + val.into().as_bytes().into() +} + +fn ptrlen_to_str<'a>(val: PtrAndLen) -> &'a ProtoStr { + unsafe { ProtoStr::from_utf8_unchecked(val.as_ref()) } +} + +macro_rules! impl_map_with_key_ops_for_scalar_values { + ($($t:ty, $t_sized:ty, $ffi_t:ty, $to_ffi_key:expr;)*) => { + paste! { + $( + impl_scalar_map_with_key_op_for_scalar_values!($t, $t_sized, $ffi_t, $to_ffi_key, [< MapWith $t:camel KeyOps >] for + f32, f32, f32, identity, identity, 0f32; + f64, f64, f64, identity, identity, 0f64; + i32, i32, i32, identity, identity, 0i32; + u32, u32, u32, identity, identity, 0u32; + i64, i64, i64, identity, identity, 0i64; + u64, u64, u64, identity, identity, 0u64; + bool, bool, bool, identity, identity, false; + ProtoStr, &'a ProtoStr, PtrAndLen, str_to_ptrlen, ptrlen_to_str, ""; + ); + )* + } + } +} + +impl_map_with_key_ops_for_scalar_values!( + i32, i32, i32, identity; + u32, u32, u32, identity; + i64, i64, i64, identity; + u64, u64, u64, identity; + bool, bool, bool, identity; + ProtoStr, &ProtoStr, PtrAndLen, str_to_ptrlen; +); + +#[cfg(test)] +pub(crate) fn new_map_i32_i64() -> MapInner<'static, i32, i64> { + Default::default() +} #[cfg(test)] -pub(crate) fn new_map_inner() -> MapInner<'static, i32, i64> { +pub(crate) fn new_map_str_str() -> MapInner<'static, ProtoStr, ProtoStr> { Default::default() } @@ -505,9 +550,9 @@ mod tests { assert_that!(map.get(3), eq(None)); assert_that!(map.size(), eq(1)); - assert_that!(map.remove(1), eq(Some(2))); + assert_that!(map.remove(1), eq(true)); assert_that!(map.size(), eq(0)); - assert_that!(map.remove(1), eq(None)); + assert_that!(map.remove(1), eq(false)); assert_that!(map.insert(4, 5), eq(true)); assert_that!(map.insert(6, 7), eq(true)); @@ -525,13 +570,61 @@ mod tests { assert_that!(map.get(3), eq(None)); assert_that!(map.size(), eq(1)); - assert_that!(map.remove(1), eq(Some(2.5))); + assert_that!(map.remove(1), eq(true)); assert_that!(map.size(), eq(0)); - assert_that!(map.remove(1), eq(None)); + assert_that!(map.remove(1), eq(false)); assert_that!(map.insert(4, 5.1), eq(true)); assert_that!(map.insert(6, 7.2), eq(true)); map.clear(); assert_that!(map.size(), eq(0)); } + + #[test] + fn str_str_map() { + let mut map = MapInner::<'_, ProtoStr, ProtoStr>::default(); + assert_that!(map.size(), eq(0)); + + map.insert("fizz".into(), "buzz".into()); + assert_that!(map.size(), eq(1)); + assert_that!(map.remove("fizz".into()), eq(true)); + map.clear(); + assert_that!(map.size(), eq(0)); + } + + #[test] + fn u64_str_map() { + let mut map = MapInner::<'_, u64, ProtoStr>::default(); + assert_that!(map.size(), eq(0)); + + map.insert(1, "fizz".into()); + map.insert(2, "buzz".into()); + assert_that!(map.size(), eq(2)); + assert_that!(map.remove(1), eq(true)); + assert_that!(map.get(1), eq(None)); + map.clear(); + assert_that!(map.size(), eq(0)); + } + + #[test] + fn test_all_maps_can_be_constructed() { + macro_rules! gen_proto_values { + ($key_t:ty, $($value_t:ty),*) => { + $( + let map = MapInner::<'_, $key_t, $value_t>::default(); + assert_that!(map.size(), eq(0)); + )* + } + } + + macro_rules! gen_proto_keys { + ($($key_t:ty),*) => { + $( + gen_proto_values!($key_t, f32, f64, i32, u32, i64, bool, ProtoStr); + )* + } + } + + gen_proto_keys!(i32, u32, i64, u64, bool, ProtoStr); + } } diff --git a/rust/cpp_kernel/BUILD b/rust/cpp_kernel/BUILD index d10f9e7db8..e9baa3a467 100644 --- a/rust/cpp_kernel/BUILD +++ b/rust/cpp_kernel/BUILD @@ -12,6 +12,7 @@ cc_library( ], deps = [ ":rust_alloc_for_cpp_api", # buildcleaner: keep + "@com_google_absl//absl/strings:string_view", "//:protobuf_nowkt", ], ) diff --git a/rust/cpp_kernel/cpp_api.cc b/rust/cpp_kernel/cpp_api.cc index 97f2c3cdd0..05aec1f0ae 100644 --- a/rust/cpp_kernel/cpp_api.cc +++ b/rust/cpp_kernel/cpp_api.cc @@ -1,3 +1,8 @@ +#include "rust/cpp_kernel/cpp_api.h" + +#include <cstdint> +#include <string> + #include "google/protobuf/map.h" #include "google/protobuf/repeated_field.h" @@ -38,59 +43,74 @@ expose_repeated_field_methods(int64_t, i64); #undef expose_repeated_field_methods -#define expose_scalar_map_methods(key_ty, rust_key_ty, value_ty, \ - rust_value_ty) \ - google::protobuf::Map<key_ty, value_ty>* \ - __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_new() { \ - return new google::protobuf::Map<key_ty, value_ty>(); \ - } \ - void __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_clear( \ - google::protobuf::Map<key_ty, value_ty>* m) { \ - m->clear(); \ - } \ - size_t __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_size( \ - google::protobuf::Map<key_ty, value_ty>* m) { \ - return m->size(); \ - } \ - void __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_insert( \ - google::protobuf::Map<key_ty, value_ty>* m, key_ty key, value_ty val) { \ - (*m)[key] = val; \ - } \ - bool __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_get( \ - google::protobuf::Map<key_ty, value_ty>* m, key_ty key, value_ty* value) { \ - auto it = m->find(key); \ - if (it == m->end()) { \ - return false; \ - } \ - *value = it->second; \ - return true; \ - } \ - bool __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_remove( \ - google::protobuf::Map<key_ty, value_ty>* m, key_ty key, value_ty* value) { \ - auto it = m->find(key); \ - if (it == m->end()) { \ - return false; \ - } else { \ - *value = it->second; \ - m->erase(it); \ - return true; \ - } \ +#define expose_scalar_map_methods(key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, \ + value_ty, rust_value_ty, ffi_value_ty, \ + to_cpp_value, to_ffi_value) \ + google::protobuf::Map<key_ty, value_ty>* \ + __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_new() { \ + return new google::protobuf::Map<key_ty, value_ty>(); \ + } \ + void __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_clear( \ + google::protobuf::Map<key_ty, value_ty>* m) { \ + m->clear(); \ + } \ + size_t __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_size( \ + google::protobuf::Map<key_ty, value_ty>* m) { \ + return m->size(); \ + } \ + void __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_insert( \ + google::protobuf::Map<key_ty, value_ty>* m, ffi_key_ty key, ffi_value_ty value) { \ + auto cpp_key = to_cpp_key; \ + auto cpp_value = to_cpp_value; \ + (*m)[cpp_key] = cpp_value; \ + } \ + bool __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_get( \ + google::protobuf::Map<key_ty, value_ty>* 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; \ + } \ + bool __pb_rust_Map_##rust_key_ty##_##rust_value_ty##_remove( \ + google::protobuf::Map<key_ty, value_ty>* 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 expose_scalar_map_methods_for_key_type(key_ty, rust_key_ty) \ - expose_scalar_map_methods(key_ty, rust_key_ty, int32_t, i32); \ - expose_scalar_map_methods(key_ty, rust_key_ty, uint32_t, u32); \ - expose_scalar_map_methods(key_ty, rust_key_ty, float, f32); \ - expose_scalar_map_methods(key_ty, rust_key_ty, double, f64); \ - expose_scalar_map_methods(key_ty, rust_key_ty, bool, bool); \ - expose_scalar_map_methods(key_ty, rust_key_ty, uint64_t, u64); \ - expose_scalar_map_methods(key_ty, rust_key_ty, int64_t, i64); +#define expose_scalar_map_methods_for_key_type(key_ty, rust_key_ty, \ + ffi_key_ty, to_cpp_key) \ + expose_scalar_map_methods(key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, \ + int32_t, i32, int32_t, value, cpp_value); \ + expose_scalar_map_methods(key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, \ + uint32_t, u32, uint32_t, value, cpp_value); \ + expose_scalar_map_methods(key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, \ + float, f32, float, value, cpp_value); \ + expose_scalar_map_methods(key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, \ + double, f64, double, value, cpp_value); \ + expose_scalar_map_methods(key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, bool, \ + bool, bool, value, cpp_value); \ + expose_scalar_map_methods(key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, \ + uint64_t, u64, uint64_t, value, cpp_value); \ + expose_scalar_map_methods(key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, \ + int64_t, i64, int64_t, value, cpp_value); \ + expose_scalar_map_methods( \ + key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, std::string, ProtoStr, \ + google::protobuf::rust_internal::PtrAndLen, std::string(value.ptr, value.len), \ + google::protobuf::rust_internal::PtrAndLen(cpp_value.data(), cpp_value.size())); -expose_scalar_map_methods_for_key_type(int32_t, i32); -expose_scalar_map_methods_for_key_type(uint32_t, u32); -expose_scalar_map_methods_for_key_type(bool, bool); -expose_scalar_map_methods_for_key_type(uint64_t, u64); -expose_scalar_map_methods_for_key_type(int64_t, i64); +expose_scalar_map_methods_for_key_type(int32_t, i32, int32_t, key); +expose_scalar_map_methods_for_key_type(uint32_t, u32, uint32_t, key); +expose_scalar_map_methods_for_key_type(bool, bool, bool, key); +expose_scalar_map_methods_for_key_type(uint64_t, u64, uint64_t, key); +expose_scalar_map_methods_for_key_type(int64_t, i64, int64_t, key); +expose_scalar_map_methods_for_key_type(std::string, ProtoStr, + google::protobuf::rust_internal::PtrAndLen, + std::string(key.ptr, key.len)); #undef expose_scalar_map_methods #undef expose_map_methods diff --git a/rust/map.rs b/rust/map.rs index c4631b0ce9..9f6a11ce35 100644 --- a/rust/map.rs +++ b/rust/map.rs @@ -6,17 +6,16 @@ // https://developers.google.com/open-source/licenses/bsd use crate::{ - Mut, MutProxy, Proxied, SettableValue, View, ViewProxy, + Mut, MutProxy, ProtoStr, Proxied, SettableValue, View, ViewProxy, __internal::Private, __runtime::{ - MapInner, MapWithBoolKeyOps, MapWithI32KeyOps, MapWithI64KeyOps, MapWithU32KeyOps, - MapWithU64KeyOps, + MapInner, MapWithBoolKeyOps, MapWithI32KeyOps, MapWithI64KeyOps, MapWithProtoStrKeyOps, + MapWithU32KeyOps, MapWithU64KeyOps, }, }; use paste::paste; use std::marker::PhantomData; -#[derive(Clone, Copy)] #[repr(transparent)] pub struct MapView<'a, K: ?Sized, V: ?Sized> { inner: MapInner<'a, K, V>, @@ -28,6 +27,13 @@ impl<'a, K: ?Sized, V: ?Sized> MapView<'a, K, V> { } } +impl<'a, K: ?Sized, V: ?Sized> Copy for MapView<'a, K, V> {} +impl<'a, K: ?Sized, V: ?Sized> Clone for MapView<'a, K, V> { + fn clone(&self) -> Self { + *self + } +} + unsafe impl<'a, K: ?Sized, V: ?Sized> Sync for MapView<'a, K, V> {} unsafe impl<'a, K: ?Sized, V: ?Sized> Send for MapView<'a, K, V> {} @@ -40,7 +46,6 @@ impl<'a, K: ?Sized, V: ?Sized> std::fmt::Debug for MapView<'a, K, V> { } } -#[derive(Debug)] #[repr(transparent)] pub struct MapMut<'a, K: ?Sized, V: ?Sized> { inner: MapInner<'a, K, V>, @@ -54,6 +59,15 @@ impl<'a, K: ?Sized, V: ?Sized> MapMut<'a, K, V> { unsafe impl<'a, K: ?Sized, V: ?Sized> Sync for MapMut<'a, K, V> {} +impl<'a, K: ?Sized, V: ?Sized> std::fmt::Debug for MapMut<'a, K, V> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("MapMut") + .field(&std::any::type_name::<K>()) + .field(&std::any::type_name::<V>()) + .finish() + } +} + impl<'a, K: ?Sized, V: ?Sized> std::ops::Deref for MapMut<'a, K, V> { type Target = MapView<'a, K, V>; fn deref(&self) -> &Self::Target { @@ -69,64 +83,79 @@ impl<'a, K: ?Sized, V: ?Sized> std::ops::Deref for MapMut<'a, K, V> { // `MapView` (`View<'_, Map>>) and `MapMut` (Mut<'_, Map>). pub struct Map<K: ?Sized, V: ?Sized>(PhantomData<K>, PhantomData<V>); -macro_rules! impl_scalar_map_keys { +macro_rules! impl_proxied_for_map_keys { ($(key_type $t:ty;)*) => { - paste! { $( - impl<V: [< MapWith $t:camel KeyOps >]> Proxied for Map<$t, V>{ - type View<'a> = MapView<'a, $t, V> where V: 'a; - type Mut<'a> = MapMut<'a, $t, V> where V: 'a; + paste! { $( + impl<V: [< MapWith $t:camel KeyOps >] + Proxied + ?Sized> Proxied for Map<$t, V>{ + type View<'a> = MapView<'a, $t, V> where V: 'a; + type Mut<'a> = MapMut<'a, $t, V> where V: 'a; + } + + impl<'a, V: [< MapWith $t:camel KeyOps >] + Proxied + ?Sized + 'a> SettableValue<Map<$t, V>> for MapView<'a, $t, V> { + fn set_on<'b>(self, _private: Private, mut mutator: Mut<'b, Map<$t, V>>) + where + Map<$t, V>: 'b { + mutator.copy_from(self); } + } - impl<'a, V: [< MapWith $t:camel KeyOps >]> SettableValue<Map<$t, V>> for MapView<'a, $t, V> { - fn set_on<'b>(self, _private: Private, mut mutator: Mut<'b, Map<$t, V>>) - where - Map<$t, V>: 'b { - mutator.copy_from(self); - } - } + impl<'a, V: [< MapWith $t:camel KeyOps >] + Proxied + ?Sized + 'a> ViewProxy<'a> for MapView<'a, $t, V> { + type Proxied = Map<$t, V>; - impl<'a, V: [< MapWith $t:camel KeyOps >]> ViewProxy<'a> for MapView<'a, $t, V> { - type Proxied = Map<$t, V>; - - fn as_view(&self) -> View<'_, Self::Proxied> { - *self - } + fn as_view(&self) -> View<'_, Self::Proxied> { + *self + } - fn into_view<'shorter>(self) -> View<'shorter, Self::Proxied> - where 'a: 'shorter, - { - MapView { inner: self.inner } - } + fn into_view<'shorter>(self) -> View<'shorter, Self::Proxied> + where 'a: 'shorter, + { + MapView { inner: self.inner } } + } - impl<'a, V: [< MapWith $t:camel KeyOps >]> ViewProxy<'a> for MapMut<'a, $t, V> { - type Proxied = Map<$t, V>; + impl<'a, V: [< MapWith $t:camel KeyOps >] + Proxied + ?Sized + 'a> ViewProxy<'a> for MapMut<'a, $t, V> { + type Proxied = Map<$t, V>; - fn as_view(&self) -> View<'_, Self::Proxied> { - **self - } + fn as_view(&self) -> View<'_, Self::Proxied> { + **self + } - fn into_view<'shorter>(self) -> View<'shorter, Self::Proxied> - where 'a: 'shorter, - { - *self.into_mut::<'shorter>() - } + fn into_view<'shorter>(self) -> View<'shorter, Self::Proxied> + where 'a: 'shorter, + { + *self.into_mut::<'shorter>() } + } - impl<'a, V: [< MapWith $t:camel KeyOps >]> MutProxy<'a> for MapMut<'a, $t, V> { - fn as_mut(&mut self) -> Mut<'_, Self::Proxied> { - MapMut { inner: self.inner } - } + impl<'a, V: [< MapWith $t:camel KeyOps >] + Proxied + ?Sized + 'a> MutProxy<'a> for MapMut<'a, $t, V> { + fn as_mut(&mut self) -> Mut<'_, Self::Proxied> { + MapMut { inner: self.inner } + } - fn into_mut<'shorter>(self) -> Mut<'shorter, Self::Proxied> - where 'a: 'shorter, - { - MapMut { inner: self.inner } - } + fn into_mut<'shorter>(self) -> Mut<'shorter, Self::Proxied> + where 'a: 'shorter, + { + MapMut { inner: self.inner } } + } + )* } + } +} - impl<'a, V: [< MapWith $t:camel KeyOps >]> MapView<'a, $t, V> { - pub fn get(&self, key: $t) -> Option<V> { +impl_proxied_for_map_keys!( + key_type i32; + key_type u32; + key_type i64; + key_type u64; + key_type bool; + key_type ProtoStr; +); + +macro_rules! impl_scalar_map_keys { + ($(key_type $t:ty;)*) => { + paste! { $( + impl<'a, V: [< MapWith $t:camel KeyOps >] + Proxied + ?Sized + 'a> MapView<'a, $t, V> { + pub fn get<'b>(&self, key: $t) -> Option<V::Value<'b>> { self.inner.get(key) } @@ -139,12 +168,12 @@ macro_rules! impl_scalar_map_keys { } } - impl<'a, V: [< MapWith $t:camel KeyOps >]> MapMut<'a, $t, V> { - pub fn insert(&mut self, key: $t, value: V) -> bool { + impl<'a, V: [< MapWith $t:camel KeyOps >] + Proxied + ?Sized + 'a> MapMut<'a, $t, V> { + pub fn insert(&mut self, key: $t, value: V::Value<'_>) -> bool { self.inner.insert(key, value) } - pub fn remove(&mut self, key: $t) -> Option<V> { + pub fn remove<'b>(&mut self, key: $t) -> bool { self.inner.remove(key) } @@ -168,15 +197,47 @@ impl_scalar_map_keys!( key_type bool; ); +impl<'a, V: MapWithProtoStrKeyOps + Proxied + ?Sized + 'a> MapView<'a, ProtoStr, V> { + pub fn get(&self, key: impl Into<&'a ProtoStr>) -> Option<V::Value<'_>> { + self.inner.get(key.into()) + } + + pub fn len(&self) -> usize { + self.inner.size() + } + + pub fn is_empty(&self) -> bool { + self.len() == 0 + } +} + +impl<'a, V: MapWithProtoStrKeyOps + Proxied + ?Sized + 'a> MapMut<'a, ProtoStr, V> { + pub fn insert(&mut self, key: impl Into<&'a ProtoStr>, value: V::Value<'_>) -> bool { + self.inner.insert(key.into(), value) + } + + pub fn remove(&mut self, key: impl Into<&'a ProtoStr>) -> bool { + self.inner.remove(key.into()) + } + + pub fn clear(&mut self) { + self.inner.clear() + } + + pub fn copy_from(&mut self, _src: MapView<'_, ProtoStr, V>) { + todo!("implement b/28530933"); + } +} + #[cfg(test)] mod tests { use super::*; - use crate::__runtime::new_map_inner; + use crate::__runtime::{new_map_i32_i64, new_map_str_str}; use googletest::prelude::*; #[test] - fn test_proxied() { - let mut map_mut = MapMut::from_inner(Private, new_map_inner()); + fn test_proxied_scalar() { + let mut map_mut = MapMut::from_inner(Private, new_map_i32_i64()); map_mut.insert(1, 2); let map_view_1 = map_mut.as_view(); assert_that!(map_view_1.len(), eq(1)); @@ -197,9 +258,33 @@ mod tests { assert_that!(map_view_4.is_empty(), eq(false)); } + #[test] + fn test_proxied_str() { + let mut map_mut = MapMut::from_inner(Private, new_map_str_str()); + map_mut.insert("a", "b".into()); + + let map_view_1 = map_mut.as_view(); + assert_that!(map_view_1.len(), eq(1)); + assert_that!(map_view_1.get("a").unwrap(), eq("b")); + + map_mut.insert("c", "d".into()); + + let map_view_2 = map_mut.into_view(); + assert_that!(map_view_2.len(), eq(2)); + assert_that!(map_view_2.get("c").unwrap(), eq("d")); + + { + let map_view_3 = map_view_2.as_view(); + assert_that!(map_view_3.is_empty(), eq(false)); + } + + let map_view_4 = map_view_2.into_view(); + assert_that!(map_view_4.is_empty(), eq(false)); + } + #[test] fn test_dbg() { - let map_view = MapView::from_inner(Private, new_map_inner()); + let map_view = MapView::from_inner(Private, new_map_i32_i64()); assert_that!(format!("{:?}", map_view), eq("MapView(\"i32\", \"i64\")")); } } diff --git a/rust/shared.rs b/rust/shared.rs index 26f0629da1..1ab53cb97c 100644 --- a/rust/shared.rs +++ b/rust/shared.rs @@ -17,7 +17,7 @@ use std::fmt; /// These are the items protobuf users can access directly. #[doc(hidden)] pub mod __public { - pub use crate::map::{MapMut, MapView}; + pub use crate::map::{Map, MapMut, MapView}; pub use crate::optional::{AbsentField, FieldEntry, Optional, PresentField}; pub use crate::primitive::{PrimitiveMut, SingularPrimitiveMut}; pub use crate::proxied::{ diff --git a/rust/test/shared/accessors_map_test.rs b/rust/test/shared/accessors_map_test.rs index a401d946a0..ecdd35a20a 100644 --- a/rust/test/shared/accessors_map_test.rs +++ b/rust/test/shared/accessors_map_test.rs @@ -41,3 +41,15 @@ generate_map_primitives_tests!( (i32, f64, int32, double), (bool, bool, bool, bool) ); + +#[test] +fn test_string_maps() { + let mut msg = TestMap::new(); + msg.map_string_string_mut().insert("hello", "world".into()); + msg.map_string_string_mut().insert("fizz", "buzz".into()); + assert_that!(msg.map_string_string().len(), eq(2)); + assert_that!(msg.map_string_string().get("fizz").unwrap(), eq("buzz")); + assert_that!(msg.map_string_string().get("not found"), eq(None)); + msg.map_string_string_mut().clear(); + assert_that!(msg.map_string_string().len(), eq(0)); +} diff --git a/rust/upb.rs b/rust/upb.rs index f123e0f6ac..50b67dfd39 100644 --- a/rust/upb.rs +++ b/rust/upb.rs @@ -7,6 +7,7 @@ //! UPB FFI wrapper code for use by Rust Protobuf. +use crate::ProtoStr; use crate::__internal::{Private, PtrAndLen, RawArena, RawMap, RawMessage, RawRepeatedField}; use core::fmt::Debug; use paste::paste; @@ -502,9 +503,6 @@ pub struct MapInner<'msg, K: ?Sized, V: ?Sized> { pub _phantom_value: PhantomData<&'msg mut V>, } -// These use manual impls instead of derives to avoid unnecessary bounds on `K` -// and `V`. This problem is referred to as "perfect derive". -// https://smallcultfollowing.com/babysteps/blog/2022/04/12/implied-bounds-and-perfect-derive/ impl<'msg, K: ?Sized, V: ?Sized> Copy for MapInner<'msg, K, V> {} impl<'msg, K: ?Sized, V: ?Sized> Clone for MapInner<'msg, K, V> { fn clone(&self) -> MapInner<'msg, K, V> { @@ -512,86 +510,27 @@ impl<'msg, K: ?Sized, V: ?Sized> Clone for MapInner<'msg, K, V> { } } -macro_rules! impl_scalar_map_for_key_type { - ($key_t:ty, $key_ufield:ident, $key_upb_tag:expr, $trait:ident for $($t:ty, $ufield:ident, $upb_tag:expr, $zero_val:literal;)*) => { - paste! { $( - impl $trait for $t { - fn new_map(a: RawArena) -> RawMap { - unsafe { upb_Map_New(a, $key_upb_tag, $upb_tag) } - } - - fn clear(m: RawMap) { - unsafe { upb_Map_Clear(m) } - } - - fn size(m: RawMap) -> usize { - unsafe { upb_Map_Size(m) } - } +macro_rules! generate_map_key_ops_traits { + ($($t:ty, $sized_t:ty;)*) => { + paste! { + $( + pub trait [< MapWith $t:camel KeyOps >] { + type Value<'a>: Sized; - fn insert(m: RawMap, a: RawArena, key: $key_t, value: $t) -> bool { - unsafe { - upb_Map_Set( - m, - upb_MessageValue { $key_ufield: key }, - upb_MessageValue { $ufield: value}, - a - ) - } - } - - fn get(m: RawMap, key: $key_t) -> Option<$t> { - let mut val = upb_MessageValue { $ufield: $zero_val }; - let found = unsafe { - upb_Map_Get(m, upb_MessageValue { $key_ufield: key }, &mut val) - }; - if !found { - return None; + fn new_map(a: RawArena) -> RawMap; + fn clear(m: RawMap) { + unsafe { upb_Map_Clear(m) } } - Some(unsafe { val.$ufield }) - } - - fn remove(m: RawMap, key: $key_t) -> Option<$t> { - let mut val = upb_MessageValue { $ufield: $zero_val }; - let removed = unsafe { - upb_Map_Delete(m, upb_MessageValue { $key_ufield: key }, &mut val) - }; - if !removed { - return None; + fn size(m: RawMap) -> usize { + unsafe { upb_Map_Size(m) } } - Some(unsafe { val.$ufield }) - } - } - )* } - } -} - -macro_rules! impl_scalar_map_for_key_types { - ($($t:ty, $ufield:ident, $upb_tag:expr;)*) => { - paste! { $( - pub trait [< MapWith $t:camel KeyOps >] : Sync + Send + Copy + Clone + Debug { - fn new_map(a: RawArena) -> RawMap; - fn clear(m: RawMap); - fn size(m: RawMap) -> usize; - fn insert(m: RawMap, a: RawArena, key: $t, value: Self) -> bool; - fn get(m: RawMap, key: $t) -> Option<Self> - where - Self: Sized; - fn remove(m: RawMap, key: $t) -> Option<Self> - where - Self: Sized; + fn insert(m: RawMap, a: RawArena, key: $sized_t, value: Self::Value<'_>) -> bool; + fn get<'a>(m: RawMap, key: $sized_t) -> Option<Self::Value<'a>>; + fn remove<'a>(m: RawMap, key: $sized_t) -> bool; } - impl_scalar_map_for_key_type!($t, $ufield, $upb_tag, [< MapWith $t:camel KeyOps >] for - f32, float_val, UpbCType::Float, 0f32; - f64, double_val, UpbCType::Double, 0f64; - i32, int32_val, UpbCType::Int32, 0i32; - u32, uint32_val, UpbCType::UInt32, 0u32; - i64, int64_val, UpbCType::Int64, 0i64; - u64, uint64_val, UpbCType::UInt64, 0u64; - bool, bool_val, UpbCType::Bool, false; - ); + impl<'msg, V: [< MapWith $t:camel KeyOps >] + ?Sized> MapInner<'msg, $t, V> { - impl<'msg, V: [< MapWith $t:camel KeyOps >]> MapInner<'msg, $t, V> { pub fn new(arena: &'msg mut Arena) -> Self { MapInner { raw: V::new_map(arena.raw()), @@ -609,28 +548,121 @@ macro_rules! impl_scalar_map_for_key_types { V::clear(self.raw) } - pub fn get(&self, key: $t) -> Option<V> { + pub fn get<'a>(&self, key: $sized_t) -> Option<V::Value<'a>> { V::get(self.raw, key) } - pub fn remove(&mut self, key: $t) -> Option<V> { + pub fn remove<'a>(&mut self, key: $sized_t) -> bool { V::remove(self.raw, key) } - pub fn insert(&mut self, key: $t, value: V) -> bool { + pub fn insert(&mut self, key: $sized_t, value: V::Value<'_>) -> bool { V::insert(self.raw, self.arena.raw(), key, value) } } - )* } + )* + } } } -impl_scalar_map_for_key_types!( - i32, int32_val, UpbCType::Int32; - u32, uint32_val, UpbCType::UInt32; - i64, int64_val, UpbCType::Int64; - u64, uint64_val, UpbCType::UInt64; - bool, bool_val, UpbCType::Bool; +generate_map_key_ops_traits!( + i32, i32; + u32, u32; + i64, i64; + u64, u64; + bool, bool; + ProtoStr, &ProtoStr; +); + +macro_rules! impl_scalar_map_key_op_for_scalar_values { + ($key_t:ty, $key_msg_val:expr, $key_upb_tag:expr, $trait:ident for $($t:ty, $sized_t:ty, $msg_val:expr, $from_msg_val:expr, $upb_tag:expr, $zero_val:literal;)*) => { + $( + impl $trait for $t { + type Value<'a> = $sized_t; + + fn new_map(a: RawArena) -> RawMap { + unsafe { upb_Map_New(a, $key_upb_tag, $upb_tag) } + } + + fn insert(m: RawMap, a: RawArena, key: $key_t, value: Self::Value<'_>) -> bool { + unsafe { + upb_Map_Set( + m, + $key_msg_val(key), + $msg_val(value), + a + ) + } + } + + fn get<'a>(m: RawMap, key: $key_t) -> Option<Self::Value<'a>> { + let mut val = $msg_val($zero_val); + let found = unsafe { + upb_Map_Get(m, $key_msg_val(key), &mut val) + }; + if !found { + return None; + } + Some($from_msg_val(val)) + } + + fn remove<'a>(m: RawMap, key: $key_t) -> bool { + let mut val = $msg_val($zero_val); + unsafe { + upb_Map_Delete(m, $key_msg_val(key), &mut val) + } + } + } + )* + } +} + +macro_rules! scalar_to_msg { + ($ufield:ident) => { + |val| upb_MessageValue { $ufield: val } + }; +} + +macro_rules! scalar_from_msg { + ($ufield:ident) => { + |msg: upb_MessageValue| unsafe { msg.$ufield } + }; +} + +fn str_to_msg<'a>(val: impl Into<&'a ProtoStr>) -> upb_MessageValue { + upb_MessageValue { str_val: val.into().as_bytes().into() } +} + +fn msg_to_str<'a>(msg: upb_MessageValue) -> &'a ProtoStr { + unsafe { ProtoStr::from_utf8_unchecked(msg.str_val.as_ref()) } +} + +macro_rules! impl_map_key_ops_for_scalar_values { + ($($t:ty, $t_sized:ty, $key_msg_val:expr, $upb_tag:expr;)*) => { + paste! { + $( + impl_scalar_map_key_op_for_scalar_values!($t_sized, $key_msg_val, $upb_tag, [< MapWith $t:camel KeyOps >] for + f32, f32, scalar_to_msg!(float_val), scalar_from_msg!(float_val), UpbCType::Float, 0f32; + f64, f64, scalar_to_msg!(double_val), scalar_from_msg!(double_val), UpbCType::Double, 0f64; + i32, i32, scalar_to_msg!(int32_val), scalar_from_msg!(int32_val), UpbCType::Int32, 0i32; + u32, u32, scalar_to_msg!(uint32_val), scalar_from_msg!(uint32_val), UpbCType::UInt32, 0u32; + i64, i64, scalar_to_msg!(int64_val), scalar_from_msg!(int64_val), UpbCType::Int64, 0i64; + u64, u64, scalar_to_msg!(uint64_val), scalar_from_msg!(uint64_val), UpbCType::UInt64, 0u64; + bool, bool, scalar_to_msg!(bool_val), scalar_from_msg!(bool_val), UpbCType::Bool, false; + ProtoStr, &'a ProtoStr, str_to_msg, msg_to_str, UpbCType::String, ""; + ); + )* + } + } +} + +impl_map_key_ops_for_scalar_values!( + i32, i32, scalar_to_msg!(int32_val), UpbCType::Int32; + u32, u32, scalar_to_msg!(uint32_val), UpbCType::UInt32; + i64, i64, scalar_to_msg!(int64_val), UpbCType::Int64; + u64, u64, scalar_to_msg!(uint64_val), UpbCType::UInt64; + bool, bool, scalar_to_msg!(bool_val), UpbCType::Bool; + ProtoStr, &ProtoStr, |val: &ProtoStr| upb_MessageValue { str_val: val.as_bytes().into() }, UpbCType::String; ); extern "C" { @@ -652,11 +684,17 @@ extern "C" { } #[cfg(test)] -pub(crate) fn new_map_inner() -> MapInner<'static, i32, i64> { +pub(crate) fn new_map_i32_i64() -> MapInner<'static, i32, i64> { let arena = Box::leak::<'static>(Box::new(Arena::new())); MapInner::<'static, i32, i64>::new(arena) } +#[cfg(test)] +pub(crate) fn new_map_str_str() -> MapInner<'static, ProtoStr, ProtoStr> { + let arena = Box::leak::<'static>(Box::new(Arena::new())); + MapInner::<'static, ProtoStr, ProtoStr>::new(arena) +} + #[cfg(test)] mod tests { use super::*; @@ -726,9 +764,9 @@ mod tests { assert_that!(map.get(3), eq(None)); assert_that!(map.size(), eq(1)); - assert_that!(map.remove(1), eq(Some(2))); + assert_that!(map.remove(1), eq(true)); assert_that!(map.size(), eq(0)); - assert_that!(map.remove(1), eq(None)); + assert_that!(map.remove(1), eq(false)); assert_that!(map.insert(4, 5), eq(true)); assert_that!(map.insert(6, 7), eq(true)); @@ -747,13 +785,63 @@ mod tests { assert_that!(map.get(3), eq(None)); assert_that!(map.size(), eq(1)); - assert_that!(map.remove(1), eq(Some(2.5))); + assert_that!(map.remove(1), eq(true)); assert_that!(map.size(), eq(0)); - assert_that!(map.remove(1), eq(None)); + assert_that!(map.remove(1), eq(false)); assert_that!(map.insert(4, 5.1), eq(true)); assert_that!(map.insert(6, 7.2), eq(true)); map.clear(); assert_that!(map.size(), eq(0)); } + + #[test] + fn str_str_map() { + let mut arena = Arena::new(); + let mut map = MapInner::<'_, ProtoStr, ProtoStr>::new(&mut arena); + assert_that!(map.size(), eq(0)); + + map.insert("fizz".into(), "buzz".into()); + assert_that!(map.size(), eq(1)); + assert_that!(map.remove("fizz".into()), eq(true)); + map.clear(); + assert_that!(map.size(), eq(0)); + } + + #[test] + fn u64_str_map() { + let mut arena = Arena::new(); + let mut map = MapInner::<'_, u64, ProtoStr>::new(&mut arena); + assert_that!(map.size(), eq(0)); + + map.insert(1, "fizz".into()); + map.insert(2, "buzz".into()); + assert_that!(map.size(), eq(2)); + assert_that!(map.remove(1), eq(true)); + map.clear(); + assert_that!(map.size(), eq(0)); + } + + #[test] + fn test_all_maps_can_be_constructed() { + macro_rules! gen_proto_values { + ($key_t:ty, $($value_t:ty),*) => { + let mut arena = Arena::new(); + $( + let map = MapInner::<'_, $key_t, $value_t>::new(&mut arena); + assert_that!(map.size(), eq(0)); + )* + } + } + + macro_rules! gen_proto_keys { + ($($key_t:ty),*) => { + $( + gen_proto_values!($key_t, f32, f64, i32, u32, i64, bool, ProtoStr); + )* + } + } + + gen_proto_keys!(i32, u32, i64, u64, bool, ProtoStr); + } } diff --git a/src/google/protobuf/compiler/rust/accessors/accessors.cc b/src/google/protobuf/compiler/rust/accessors/accessors.cc index 2f08da5694..c25fbc207f 100644 --- a/src/google/protobuf/compiler/rust/accessors/accessors.cc +++ b/src/google/protobuf/compiler/rust/accessors/accessors.cc @@ -32,6 +32,20 @@ std::unique_ptr<AccessorGenerator> AccessorGeneratorFor( "fields with ctype not supported"); } + if (desc.is_map()) { + auto value_type = desc.message_type()->map_value()->type(); + switch (value_type) { + case FieldDescriptor::TYPE_BYTES: + case FieldDescriptor::TYPE_ENUM: + case FieldDescriptor::TYPE_MESSAGE: + return std::make_unique<UnsupportedField>( + "Maps with values of type bytes, enum and message are not " + "supported"); + default: + return std::make_unique<Map>(); + } + } + switch (desc.type()) { case FieldDescriptor::TYPE_INT32: case FieldDescriptor::TYPE_INT64: @@ -57,19 +71,6 @@ std::unique_ptr<AccessorGenerator> AccessorGeneratorFor( } return std::make_unique<SingularString>(); case FieldDescriptor::TYPE_MESSAGE: - if (desc.is_map()) { - // This switch statement will be removed as we support all map - // value types. - switch (desc.message_type()->map_value()->type()) { - case FieldDescriptor::TYPE_STRING: - case FieldDescriptor::TYPE_ENUM: - case FieldDescriptor::TYPE_MESSAGE: - return std::make_unique<UnsupportedField>( - "message types in maps are not supported"); - default: - return std::make_unique<Map>(); - } - } if (desc.is_repeated()) { return std::make_unique<UnsupportedField>("repeated msg not supported"); } diff --git a/src/google/protobuf/compiler/rust/accessors/map.cc b/src/google/protobuf/compiler/rust/accessors/map.cc index b398df17c4..0c2c1eb29d 100644 --- a/src/google/protobuf/compiler/rust/accessors/map.cc +++ b/src/google/protobuf/compiler/rust/accessors/map.cc @@ -30,7 +30,8 @@ void Map::InMsgImpl(Context<FieldDescriptor> field) const { [&] { if (field.is_upb()) { field.Emit({}, R"rs( - pub fn r#$field$(&self) -> $pb$::MapView<'_, $Key$, $Value$> { + pub fn r#$field$(&self) + -> $pb$::View<'_, $pb$::Map<$Key$, $Value$>> { let inner = unsafe { $getter_thunk$(self.inner.msg) }.map_or_else(|| unsafe {$pbr$::empty_map()}, |raw| { @@ -45,7 +46,8 @@ void Map::InMsgImpl(Context<FieldDescriptor> field) const { })rs"); } else { field.Emit({}, R"rs( - pub fn r#$field$(&self) -> $pb$::MapView<'_, $Key$, $Value$> { + pub fn r#$field$(&self) + -> $pb$::View<'_, $pb$::Map<$Key$, $Value$>> { let inner = $pbr$::MapInner { raw: unsafe { $getter_thunk$(self.inner.msg) }, _phantom_key: std::marker::PhantomData, @@ -59,9 +61,11 @@ void Map::InMsgImpl(Context<FieldDescriptor> field) const { [&] { if (field.is_upb()) { field.Emit({}, R"rs( - pub fn r#$field$_mut(&mut self) -> $pb$::MapMut<'_, $Key$, $Value$> { + pub fn r#$field$_mut(&mut self) + -> $pb$::Mut<'_, $pb$::Map<$Key$, $Value$>> { let raw = unsafe { - $getter_mut_thunk$(self.inner.msg, self.inner.arena.raw()) + $getter_mut_thunk$(self.inner.msg, + self.inner.arena.raw()) }; let inner = $pbr$::MapInner{ raw, @@ -73,7 +77,8 @@ void Map::InMsgImpl(Context<FieldDescriptor> field) const { })rs"); } else { field.Emit({}, R"rs( - pub fn r#$field$_mut(&mut self) -> $pb$::MapMut<'_, $Key$, $Value$> { + pub fn r#$field$_mut(&mut self) + -> $pb$::Mut<'_, $pb$::Map<$Key$, $Value$>> { let inner = $pbr$::MapInner { raw: unsafe { $getter_mut_thunk$(self.inner.msg) }, _phantom_key: std::marker::PhantomData,