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.

Sign up for GitHub

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

Jump to bottom

Upgrade to Bytes 0.0.6 #72

Closed
wants to merge 1 commit into from
Closed

Upgrade to Bytes 0.0.6 #72

wants to merge 1 commit into from

Conversation

benbromhead
Copy link

@benbromhead benbromhead commented Nov 16, 2020 •
edited
Loading

Quick update for evmap to use latest version of bytes

cargo test makes this look simple enough

This change is Reviewable

@benbromhead
Update Cargo.toml
cd55c78 
Update evmap to use latest version of bytes
@codecov
Copy link

codecov bot commented Nov 16, 2020

Codecov Report

Merging #72 (cd55c78) into master (d4c12b8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #72   +/-   ##
=======================================
  Coverage   83.06%   83.06%           
=======================================
  Files          12       12           
  Lines        1234     1234           
=======================================
  Hits         1025     1025           
  Misses        209      209

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 d4c12b8...cd55c78. Read the comment docs.

@jonhoo
Copy link
Owner

jonhoo commented Nov 17, 2020

Thank you! Sadly, this is a breaking change, so I'll have to hold off on merging this until the next major release. I think that'll come in not too long though :)

@benbromhead
Copy link
Author

All good I'd just left the version as version = "11.0.0-alpha.1" based on master, but yeah its def a major version change.

I ended up moving my other deps back to bytes=0.5 anyway (:

jonhoo added a commit that referenced this pull request Nov 30, 2020
@jonhoo
Remove bytes with ShallowCopy removal
b6c7a31 
We no longer need to implement anything for `Bytes`!
Fixes #72.
@jonhoo
Copy link
Owner

jonhoo commented Nov 30, 2020

Heads up that this will be fixed by #83, since it no longer uses the ShallowCopy trait, and thus we don't need to implement anything for Bytes at all!

jonhoo added a commit that referenced this pull request Dec 6, 2020
@jonhoo
Implement sound map value aliasing
f53e0ee 
This implementation gets rid of the unsound `ShallowCopy` trait (#74 &
rust-lang/unsafe-code-guidelines#35 (comment)),
and replaces it with a wrapper type around aliased values.

The core mechanism is that the wrapper type holds a `MaybeUninit<T>`,
and aliases it by doing a `ptr::read` of the whole `MaybeUninit<T>` to
alias. It then takes care to only give out `&T`s, which is allowed to
alias.

To drop, the implementation casts between two different generic
arguments of the new `Aliased` type, where one type causes the
`Aliased<T>` to drop the inner `T` and the other does not. The
documentation goes into more detail. The resulting cast _should_ be safe
(unlike the old `ManuallyDrop` cast).

The removal of `ShallowCopy` makes some of the API nicer by removing
trait bounds, and obviates the need for `evmap-derive`. While I was
going through, I also took the liberty of tidying up the external API of
`evmap` a bit.

The implementation passes all tests, and I _think_ it is sound (if you
think it's not, please let me know!).

Note that this does _not_ take care of #78.

Fixes #74.
Fixes #55 since `ShallowCopy` is no longer neeeded.
Also fixes #72 for the same reason.
@jonhoo jonhoo closed this in 416ccef Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@benbromhead @jonhoo