Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Added MutableArray::as_box #450

Merged
merged 1 commit into from Sep 26, 2021

Conversation

sd2k
Copy link
Contributor

@sd2k sd2k commented Sep 25, 2021

This basically just changes the impls of as_arc to as_box and uses
the From<Box<T>> for Arc<T> impl to provide a default implementation
for as_arc.

Closes #447.

@sd2k
Copy link
Contributor Author

sd2k commented Sep 25, 2021

Just wondering, is there a reason that the as_arc method wasn't a (consuming) into_arc method?

@sd2k sd2k changed the title Add MutableArray::as_box Added MutableArray::as_box Sep 25, 2021
Copy link
Owner

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR 🙇 Looks great and ready to go.

Just wondering, is there a reason that the as_arc method wasn't a (consuming) into_arc method?

Great question :)

For a trait to be a trait object, it cannot be Sized. Any method that consumes self (and not &mut self) makes the trait automatically Sized (because fn a(self) requires knowing the size of Self to make it to a registry). So, we either have into_arc(self) and not a trait object, or a trait object and as_arc(&mut self). Since we really need the trait object here, we make it a trait object (and use as_arc).

But yes, you are right that into_arc is what makes sense here.

src/array/mod.rs Show resolved Hide resolved
@sd2k
Copy link
Contributor Author

sd2k commented Sep 25, 2021

Ah of course, I'd forgotten we needed object safety 😅 Kind of fundamental hehe. Thanks!

@codecov
Copy link

codecov bot commented Sep 25, 2021

Codecov Report

Merging #450 (f5dbdb8) into main (885e6c1) will decrease coverage by 0.28%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #450      +/-   ##
==========================================
- Coverage   80.77%   80.49%   -0.29%     
==========================================
  Files         372      372              
  Lines       22637    22735      +98     
==========================================
+ Hits        18286    18300      +14     
- Misses       4351     4435      +84     
Impacted Files Coverage Δ
src/array/binary/mutable.rs 63.35% <0.00%> (-2.06%) ⬇️
src/array/boolean/mutable.rs 85.23% <0.00%> (-2.96%) ⬇️
src/array/dictionary/mutable.rs 76.66% <0.00%> (-5.48%) ⬇️
src/array/fixed_size_binary/mutable.rs 71.76% <0.00%> (-4.49%) ⬇️
src/array/fixed_size_list/mutable.rs 36.84% <0.00%> (-3.55%) ⬇️
src/array/list/mutable.rs 70.96% <0.00%> (-4.90%) ⬇️
src/array/mod.rs 54.62% <0.00%> (-4.36%) ⬇️
src/array/primitive/mutable.rs 86.19% <0.00%> (-2.11%) ⬇️
src/array/utf8/mutable.rs 87.70% <0.00%> (-2.22%) ⬇️
src/array/dictionary/mod.rs 44.82% <0.00%> (-14.00%) ⬇️
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 885e6c1...f5dbdb8. Read the comment docs.

@jorgecarleitao jorgecarleitao added the enhancement An improvement to an existing feature label Sep 26, 2021
@jorgecarleitao jorgecarleitao merged commit f761d41 into jorgecarleitao:main Sep 26, 2021
@sd2k sd2k deleted the add-as-box-to-mutablearray branch September 26, 2021 18:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement An improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add as_boxed method to MutableArray
2 participants