Skip to content

Commit

Permalink
Remove superfluous blanket impls in our logging traits (#8605)
Browse files Browse the repository at this point in the history
Our core logging traits have to be object-safe, which means `Self` must
be unsized, which means all methods must take either a shared or
exclusive reference.
This in turns means that any `impl Trait for &something` is
non-sensical, since this lowers to `impl Trait for &&self`, which just
gives extra work to the compiler for no reason since this is
semantically identical to the builtin autoderef magic.
  • Loading branch information
teh-cmc authored Jan 7, 2025
1 parent 8f3bbc2 commit ebfb6fb
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 79 deletions.
49 changes: 9 additions & 40 deletions crates/store/re_types_core/src/as_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ pub trait AsComponents {
}
}

#[allow(dead_code)]
fn assert_object_safe() {
let _: &dyn AsComponents;
}

impl AsComponents for dyn ComponentBatch {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
Expand All @@ -121,24 +126,6 @@ impl<const N: usize> AsComponents for [Box<dyn ComponentBatch>; N] {
}
}

impl AsComponents for &[&dyn ComponentBatch] {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
self.iter()
.map(|batch| ComponentBatchCowWithDescriptor::new(*batch))
.collect()
}
}

impl AsComponents for &[Box<dyn ComponentBatch>] {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
self.iter()
.map(|batch| ComponentBatchCowWithDescriptor::new(&**batch))
.collect()
}
}

impl AsComponents for Vec<&dyn ComponentBatch> {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
Expand All @@ -157,7 +144,7 @@ impl AsComponents for Vec<Box<dyn ComponentBatch>> {
}
}

impl<AS: AsComponents, const N: usize> AsComponents for [AS; N] {
impl<AS: AsComponents> AsComponents for [AS] {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
self.iter()
Expand All @@ -166,25 +153,7 @@ impl<AS: AsComponents, const N: usize> AsComponents for [AS; N] {
}
}

impl<const N: usize> AsComponents for [&dyn AsComponents; N] {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
self.iter()
.flat_map(|as_components| as_components.as_component_batches())
.collect()
}
}

impl<const N: usize> AsComponents for [Box<dyn AsComponents>; N] {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
self.iter()
.flat_map(|as_components| as_components.as_component_batches())
.collect()
}
}

impl<AS: AsComponents> AsComponents for &[AS] {
impl<AS: AsComponents, const N: usize> AsComponents for [AS; N] {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
self.iter()
Expand All @@ -193,7 +162,7 @@ impl<AS: AsComponents> AsComponents for &[AS] {
}
}

impl AsComponents for &[&dyn AsComponents] {
impl<const N: usize> AsComponents for [&dyn AsComponents; N] {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
self.iter()
Expand All @@ -202,7 +171,7 @@ impl AsComponents for &[&dyn AsComponents] {
}
}

impl AsComponents for &[Box<dyn AsComponents>] {
impl<const N: usize> AsComponents for [Box<dyn AsComponents>; N] {
#[inline]
fn as_component_batches(&self) -> Vec<ComponentBatchCowWithDescriptor<'_>> {
self.iter()
Expand Down
53 changes: 14 additions & 39 deletions crates/store/re_types_core/src/loggable_batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ pub trait LoggableBatch {
fn to_arrow2(&self) -> SerializationResult<Box<dyn ::arrow2::array::Array>>;
}

#[allow(dead_code)]
fn assert_loggablebatch_object_safe() {
let _: &dyn LoggableBatch;
}

/// A [`ComponentBatch`] represents an array's worth of [`Component`] instances.
pub trait ComponentBatch: LoggableBatch {
/// Serializes the batch into an Arrow list array with a single component per list.
Expand Down Expand Up @@ -75,6 +80,11 @@ pub trait ComponentBatch: LoggableBatch {
}
}

#[allow(dead_code)]
fn assert_component_batch_object_safe() {
let _: &dyn LoggableBatch;
}

/// Some [`ComponentBatch`], optionally with an overridden [`ComponentDescriptor`].
///
/// Used by implementers of [`crate::AsComponents`] to both efficiently expose their component data
Expand Down Expand Up @@ -287,14 +297,14 @@ impl<C: Component, const N: usize> ComponentBatch for [Option<C>; N] {

// --- Slice ---

impl<L: Loggable> LoggableBatch for &[L] {
impl<L: Loggable> LoggableBatch for [L] {
#[inline]
fn to_arrow2(&self) -> SerializationResult<Box<dyn ::arrow2::array::Array>> {
L::to_arrow2(self.iter().map(|v| std::borrow::Cow::Borrowed(v)))
}
}

impl<C: Component> ComponentBatch for &[C] {
impl<C: Component> ComponentBatch for [C] {
#[inline]
fn descriptor(&self) -> Cow<'_, ComponentDescriptor> {
C::descriptor().into()
Expand All @@ -303,42 +313,7 @@ impl<C: Component> ComponentBatch for &[C] {

// --- Slice<Option> ---

impl<L: Loggable> LoggableBatch for &[Option<L>] {
#[inline]
fn to_arrow2(&self) -> SerializationResult<Box<dyn ::arrow2::array::Array>> {
L::to_arrow2_opt(
self.iter()
.map(|opt| opt.as_ref().map(|v| std::borrow::Cow::Borrowed(v))),
)
}
}

impl<C: Component> ComponentBatch for &[Option<C>] {
#[inline]
fn descriptor(&self) -> Cow<'_, ComponentDescriptor> {
C::descriptor().into()
}
}

// --- ArrayRef ---

impl<L: Loggable, const N: usize> LoggableBatch for &[L; N] {
#[inline]
fn to_arrow2(&self) -> SerializationResult<Box<dyn ::arrow2::array::Array>> {
L::to_arrow2(self.iter().map(|v| std::borrow::Cow::Borrowed(v)))
}
}

impl<C: Component, const N: usize> ComponentBatch for &[C; N] {
#[inline]
fn descriptor(&self) -> Cow<'_, ComponentDescriptor> {
C::descriptor().into()
}
}

// --- ArrayRef<Option> ---

impl<L: Loggable, const N: usize> LoggableBatch for &[Option<L>; N] {
impl<L: Loggable> LoggableBatch for [Option<L>] {
#[inline]
fn to_arrow2(&self) -> SerializationResult<Box<dyn ::arrow2::array::Array>> {
L::to_arrow2_opt(
Expand All @@ -348,7 +323,7 @@ impl<L: Loggable, const N: usize> LoggableBatch for &[Option<L>; N] {
}
}

impl<C: Component, const N: usize> ComponentBatch for &[Option<C>; N] {
impl<C: Component> ComponentBatch for [Option<C>] {
#[inline]
fn descriptor(&self) -> Cow<'_, ComponentDescriptor> {
C::descriptor().into()
Expand Down

0 comments on commit ebfb6fb

Please sign in to comment.