From b6ea6f91922fbc04976e58b74db8f81244f37465 Mon Sep 17 00:00:00 2001 From: Alyssa Haroldsen Date: Wed, 17 Jan 2024 16:50:36 -0800 Subject: [PATCH] Remove Deref from RepeatedMut PiperOrigin-RevId: 599328590 --- rust/repeated.rs | 51 +++++++++++++-------- rust/test/shared/accessors_repeated_test.rs | 11 +++-- 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/rust/repeated.rs b/rust/repeated.rs index 498b55077a..8dc67f4348 100644 --- a/rust/repeated.rs +++ b/rust/repeated.rs @@ -53,19 +53,9 @@ pub struct RepeatedMut<'msg, T: ?Sized> { unsafe impl<'msg, T: ?Sized> Sync for RepeatedMut<'msg, T> {} -impl<'msg, T: ?Sized> Deref for RepeatedMut<'msg, T> { - type Target = RepeatedView<'msg, T>; - fn deref(&self) -> &Self::Target { - // SAFETY: - // - `RepeatedView<'msg, T>` is `#[repr(transparent)]` over - // `RawRepeatedField`. - unsafe { &*(&self.inner.raw as *const RawRepeatedField as *const RepeatedView<'msg, T>) } - } -} - impl<'msg, T: ?Sized> Debug for RepeatedMut<'msg, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("RepeatedMut").field("raw", &self.raw).finish() + f.debug_struct("RepeatedMut").field("raw", &self.inner.raw).finish() } } @@ -146,6 +136,32 @@ where self.inner.raw } + /// Gets the length of the repeated field. + pub fn len(&self) -> usize { + self.as_view().len() + } + + /// Returns true if the repeated field has no values. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Gets the value at `index`. + /// + /// Returns `None` if `index > len`. + pub fn get(&self, index: usize) -> Option> { + self.as_view().get(index) + } + + /// Gets the value at `index` without bounds-checking. + /// + /// # Safety + /// Undefined behavior if `index >= len` + pub unsafe fn get_unchecked(&self, index: usize) -> View { + // SAFETY: in-bounds as promised + unsafe { self.as_view().get_unchecked(index) } + } + /// Appends `val` to the end of the repeated field. pub fn push(&mut self, val: View) { T::repeated_push(self.as_mut(), val); @@ -173,12 +189,9 @@ where unsafe { T::repeated_set_unchecked(self.as_mut(), index, val) } } - /// Returns the value at `index`. - // This is defined as an inherent function to prevent `MutProxy::get` from being - // preferred over `RepeatedView::get`. The former gets priority as it does - // not require a deref. - pub fn get(&self, index: usize) -> Option> { - self.as_view().get(index) + /// Iterates over the values in the repeated field. + pub fn iter(&self) -> RepeatedIter { + self.as_view().into_iter() } /// Copies from the `src` repeated field into this one. @@ -343,14 +356,14 @@ where type Proxied = Repeated; fn as_view(&self) -> View<'_, Self::Proxied> { - **self + RepeatedView { raw: self.inner.raw, _phantom: PhantomData } } fn into_view<'shorter>(self) -> View<'shorter, Self::Proxied> where 'msg: 'shorter, { - *self.into_mut::<'shorter>() + RepeatedView { raw: self.inner.raw, _phantom: PhantomData } } } diff --git a/rust/test/shared/accessors_repeated_test.rs b/rust/test/shared/accessors_repeated_test.rs index 0f9d724505..1852847b4f 100644 --- a/rust/test/shared/accessors_repeated_test.rs +++ b/rust/test/shared/accessors_repeated_test.rs @@ -7,6 +7,7 @@ use googletest::prelude::*; use paste::paste; +use protobuf::ViewProxy; use unittest_proto::proto2_unittest::{TestAllTypes, TestAllTypes_, TestAllTypes_::NestedMessage}; macro_rules! generate_repeated_numeric_test { @@ -38,7 +39,7 @@ macro_rules! generate_repeated_numeric_test { elements_are![eq(2 as $t), eq(1 as $t), eq(0 as $t)] ); assert_that!( - (*mutator).into_iter().collect::>(), + mutator.as_view().into_iter().collect::>(), elements_are![eq(2 as $t), eq(1 as $t), eq(0 as $t)] ); @@ -60,7 +61,7 @@ macro_rules! generate_repeated_numeric_test { for i in 0..5 { mutator2.push(i as $t); } - protobuf::MutProxy::set(&mut mutator, *mutator2); + protobuf::MutProxy::set(&mut mutator, mutator2.as_view()); assert_that!( mutator.iter().collect::>(), @@ -103,7 +104,7 @@ fn test_repeated_bool_accessors() { assert_that!(mutator.iter().collect::>(), elements_are![eq(false), eq(true), eq(false)]); assert_that!( - (*mutator).into_iter().collect::>(), + mutator.as_view().into_iter().collect::>(), elements_are![eq(false), eq(true), eq(false)] ); @@ -141,7 +142,7 @@ fn test_repeated_enum_accessors() { elements_are![eq(NestedEnum::Bar), eq(NestedEnum::Baz), eq(NestedEnum::Foo)] ); assert_that!( - (*mutator).into_iter().collect::>(), + mutator.as_view().into_iter().collect::>(), elements_are![eq(NestedEnum::Bar), eq(NestedEnum::Baz), eq(NestedEnum::Foo)] ); @@ -160,7 +161,7 @@ fn test_repeated_bool_set() { for _ in 0..5 { mutator2.push(true); } - protobuf::MutProxy::set(&mut mutator, *mutator2); + protobuf::MutProxy::set(&mut mutator, mutator2.as_view()); assert_that!(mutator.iter().collect::>(), eq(mutator2.iter().collect::>())); }