Chiebot-Mirror
/
protobuf
mirror of https://github.com/protocolbuffers/protobuf.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Improve field publicity for repeated/maps

Browse Source 
A public "raw" field in a safe wrapper is guaranteed unsound!
This makes repeated and map inner access consistent and
avoids exposing raw internals.

It also provides the accessors necessary for implementing map
access for external types.

PiperOrigin-RevId: 604405543
pull/15688/head
Alyssa Haroldsen 1 year ago committed by Copybara-Service
parent 3995f538ed
commit 3657e05292
4 changed files with 72 additions and 51 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 18
      rust/cpp.rs
  2. 37
      rust/map.rs
  3. 29
      rust/repeated.rs
  4. 39
      rust/upb.rs

18
rust/cpp.rs
Unescape View File

@ -437,38 +437,38 @@ macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types {
// SAFETY: // SAFETY:
// - `map.inner.raw` is a live `RawMap` // - `map.inner.raw` is a live `RawMap`
// - This function is only called once for `map` in `Drop`. // - This function is only called once for `map` in `Drop`.
unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _free >](map.inner.raw); } unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _free >](map.as_mut().as_raw(Private)); }
} }
fn map_clear(map: Mut<'_, Map<$key_t, Self>>) { fn map_clear(mut map: Mut<'_, Map<$key_t, Self>>) {
unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _clear >](map.inner.raw); } unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _clear >](map.as_raw(Private)); }
} }
fn map_len(map: View<'_, Map<$key_t, Self>>) -> usize { fn map_len(map: View<'_, Map<$key_t, Self>>) -> usize {
unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _size >](map.raw) } unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _size >](map.as_raw(Private)) }
} }
fn map_insert(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: View<'_, Self>) -> bool {
let ffi_key = $to_ffi_key(key); let ffi_key = $to_ffi_key(key);
let ffi_value = $to_ffi_value(value); let ffi_value = $to_ffi_value(value);
unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _insert >](map.inner.raw, ffi_key, ffi_value) } unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _insert >](map.as_raw(Private), ffi_key, ffi_value) }
} }
fn map_get<'a>(map: View<'a, Map<$key_t, Self>>, key: View<'_, $key_t>) -> Option<View<'a, Self>> { fn map_get<'a>(map: View<'a, Map<$key_t, Self>>, key: View<'_, $key_t>) -> Option<View<'a, Self>> {
let ffi_key = $to_ffi_key(key); let ffi_key = $to_ffi_key(key);
let mut ffi_value = $to_ffi_value($zero_val); let mut ffi_value = $to_ffi_value($zero_val);
let found = unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _get >](map.raw, ffi_key, &mut ffi_value) }; let found = unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _get >](map.as_raw(Private), ffi_key, &mut ffi_value) };
if !found { if !found {
return None; return None;
} }
Some($from_ffi_value(ffi_value)) Some($from_ffi_value(ffi_value))
} }
fn map_remove(map: Mut<'_, Map<$key_t, Self>>, key: View<'_, $key_t>) -> bool { fn map_remove(mut map: Mut<'_, Map<$key_t, Self>>, key: View<'_, $key_t>) -> bool {
let ffi_key = $to_ffi_key(key); let ffi_key = $to_ffi_key(key);
let mut ffi_value = $to_ffi_value($zero_val); let mut ffi_value = $to_ffi_value($zero_val);
unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _remove >](map.inner.raw, ffi_key, &mut ffi_value) } unsafe { [< __rust_proto_thunk__Map_ $key_t _ $t _remove >](map.as_raw(Private), ffi_key, &mut ffi_value) }
} }
} }
)* } )* }

37
rust/map.rs
Unescape View File

@ -14,7 +14,7 @@ use std::marker::PhantomData;
#[repr(transparent)] #[repr(transparent)]
pub struct MapView<'msg, K: ?Sized, V: ?Sized> { pub struct MapView<'msg, K: ?Sized, V: ?Sized> {
pub raw: RawMap, raw: RawMap,
_phantom: PhantomData<(&'msg K, &'msg V)>, _phantom: PhantomData<(&'msg K, &'msg V)>,
} }
@ -54,7 +54,7 @@ impl<'msg, K: ?Sized, V: ?Sized> std::fmt::Debug for MapMut<'msg, K, V> {
} }
pub struct Map<K: ?Sized + Proxied, V: ?Sized + ProxiedInMapValue<K>> { pub struct Map<K: ?Sized + Proxied, V: ?Sized + ProxiedInMapValue<K>> {
pub(crate) inner: InnerMapMut<'static>, inner: InnerMapMut<'static>,
_phantom: PhantomData<(PhantomData<K>, PhantomData<V>)>, _phantom: PhantomData<(PhantomData<K>, PhantomData<V>)>,
} }
@ -171,16 +171,14 @@ where
/// - `inner` must be valid to read and write from for `'static`. /// - `inner` must be valid to read and write from for `'static`.
/// - There must be no aliasing references or mutations on the same /// - There must be no aliasing references or mutations on the same
/// underlying object. /// underlying object.
#[doc(hidden)]
pub unsafe fn from_inner(_private: Private, inner: InnerMapMut<'static>) -> Self { pub unsafe fn from_inner(_private: Private, inner: InnerMapMut<'static>) -> Self {
Self { inner, _phantom: PhantomData } Self { inner, _phantom: PhantomData }
} }
} }
impl<'msg, K, V> MapView<'msg, K, V> #[doc(hidden)]
where impl<'msg, K: ?Sized, V: ?Sized> MapView<'msg, K, V> {
K: Proxied + ?Sized + 'msg,
V: ProxiedInMapValue<K> + ?Sized + 'msg,
{
#[doc(hidden)] #[doc(hidden)]
pub fn as_raw(&self, _private: Private) -> RawMap { pub fn as_raw(&self, _private: Private) -> RawMap {
self.raw self.raw
@ -192,7 +190,13 @@ where
pub unsafe fn from_raw(_private: Private, raw: RawMap) -> Self { pub unsafe fn from_raw(_private: Private, raw: RawMap) -> Self {
Self { raw, _phantom: PhantomData } Self { raw, _phantom: PhantomData }
} }
}
impl<'msg, K, V> MapView<'msg, K, V>
where
K: Proxied + ?Sized + 'msg,
V: ProxiedInMapValue<K> + ?Sized + 'msg,
{
pub fn get<'a>(self, key: impl Into<View<'a, K>>) -> Option<View<'msg, V>> pub fn get<'a>(self, key: impl Into<View<'a, K>>) -> Option<View<'msg, V>>
where where
K: 'a, K: 'a,
@ -209,17 +213,26 @@ where
} }
} }
impl<'msg, K, V> MapMut<'msg, K, V> #[doc(hidden)]
where impl<'msg, K: ?Sized, V: ?Sized> MapMut<'msg, K, V> {
K: Proxied + ?Sized + 'msg,
V: ProxiedInMapValue<K> + ?Sized + 'msg,
{
/// # Safety /// # Safety
/// - `inner` must be valid to read and write from for `'msg`. /// - `inner` must be valid to read and write from for `'msg`.
#[doc(hidden)]
pub unsafe fn from_inner(_private: Private, inner: InnerMapMut<'msg>) -> Self { pub unsafe fn from_inner(_private: Private, inner: InnerMapMut<'msg>) -> Self {
Self { inner, _phantom: PhantomData } Self { inner, _phantom: PhantomData }
} }
#[doc(hidden)]
pub fn as_raw(&mut self, _private: Private) -> RawMap {
self.inner.raw
}
}
impl<'msg, K, V> MapMut<'msg, K, V>
where
K: Proxied + ?Sized + 'msg,
V: ProxiedInMapValue<K> + ?Sized + 'msg,
{
pub fn len(self) -> usize { pub fn len(self) -> usize {
self.as_view().len() self.as_view().len()
} }

29
rust/repeated.rs
Unescape View File

@ -58,10 +58,8 @@ impl<'msg, T: ?Sized> Debug for RepeatedMut<'msg, T> {
} }
} }
impl<'msg, T> RepeatedView<'msg, T> #[doc(hidden)]
where impl<'msg, T: ?Sized> RepeatedView<'msg, T> {
T: ProxiedInRepeated + ?Sized + 'msg,
{
#[doc(hidden)] #[doc(hidden)]
pub fn as_raw(&self, _private: Private) -> RawRepeatedField { pub fn as_raw(&self, _private: Private) -> RawRepeatedField {
self.raw self.raw
@ -73,7 +71,12 @@ where
pub unsafe fn from_raw(_private: Private, raw: RawRepeatedField) -> Self { pub unsafe fn from_raw(_private: Private, raw: RawRepeatedField) -> Self {
Self { raw, _phantom: PhantomData } Self { raw, _phantom: PhantomData }
} }
}
impl<'msg, T> RepeatedView<'msg, T>
where
T: ProxiedInRepeated + ?Sized + 'msg,
{
/// Gets the length of the repeated field. /// Gets the length of the repeated field.
pub fn len(&self) -> usize { pub fn len(&self) -> usize {
T::repeated_len(*self) T::repeated_len(*self)
@ -110,10 +113,8 @@ where
} }
} }
impl<'msg, T> RepeatedMut<'msg, T> #[doc(hidden)]
where impl<'msg, T: ?Sized> RepeatedMut<'msg, T> {
T: ProxiedInRepeated + ?Sized + 'msg,
{
/// # Safety /// # Safety
/// - `inner` must be valid to read and write from for `'msg` /// - `inner` must be valid to read and write from for `'msg`
/// - There must be no aliasing references or mutations on the same /// - There must be no aliasing references or mutations on the same
@ -123,18 +124,16 @@ where
Self { inner, _phantom: PhantomData } Self { inner, _phantom: PhantomData }
} }
/// # Safety
/// - The return value must not be mutated through without synchronization.
#[allow(dead_code)]
pub(crate) unsafe fn into_inner(self) -> InnerRepeatedMut<'msg> {
self.inner
}
#[doc(hidden)] #[doc(hidden)]
pub fn as_raw(&mut self, _private: Private) -> RawRepeatedField { pub fn as_raw(&mut self, _private: Private) -> RawRepeatedField {
self.inner.raw self.inner.raw
} }
}
impl<'msg, T> RepeatedMut<'msg, T>
where
T: ProxiedInRepeated + ?Sized + 'msg,
{
/// Gets the length of the repeated field. /// Gets the length of the repeated field.
pub fn len(&self) -> usize { pub fn len(&self) -> usize {
self.as_view().len() self.as_view().len()

39
rust/upb.rs
Unescape View File

@ -9,7 +9,7 @@
use crate::__internal::{Enum, Private, PtrAndLen, RawArena, RawMap, RawMessage, RawRepeatedField}; use crate::__internal::{Enum, Private, PtrAndLen, RawArena, RawMap, RawMessage, RawRepeatedField};
use crate::{ use crate::{
Map, MapView, Mut, ProtoStr, Proxied, ProxiedInMapValue, ProxiedInRepeated, Repeated, Map, MapMut, MapView, Mut, ProtoStr, Proxied, ProxiedInMapValue, ProxiedInRepeated, Repeated,
RepeatedMut, RepeatedView, SettableValue, View, ViewProxy, RepeatedMut, RepeatedView, SettableValue, View, ViewProxy,
}; };
use core::fmt::Debug; use core::fmt::Debug;
@ -511,7 +511,7 @@ macro_rules! impl_repeated_primitives {
// - `copy_nonoverlapping` is unsafe but here we guarantee that both pointers // - `copy_nonoverlapping` is unsafe but here we guarantee that both pointers
// are valid, the pointers are `#[repr(u8)]`, and the size is correct. // are valid, the pointers are `#[repr(u8)]`, and the size is correct.
unsafe { unsafe {
if (!upb_Array_Resize(dest.as_raw(Private), src.len(), dest.inner.arena)) { if (!upb_Array_Resize(dest.as_raw(Private), src.len(), dest.raw_arena(Private))) {
panic!("upb_Array_Resize failed."); panic!("upb_Array_Resize failed.");
} }
ptr::copy_nonoverlapping( ptr::copy_nonoverlapping(
@ -528,7 +528,7 @@ macro_rules! impl_repeated_primitives {
impl<'msg, T: ?Sized> RepeatedMut<'msg, T> { impl<'msg, T: ?Sized> RepeatedMut<'msg, T> {
// Returns a `RawArena` which is live for at least `'msg` // Returns a `RawArena` which is live for at least `'msg`
#[doc(hidden)] #[doc(hidden)]
pub fn raw_arena(&self, _private: Private) -> RawArena { pub fn raw_arena(&mut self, _private: Private) -> RawArena {
self.inner.arena self.inner.arena
} }
} }
@ -598,7 +598,7 @@ pub fn cast_enum_repeated_mut<E: Enum + ProxiedInRepeated>(
// - Reading an enum array as an i32 array is sound. // - Reading an enum array as an i32 array is sound.
// - No shared mutation is possible through the output. // - No shared mutation is possible through the output.
unsafe { unsafe {
let InnerRepeatedMut { arena, raw, .. } = repeated.into_inner(); let InnerRepeatedMut { arena, raw, .. } = repeated.inner;
RepeatedMut::from_inner(private, InnerRepeatedMut { arena, raw, _phantom: PhantomData }) RepeatedMut::from_inner(private, InnerRepeatedMut { arena, raw, _phantom: PhantomData })
} }
} }
@ -649,6 +649,14 @@ where
} }
} }
impl<'msg, K: ?Sized, V: ?Sized> MapMut<'msg, K, V> {
// Returns a `RawArena` which is live for at least `'msg`
#[doc(hidden)]
pub fn raw_arena(&mut self, _private: Private) -> RawArena {
self.inner.raw_arena
}
}
#[derive(Clone, Copy, Debug)] #[derive(Clone, Copy, Debug)]
pub struct InnerMapMut<'msg> { pub struct InnerMapMut<'msg> {
pub(crate) raw: RawMap, pub(crate) raw: RawMap,
@ -814,29 +822,30 @@ macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types {
// - `map.inner.raw_arena` is a live `upb_Arena*` // - `map.inner.raw_arena` is a live `upb_Arena*`
// - This function is only called once for `map` in `Drop`. // - This function is only called once for `map` in `Drop`.
unsafe { unsafe {
upb_Arena_Free(map.inner.raw_arena); upb_Arena_Free(map.as_mut().raw_arena(Private));
} }
} }
fn map_clear(map: Mut<'_, Map<$key_t, Self>>) { fn map_clear(mut map: Mut<'_, Map<$key_t, Self>>) {
unsafe { unsafe {
upb_Map_Clear(map.inner.raw); upb_Map_Clear(map.as_raw(Private));
} }
} }
fn map_len(map: View<'_, Map<$key_t, Self>>) -> usize { fn map_len(map: View<'_, Map<$key_t, Self>>) -> usize {
unsafe { unsafe {
upb_Map_Size(map.raw) upb_Map_Size(map.as_raw(Private))
} }
} }
fn map_insert(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: View<'_, Self>) -> bool {
let arena = map.raw_arena(Private);
unsafe { unsafe {
upb_Map_InsertAndReturnIfInserted( upb_Map_InsertAndReturnIfInserted(
map.inner.raw, map.as_raw(Private),
<$key_t as UpbTypeConversions>::to_message_value(key), <$key_t as UpbTypeConversions>::to_message_value(key),
<$t as UpbTypeConversions>::to_message_value_copy_if_required(map.inner.raw_arena, value), <$t as UpbTypeConversions>::to_message_value_copy_if_required(arena, value),
map.inner.raw_arena arena
) )
} }
} }
@ -844,7 +853,7 @@ macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types {
fn map_get<'a>(map: View<'a, Map<$key_t, Self>>, key: View<'_, $key_t>) -> Option<View<'a, Self>> { fn map_get<'a>(map: View<'a, Map<$key_t, Self>>, key: View<'_, $key_t>) -> Option<View<'a, Self>> {
let mut val = <$t as UpbTypeConversions>::empty_message_value(); let mut val = <$t as UpbTypeConversions>::empty_message_value();
let found = unsafe { let found = unsafe {
upb_Map_Get(map.raw, <$key_t as UpbTypeConversions>::to_message_value(key), upb_Map_Get(map.as_raw(Private), <$key_t as UpbTypeConversions>::to_message_value(key),
&mut val) &mut val)
}; };
if !found { if !found {
@ -853,10 +862,10 @@ macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types {
Some(unsafe { <$t as UpbTypeConversions>::from_message_value(val) }) Some(unsafe { <$t as UpbTypeConversions>::from_message_value(val) })
} }
fn map_remove(map: Mut<'_, Map<$key_t, Self>>, key: View<'_, $key_t>) -> bool { fn map_remove(mut map: Mut<'_, Map<$key_t, Self>>, key: View<'_, $key_t>) -> bool {
let mut val = <$t as UpbTypeConversions>::empty_message_value(); let mut val = <$t as UpbTypeConversions>::empty_message_value();
unsafe { unsafe {
upb_Map_Delete(map.inner.raw, upb_Map_Delete(map.as_raw(Private),
<$key_t as UpbTypeConversions>::to_message_value(key), <$key_t as UpbTypeConversions>::to_message_value(key),
&mut val) &mut val)
} }

Write Preview
Loading…
Cancel
Save