Remove the protobuf::__runtime::Map type
Before this change the runtimes export a Map and MapInner. This change merges the Map and MapInner types into a single MapInner type, removing the Map type from the runtime (upb.rs, cpp.rs).
The motivation for this change is twofold:
1) A separate Map type is not strictly needed by the runtime. I hope this reduces some complexity.
2) After this change we can introduce a runtime-agnostic protobuf::Map type that implements Proxied.
PiperOrigin-RevId: 582978008
diff --git a/rust/cpp.rs b/rust/cpp.rs
index 11fd634..a0c319d 100644
--- a/rust/cpp.rs
+++ b/rust/cpp.rs
@@ -320,34 +320,22 @@
}
#[derive(Debug)]
-pub struct Map<'msg, K: ?Sized, V: ?Sized> {
- inner: MapInner<'msg>,
- _phantom_key: PhantomData<&'msg mut K>,
- _phantom_value: PhantomData<&'msg mut V>,
-}
-
-#[derive(Clone, Copy, Debug)]
-pub struct MapInner<'msg> {
+pub struct MapInner<'msg, K: ?Sized, V: ?Sized> {
pub raw: RawMap,
- pub _phantom: PhantomData<&'msg ()>,
+ pub _phantom_key: PhantomData<&'msg mut K>,
+ 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 Map<'msg, K, V> {}
-impl<'msg, K: ?Sized, V: ?Sized> Clone for Map<'msg, K, V> {
- fn clone(&self) -> Map<'msg, K, V> {
+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> {
*self
}
}
-impl<'msg, K: ?Sized, V: ?Sized> Map<'msg, K, V> {
- pub fn from_inner(_private: Private, inner: MapInner<'msg>) -> Self {
- Map { inner, _phantom_key: PhantomData, _phantom_value: PhantomData }
- }
-}
-
macro_rules! impl_scalar_map_values {
($kt:ty, $trait:ident for $($t:ty),*) => {
paste! { $(
@@ -419,34 +407,35 @@
$t, [< MapWith $t:camel KeyOps >] for i32, u32, f32, f64, bool, u64, i64
);
- impl<'msg, V: [< MapWith $t:camel KeyOps >]> Map<'msg, $t, V> {
- pub fn new() -> Self {
- let inner = MapInner { raw: V::new_map(), _phantom: PhantomData };
- Map {
- inner,
+ impl<'msg, V: [< MapWith $t:camel KeyOps >]> Default for MapInner<'msg, $t, V> {
+ fn default() -> Self {
+ MapInner {
+ raw: V::new_map(),
_phantom_key: PhantomData,
_phantom_value: PhantomData
}
}
+ }
+ impl<'msg, V: [< MapWith $t:camel KeyOps >]> MapInner<'msg, $t, V> {
pub fn size(&self) -> usize {
- V::size(self.inner.raw)
+ V::size(self.raw)
}
pub fn clear(&mut self) {
- V::clear(self.inner.raw)
+ V::clear(self.raw)
}
pub fn get(&self, key: $t) -> Option<V> {
- V::get(self.inner.raw, key)
+ V::get(self.raw, key)
}
pub fn remove(&mut self, key: $t) -> Option<V> {
- V::remove(self.inner.raw, key)
+ V::remove(self.raw, key)
}
pub fn insert(&mut self, key: $t, value: V) -> bool {
- V::insert(self.inner.raw, key, value);
+ V::insert(self.raw, key, value);
true
}
}
@@ -502,7 +491,7 @@
#[test]
fn i32_i32_map() {
- let mut map = Map::<'_, i32, i32>::new();
+ let mut map: MapInner<'_, i32, i32> = Default::default();
assert_that!(map.size(), eq(0));
assert_that!(map.insert(1, 2), eq(true));
@@ -522,7 +511,7 @@
#[test]
fn i64_f64_map() {
- let mut map = Map::<'_, i64, f64>::new();
+ let mut map: MapInner<'_, i64, f64> = Default::default();
assert_that!(map.size(), eq(0));
assert_that!(map.insert(1, 2.5), eq(true));
diff --git a/rust/map.rs b/rust/map.rs
index ab54698..a18ccdc 100644
--- a/rust/map.rs
+++ b/rust/map.rs
@@ -8,7 +8,7 @@
use crate::{
__internal::Private,
__runtime::{
- Map, MapInner, MapWithBoolKeyOps, MapWithI32KeyOps, MapWithI64KeyOps, MapWithU32KeyOps,
+ MapInner, MapWithBoolKeyOps, MapWithI32KeyOps, MapWithI64KeyOps, MapWithU32KeyOps,
MapWithU64KeyOps,
},
};
@@ -17,24 +17,24 @@
#[derive(Clone, Copy)]
#[repr(transparent)]
pub struct MapView<'a, K: ?Sized, V: ?Sized> {
- inner: Map<'a, K, V>,
+ inner: MapInner<'a, K, V>,
}
#[derive(Clone, Copy)]
#[repr(transparent)]
pub struct MapMut<'a, K: ?Sized, V: ?Sized> {
- inner: Map<'a, K, V>,
+ inner: MapInner<'a, K, V>,
}
impl<'a, K: ?Sized, V: ?Sized> MapView<'a, K, V> {
- pub fn from_inner(_private: Private, inner: MapInner<'a>) -> Self {
- Self { inner: Map::<'a, K, V>::from_inner(_private, inner) }
+ pub fn from_inner(_private: Private, inner: MapInner<'a, K, V>) -> Self {
+ Self { inner }
}
}
impl<'a, K: ?Sized, V: ?Sized> MapMut<'a, K, V> {
- pub fn from_inner(_private: Private, inner: MapInner<'a>) -> Self {
- Self { inner: Map::<'a, K, V>::from_inner(_private, inner) }
+ pub fn from_inner(_private: Private, inner: MapInner<'a, K, V>) -> Self {
+ Self { inner }
}
}
diff --git a/rust/upb.rs b/rust/upb.rs
index 8f713da..cb3551d 100644
--- a/rust/upb.rs
+++ b/rust/upb.rs
@@ -474,49 +474,43 @@
///
/// TODO: Split MapInner into mut and const variants to
/// enforce safety. The returned array must never be mutated.
-pub unsafe fn empty_map() -> MapInner<'static> {
- fn new_map_inner() -> MapInner<'static> {
+pub unsafe fn empty_map<K: ?Sized + 'static, V: ?Sized + 'static>() -> MapInner<'static, K, V> {
+ fn new_map_inner() -> MapInner<'static, i32, i32> {
// TODO: Consider creating empty map in C.
let arena = Box::leak::<'static>(Box::new(Arena::new()));
// Provide `i32` as a placeholder type.
- Map::<'static, i32, i32>::new(arena).inner
+ MapInner::<'static, i32, i32>::new(arena)
}
thread_local! {
- static MAP: MapInner<'static> = new_map_inner();
+ static MAP: MapInner<'static, i32, i32> = new_map_inner();
}
- MAP.with(|inner| *inner)
-}
-
-#[derive(Clone, Copy, Debug)]
-pub struct MapInner<'msg> {
- pub raw: RawMap,
- pub arena: &'msg Arena,
+ MAP.with(|inner| MapInner {
+ raw: inner.raw,
+ arena: inner.arena,
+ _phantom_key: PhantomData,
+ _phantom_value: PhantomData,
+ })
}
#[derive(Debug)]
-pub struct Map<'msg, K: ?Sized, V: ?Sized> {
- inner: MapInner<'msg>,
- _phantom_key: PhantomData<&'msg mut K>,
- _phantom_value: PhantomData<&'msg mut V>,
+pub struct MapInner<'msg, K: ?Sized, V: ?Sized> {
+ pub raw: RawMap,
+ pub arena: &'msg Arena,
+ pub _phantom_key: PhantomData<&'msg mut K>,
+ 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 Map<'msg, K, V> {}
-impl<'msg, K: ?Sized, V: ?Sized> Clone for Map<'msg, K, V> {
- fn clone(&self) -> Map<'msg, K, V> {
+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> {
*self
}
}
-impl<'msg, K: ?Sized, V: ?Sized> Map<'msg, K, V> {
- pub fn from_inner(_private: Private, inner: MapInner<'msg>) -> Self {
- Map { inner, _phantom_key: PhantomData, _phantom_value: PhantomData }
- }
-}
-
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! { $(
@@ -596,34 +590,34 @@
bool, bool_val, UpbCType::Bool, false;
);
- impl<'msg, V: [< MapWith $t:camel KeyOps >]> Map<'msg, $t, V> {
+ impl<'msg, V: [< MapWith $t:camel KeyOps >]> MapInner<'msg, $t, V> {
pub fn new(arena: &'msg mut Arena) -> Self {
- let inner = MapInner { raw: V::new_map(arena.raw()), arena };
- Map {
- inner,
+ MapInner {
+ raw: V::new_map(arena.raw()),
+ arena,
_phantom_key: PhantomData,
_phantom_value: PhantomData
}
}
pub fn size(&self) -> usize {
- V::size(self.inner.raw)
+ V::size(self.raw)
}
pub fn clear(&mut self) {
- V::clear(self.inner.raw)
+ V::clear(self.raw)
}
pub fn get(&self, key: $t) -> Option<V> {
- V::get(self.inner.raw, key)
+ V::get(self.raw, key)
}
pub fn remove(&mut self, key: $t) -> Option<V> {
- V::remove(self.inner.raw, key)
+ V::remove(self.raw, key)
}
pub fn insert(&mut self, key: $t, value: V) -> bool {
- V::insert(self.inner.raw, self.inner.arena.raw(), key, value)
+ V::insert(self.raw, self.arena.raw(), key, value)
}
}
)* }
@@ -717,7 +711,7 @@
#[test]
fn i32_i32_map() {
let mut arena = Arena::new();
- let mut map = Map::<'_, i32, i32>::new(&mut arena);
+ let mut map = MapInner::<'_, i32, i32>::new(&mut arena);
assert_that!(map.size(), eq(0));
assert_that!(map.insert(1, 2), eq(true));
@@ -738,7 +732,7 @@
#[test]
fn i64_f64_map() {
let mut arena = Arena::new();
- let mut map = Map::<'_, i64, f64>::new(&mut arena);
+ let mut map = MapInner::<'_, i64, f64>::new(&mut arena);
assert_that!(map.size(), eq(0));
assert_that!(map.insert(1, 2.5), eq(true));
diff --git a/src/google/protobuf/compiler/rust/accessors/map.cc b/src/google/protobuf/compiler/rust/accessors/map.cc
index 1df6b23..b398df1 100644
--- a/src/google/protobuf/compiler/rust/accessors/map.cc
+++ b/src/google/protobuf/compiler/rust/accessors/map.cc
@@ -34,7 +34,12 @@
let inner = unsafe {
$getter_thunk$(self.inner.msg)
}.map_or_else(|| unsafe {$pbr$::empty_map()}, |raw| {
- $pbr$::MapInner{ raw, arena: &self.inner.arena }
+ $pbr$::MapInner{
+ raw,
+ arena: &self.inner.arena,
+ _phantom_key: std::marker::PhantomData,
+ _phantom_value: std::marker::PhantomData,
+ }
});
$pb$::MapView::from_inner($pbi$::Private, inner)
})rs");
@@ -43,7 +48,8 @@
pub fn r#$field$(&self) -> $pb$::MapView<'_, $Key$, $Value$> {
let inner = $pbr$::MapInner {
raw: unsafe { $getter_thunk$(self.inner.msg) },
- _phantom: std::marker::PhantomData
+ _phantom_key: std::marker::PhantomData,
+ _phantom_value: std::marker::PhantomData,
};
$pb$::MapView::from_inner($pbi$::Private, inner)
})rs");
@@ -57,7 +63,12 @@
let raw = unsafe {
$getter_mut_thunk$(self.inner.msg, self.inner.arena.raw())
};
- let inner = $pbr$::MapInner{ raw, arena: &self.inner.arena };
+ let inner = $pbr$::MapInner{
+ raw,
+ arena: &self.inner.arena,
+ _phantom_key: std::marker::PhantomData,
+ _phantom_value: std::marker::PhantomData,
+ };
$pb$::MapMut::from_inner($pbi$::Private, inner)
})rs");
} else {
@@ -65,7 +76,8 @@
pub fn r#$field$_mut(&mut self) -> $pb$::MapMut<'_, $Key$, $Value$> {
let inner = $pbr$::MapInner {
raw: unsafe { $getter_mut_thunk$(self.inner.msg) },
- _phantom: std::marker::PhantomData
+ _phantom_key: std::marker::PhantomData,
+ _phantom_value: std::marker::PhantomData,
};
$pb$::MapMut::from_inner($pbi$::Private, inner)
})rs");
