From 1980e025eb315ed6c8d1c0ba2c5653022d889189 Mon Sep 17 00:00:00 2001 From: Jakob Buchgraber Date: Mon, 29 Apr 2024 03:39:57 -0700 Subject: [PATCH] Remove PrimitiveMut and related vtable types The last callside that used PrimitiveMut was in our enums code. This change makes it so that enums nolonger implement MutProxied and thus no longer need the PrimitiveMut type. PiperOrigin-RevId: 629017282 --- rust/BUILD | 2 +- rust/cpp.rs | 1 - rust/internal.rs | 5 +- rust/primitive.rs | 129 +-------------------- rust/shared.rs | 1 - rust/upb.rs | 1 - rust/vtable.rs | 135 +--------------------- src/google/protobuf/compiler/rust/enum.cc | 33 ------ 8 files changed, 6 insertions(+), 301 deletions(-) diff --git a/rust/BUILD b/rust/BUILD index 24dce3acce..21fe525d4b 100644 --- a/rust/BUILD +++ b/rust/BUILD @@ -48,8 +48,8 @@ rust_library( PROTOBUF_SHARED = [ "enum.rs", "internal.rs", - "optional.rs", "primitive.rs", + "optional.rs", "proxied.rs", "repeated.rs", "shared.rs", diff --git a/rust/cpp.rs b/rust/cpp.rs index 3b70dff6ce..06b45f6f92 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -238,7 +238,6 @@ pub type MessageAbsentMutData<'msg, T> = crate::vtable::RawVTableOptionalMutator pub type BytesPresentMutData<'msg> = crate::vtable::RawVTableOptionalMutatorData<'msg, [u8]>; pub type BytesAbsentMutData<'msg> = crate::vtable::RawVTableOptionalMutatorData<'msg, [u8]>; pub type InnerBytesMut<'msg> = crate::vtable::RawVTableMutator<'msg, [u8]>; -pub type InnerPrimitiveMut<'msg, T> = crate::vtable::RawVTableMutator<'msg, T>; pub type RawMapIter = UntypedMapIterator; #[derive(Debug)] diff --git a/rust/internal.rs b/rust/internal.rs index 4a9486e91a..742423bc10 100644 --- a/rust/internal.rs +++ b/rust/internal.rs @@ -14,9 +14,8 @@ pub use paste::paste; pub use crate::r#enum::Enum; pub use crate::vtable::{ - new_vtable_field_entry, BytesMutVTable, BytesOptionalMutVTable, PrimitiveOptionalMutVTable, - PrimitiveVTable, PrimitiveWithRawVTable, ProxiedWithRawOptionalVTable, ProxiedWithRawVTable, - RawVTableMutator, RawVTableOptionalMutatorData, + new_vtable_field_entry, BytesMutVTable, BytesOptionalMutVTable, ProxiedWithRawOptionalVTable, + ProxiedWithRawVTable, RawVTableMutator, RawVTableOptionalMutatorData, }; pub use crate::ProtoStr; diff --git a/rust/primitive.rs b/rust/primitive.rs index f14863187b..75665eca2b 100644 --- a/rust/primitive.rs +++ b/rust/primitive.rs @@ -4,96 +4,7 @@ // 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 std::fmt::Debug; - -use crate::__internal::Private; -use crate::__runtime::InnerPrimitiveMut; -use crate::vtable::{PrimitiveWithRawVTable, ProxiedWithRawVTable, RawVTableOptionalMutatorData}; -use crate::{ - Mut, MutProxied, MutProxy, Proxied, ProxiedWithPresence, SettableValue, View, ViewProxy, -}; - -/// A mutator for a primitive (numeric or enum) value of `T`. -/// -/// This type is `protobuf::Mut<'msg, T>`. -pub struct PrimitiveMut<'msg, T> { - inner: InnerPrimitiveMut<'msg, T>, -} - -impl<'msg, T> Debug for PrimitiveMut<'msg, T> -where - T: PrimitiveWithRawVTable, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("PrimitiveMut").field("inner", &self.inner).finish() - } -} - -impl<'msg, T> PrimitiveMut<'msg, T> { - /// # Safety - /// `inner` must be valid and non-aliased for `T` for `'msg` - #[doc(hidden)] - pub unsafe fn from_inner(_private: Private, inner: InnerPrimitiveMut<'msg, T>) -> Self { - Self { inner } - } -} - -// SAFETY: all `T` that can perform mutations don't mutate through a shared -// reference. -unsafe impl<'msg, T> Sync for PrimitiveMut<'msg, T> {} - -impl<'msg, T> PrimitiveMut<'msg, T> -where - T: PrimitiveWithRawVTable, -{ - /// Gets the current value of the field. - pub fn get(&self) -> View<'_, T> { - T::make_view(Private, self.inner) - } - - /// Sets a new value for the field. - pub fn set(&mut self, val: impl SettableValue) { - val.set_on(Private, self.as_mut()) - } - - #[doc(hidden)] - pub fn set_primitive(&mut self, _private: Private, value: T) { - // SAFETY: the raw mutator is valid for `'msg` as enforced by `Mut` - unsafe { self.inner.set(value) } - } -} - -impl<'msg, T> ViewProxy<'msg> for PrimitiveMut<'msg, T> -where - T: PrimitiveWithRawVTable, -{ - type Proxied = T; - - fn as_view(&self) -> View<'_, Self::Proxied> { - self.get() - } - - fn into_view<'shorter>(self) -> View<'shorter, Self::Proxied> { - self.get() - } -} - -impl<'msg, T> MutProxy<'msg> for PrimitiveMut<'msg, T> -where - T: PrimitiveWithRawVTable, -{ - fn as_mut(&mut self) -> Mut<'_, Self::Proxied> { - PrimitiveMut { inner: self.inner } - } - - fn into_mut<'shorter>(self) -> Mut<'shorter, Self::Proxied> - where - 'msg: 'shorter, - { - self - } -} +use crate::{Proxied, View, ViewProxy}; macro_rules! impl_singular_primitives { ($($t:ty),*) => { @@ -102,10 +13,6 @@ macro_rules! impl_singular_primitives { type View<'msg> = $t; } - impl MutProxied for $t { - type Mut<'msg> = PrimitiveMut<'msg, $t>; - } - impl<'msg> ViewProxy<'msg> for $t { type Proxied = $t; @@ -118,40 +25,6 @@ macro_rules! impl_singular_primitives { } } - impl SettableValue<$t> for $t { - fn set_on<'msg>(self, private: Private, mut mutator: Mut<'msg, $t>) where $t: 'msg { - mutator.set_primitive(private, self) - } - - fn set_on_absent( - self, - _private: Private, - absent_mutator: <$t as ProxiedWithPresence>::PresentMutData<'_>, - ) -> <$t as ProxiedWithPresence>::AbsentMutData<'_> - { - absent_mutator.set(Private, self) - } - } - - impl ProxiedWithPresence for $t { - type PresentMutData<'msg> = RawVTableOptionalMutatorData<'msg, $t>; - type AbsentMutData<'msg> = RawVTableOptionalMutatorData<'msg, $t>; - - fn clear_present_field( - present_mutator: Self::PresentMutData<'_>, - ) -> Self::AbsentMutData<'_> { - present_mutator.clear(Private) - } - - fn set_absent_to_default( - absent_mutator: Self::AbsentMutData<'_>, - ) -> Self::PresentMutData<'_> { - absent_mutator.set_absent_to_default(Private) - } - } - - impl PrimitiveWithRawVTable for $t {} - // ProxiedInRepeated is implemented in {cpp,upb}.rs )* } diff --git a/rust/shared.rs b/rust/shared.rs index 4eacf7c767..8ea0684a39 100644 --- a/rust/shared.rs +++ b/rust/shared.rs @@ -25,7 +25,6 @@ pub mod __public { pub use crate::r#enum::UnknownEnumValue; pub use crate::map::{Map, MapIter, MapMut, MapView, ProxiedInMapValue}; pub use crate::optional::{AbsentField, FieldEntry, Optional, PresentField}; - pub use crate::primitive::PrimitiveMut; pub use crate::proto; pub use crate::proxied::{ IntoProxied, Mut, MutProxied, MutProxy, Proxied, ProxiedWithPresence, SettableValue, View, diff --git a/rust/upb.rs b/rust/upb.rs index 413f9851e1..98e1c849c1 100644 --- a/rust/upb.rs +++ b/rust/upb.rs @@ -66,7 +66,6 @@ pub type MessageAbsentMutData<'msg, T> = crate::vtable::RawVTableOptionalMutator pub type BytesPresentMutData<'msg> = crate::vtable::RawVTableOptionalMutatorData<'msg, [u8]>; pub type BytesAbsentMutData<'msg> = crate::vtable::RawVTableOptionalMutatorData<'msg, [u8]>; pub type InnerBytesMut<'msg> = crate::vtable::RawVTableMutator<'msg, [u8]>; -pub type InnerPrimitiveMut<'msg, T> = crate::vtable::RawVTableMutator<'msg, T>; #[derive(Debug)] pub struct MessageVTable { diff --git a/rust/vtable.rs b/rust/vtable.rs index fbcf4c46a2..27f14db922 100644 --- a/rust/vtable.rs +++ b/rust/vtable.rs @@ -7,11 +7,10 @@ use crate::__internal::Private; use crate::__runtime::{ - copy_bytes_in_arena_if_needed_by_runtime, InnerPrimitiveMut, MutatorMessageRef, PtrAndLen, - RawMessage, + copy_bytes_in_arena_if_needed_by_runtime, MutatorMessageRef, PtrAndLen, RawMessage, }; use crate::{ - AbsentField, FieldEntry, Mut, MutProxied, MutProxy, Optional, PresentField, PrimitiveMut, + AbsentField, FieldEntry, Mut, MutProxied, MutProxy, Optional, PresentField, ProxiedWithPresence, View, ViewProxy, }; use std::fmt::{self, Debug}; @@ -281,47 +280,6 @@ impl ProxiedWithRawOptionalVTable for [u8] { } } -/// A generic thunk vtable for mutating a present primitive field. -#[doc(hidden)] -#[derive(Debug)] -pub struct PrimitiveVTable { - pub(crate) setter: unsafe extern "C" fn(msg: RawMessage, val: T), - pub(crate) getter: unsafe extern "C" fn(msg: RawMessage) -> T, -} - -#[doc(hidden)] -#[derive(Debug)] -/// A generic thunk vtable for mutating an `optional` primitive field. -pub struct PrimitiveOptionalMutVTable { - pub(crate) base: PrimitiveVTable, - pub(crate) clearer: unsafe extern "C" fn(msg: RawMessage), - pub(crate) default: T, -} - -impl PrimitiveVTable { - #[doc(hidden)] - pub const fn new( - _private: Private, - getter: unsafe extern "C" fn(msg: RawMessage) -> T, - setter: unsafe extern "C" fn(msg: RawMessage, val: T), - ) -> Self { - Self { getter, setter } - } -} - -impl PrimitiveOptionalMutVTable { - #[doc(hidden)] - pub const fn new( - _private: Private, - getter: unsafe extern "C" fn(msg: RawMessage) -> T, - setter: unsafe extern "C" fn(msg: RawMessage, val: T), - clearer: unsafe extern "C" fn(msg: RawMessage), - default: T, - ) -> Self { - Self { base: PrimitiveVTable { getter, setter }, clearer, default } - } -} - /// A generic thunk vtable for mutating a present `bytes` or `string` field. #[doc(hidden)] #[derive(Debug)] @@ -425,92 +383,3 @@ impl<'msg> RawVTableOptionalMutatorData<'msg, [u8]> { self } } - -/// Primitive types using a vtable for message access that are trivial to copy -/// and have a `'static` lifetime. -/// -/// Implementing this trait automatically implements `ProxiedWithRawVTable`, -/// `ProxiedWithRawOptionalVTable`, and get/set/clear methods on -/// `RawVTableMutator` and `RawVTableOptionalMutatorData` that use the vtable. -/// -/// It doesn't implement `Proxied`, `ViewProxy`, `SettableValue` or -/// `ProxiedWithPresence` for `Self` to avoid future conflicting blanket impls -/// on those traits. -pub trait PrimitiveWithRawVTable: - Copy - + Debug - + 'static - + ProxiedWithPresence - + Sync - + Send - + for<'msg> MutProxied = Self, Mut<'msg> = PrimitiveMut<'msg, Self>> -{ -} - -impl ProxiedWithRawVTable for T { - type VTable = PrimitiveVTable; - - fn make_view(_private: Private, mut_inner: InnerPrimitiveMut<'_, Self>) -> Self { - mut_inner.get() - } - - fn make_mut(_private: Private, inner: InnerPrimitiveMut<'_, Self>) -> PrimitiveMut<'_, Self> { - // SAFETY: `inner` is valid for the necessary lifetime and `T` as promised by - // the caller of `InnerPrimitiveMut::new`. - unsafe { PrimitiveMut::from_inner(Private, inner) } - } -} - -impl ProxiedWithRawOptionalVTable for T { - type OptionalVTable = PrimitiveOptionalMutVTable; - - fn upcast_vtable( - _private: Private, - optional_vtable: &'static Self::OptionalVTable, - ) -> &'static Self::VTable { - &optional_vtable.base - } -} - -impl RawVTableMutator<'_, T> { - pub(crate) fn get(self) -> T { - // SAFETY: - // - `msg_ref` is valid for the lifetime of `RawVTableMutator` as promised by - // the caller of `new`. - unsafe { (self.vtable().getter)(self.msg_ref.msg()) } - } - - /// # Safety - /// - `msg_ref` must be valid for the lifetime of `RawVTableMutator`. - pub(crate) unsafe fn set(self, val: T) { - // SAFETY: - // - `msg_ref` is valid for the lifetime of `RawVTableMutator` as promised by - // the caller of `new`. - unsafe { (self.vtable().setter)(self.msg_ref.msg(), val) } - } -} - -impl<'msg, T: PrimitiveWithRawVTable> RawVTableOptionalMutatorData<'msg, T> { - pub fn set_absent_to_default(self, private: Private) -> Self { - // SAFETY: - // - `msg_ref` is valid for the lifetime of `RawVTableOptionalMutatorData` as - // promised by the caller of `new`. - self.set(private, self.optional_vtable().default) - } - - pub fn set(self, _private: Private, val: T) -> Self { - // SAFETY: - // - `msg_ref` is valid for the lifetime of `RawVTableOptionalMutatorData` as - // promised by the caller of `new`. - unsafe { (self.optional_vtable().base.setter)(self.msg_ref.msg(), val) } - self - } - - pub fn clear(self, _private: Private) -> Self { - // SAFETY: - // - `msg_ref` is valid for the lifetime of `RawVTableOptionalMutatorData` as - // promised by the caller of `new`. - unsafe { (self.optional_vtable().clearer)(self.msg_ref.msg()) } - self - } -} diff --git a/src/google/protobuf/compiler/rust/enum.cc b/src/google/protobuf/compiler/rust/enum.cc index 5ce270b54f..521dcd3ce3 100644 --- a/src/google/protobuf/compiler/rust/enum.cc +++ b/src/google/protobuf/compiler/rust/enum.cc @@ -395,10 +395,6 @@ void GenerateEnumDefinition(Context& ctx, const EnumDescriptor& desc) { type View<'a> = $name$; } - impl $pb$::MutProxied for $name$ { - type Mut<'a> = $pb$::PrimitiveMut<'a, $name$>; - } - impl $pb$::ViewProxy<'_> for $name$ { type Proxied = $name$; @@ -411,33 +407,6 @@ void GenerateEnumDefinition(Context& ctx, const EnumDescriptor& desc) { } } - impl $pb$::SettableValue<$name$> for $name$ { - fn set_on<'msg>( - self, - private: $pbi$::Private, - mut mutator: $pb$::Mut<'msg, $name$> - ) where $name$: 'msg { - mutator.set_primitive(private, self) - } - } - - impl $pb$::ProxiedWithPresence for $name$ { - type PresentMutData<'a> = $pbi$::RawVTableOptionalMutatorData<'a, $name$>; - type AbsentMutData<'a> = $pbi$::RawVTableOptionalMutatorData<'a, $name$>; - - fn clear_present_field( - present_mutator: Self::PresentMutData<'_>, - ) -> Self::AbsentMutData<'_> { - present_mutator.clear($pbi$::Private) - } - - fn set_absent_to_default( - absent_mutator: Self::AbsentMutData<'_>, - ) -> Self::PresentMutData<'_> { - absent_mutator.set_absent_to_default($pbi$::Private) - } - } - unsafe impl $pb$::ProxiedInRepeated for $name$ { fn repeated_len(r: $pb$::View<$pb$::Repeated>) -> usize { $pbr$::cast_enum_repeated_view($pbi$::Private, r).len() @@ -485,8 +454,6 @@ void GenerateEnumDefinition(Context& ctx, const EnumDescriptor& desc) { } } - impl $pbi$::PrimitiveWithRawVTable for $name$ {} - // SAFETY: this is an enum type unsafe impl $pbi$::Enum for $name$ { const NAME: &'static str = "$name$";