Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bytes const size borrowed arrays #325

Merged
merged 2 commits into from
Jun 14, 2021
Merged

Bytes const size borrowed arrays #325

merged 2 commits into from
Jun 14, 2021

Conversation

matix2267
Copy link
Contributor

Currently Bytes (added in #277) supports some const size array types (e.g. [u8; N] and Box<[u8; N]>)
but not others (e.g. &[u8; N] and Cow<'_, [u8; N]>).

This pull request implements Bytes support for &[u8; N] and Cow<'_, [u8; N]>
This bring parity between dynamic and const sized types.

Before:

Sizing Owned Borrowed Cow Box
Const [u8; N] &[u8; N] Cow<'_, [u8; N]> Box<[u8; N]>
Dynamic Vec<u8>* &[u8] Cow<'_, [u8]> Box<[u8]>

After:

Sizing Owned Borrowed Cow Box
Const [u8; N] &[u8; N] Cow<'_, [u8; N]> Box<[u8; N]>
Dynamic Vec<u8>* &[u8] Cow<'_, [u8]> Box<[u8]>

I've also added a separate commit with an example of using fully borrowed types, but that example only does serialization
because RON doesn't support 0-copy deserialization for byte arrays (as it needs to decode them from base64).
Because of that I'm not sure if this example is useful so feel free to merge only the main commit.

* Technically [u8] corresponds to [u8; N] but [u8] is an unSized type and cannot be
directly used in structs.

This bring parity between dynamic and const sized types.

### Before:

| Sizing  | Owned          | Borrowed      | Cow                   | Box               |
| ------- | -------------- | ------------- | --------------------- | ----------------- |
| Const   | ✅ `[u8; N]`   | ❌ `&[u8; N]` | ❌ `Cow<'_, [u8; N]>` | ✅ `Box<[u8; N]>` |
| Dynamic | ✅ `Vec<u8>`\* | ✅ `&[u8]`    | ✅ `Cow<'_, [u8]>`    | ✅ `Box<[u8]>`    |

### After:

| Sizing  | Owned          | Borrowed      | Cow                   | Box               |
| ------- | -------------- | ------------- | --------------------- | ----------------- |
| Const   | ✅ `[u8; N]`   | ✅ `&[u8; N]` | ✅ `Cow<'_, [u8; N]>` | ✅ `Box<[u8; N]>` |
| Dynamic | ✅ `Vec<u8>`\* | ✅ `&[u8]`    | ✅ `Cow<'_, [u8]>`    | ✅ `Box<[u8]>`    |

<sub>\* Technically `[u8]` corresponds to `[u8; N]` but `[u8]` is an un`Sized` type and cannot be
directly used in structs.</sub>
It only shows serialization as RON (which is used in these examples)
doesn't support 0-copy deserialization for byte arrays.
@codecov
Copy link

codecov bot commented Jun 14, 2021

Codecov Report

Merging #325 (3130c38) into master (bb1c80a) will decrease coverage by 1.54%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #325      +/-   ##
==========================================
- Coverage   79.39%   77.85%   -1.55%     
==========================================
  Files          45       45              
  Lines        2577     2628      +51     
==========================================
  Hits         2046     2046              
- Misses        531      582      +51     
Impacted Files Coverage Δ
src/de/const_arrays.rs 36.73% <0.00%> (-36.74%) ⬇️
src/lib.rs 75.00% <ø> (ø)
src/ser/const_arrays.rs 86.66% <0.00%> (-13.34%) ⬇️

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 bb1c80a...3130c38. Read the comment docs.

@jonasbb
Copy link
Owner

jonasbb commented Jun 14, 2021

Thank you. This definitely is an oversight of mine. For serialization &Bytes should work. I'm unsure if that would support 0-copy deserialization, though. The missing implementation on Cow<'_, [u8; N]> is a problem :)
It is great that you added the extra documentation.
I can create a new bugfix release later with those changes.

@jonasbb
Copy link
Owner

jonasbb commented Jun 14, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 14, 2021

Build succeeded:

@bors bors bot merged commit 4f2cbae into jonasbb:master Jun 14, 2021
@matix2267 matix2267 deleted the bytes-const-size-borrowed-arrays branch June 14, 2021 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants