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

Add a struct HashMapRef which holds a reference and guard #45

Merged
merged 8 commits into from
Jan 31, 2020

Conversation

cuviper
Copy link
Collaborator

@cuviper cuviper commented Jan 29, 2020

This is created by HashMap::as_ref(&self), and it implements most of
the same public API, just with implicit epoch guards. Additionally, it
implements IntoIterator by reference, and a new trick -- Index!

@cuviper
Copy link
Collaborator Author

cuviper commented Jan 29, 2020

it implements most of the same public API

Obvious absences are all of the HashMap constructors. In addition, I didn't add retain and retain_force because those take &mut self instead of &self -- was that intentional? They talk about concurrent access, but &mut prevents that entirely.

I also need to add tests, but I wanted to float the idea in a PR before I continue.

@codecov
Copy link

codecov bot commented Jan 29, 2020

Codecov Report

Merging #45 into master will increase coverage by 2.19%.
The diff coverage is 71.18%.

Impacted Files Coverage Δ
src/map.rs 87.34% <100%> (+1.13%) ⬆️
src/map_ref.rs 66.66% <66.66%> (ø)
src/raw/mod.rs 93.02% <0%> (+6.97%) ⬆️
src/node.rs 94.5% <0%> (+49.05%) ⬆️

@jonhoo
Copy link
Owner

jonhoo commented Jan 29, 2020

Ah, that is just beautiful!

No, &mut self for retain was not intentional. Feel free to fix in this PR.

src/map_ref.rs Show resolved Hide resolved
src/map_ref.rs Show resolved Hide resolved
src/map_ref.rs Outdated Show resolved Hide resolved
@jonhoo
Copy link
Owner

jonhoo commented Jan 30, 2020

I pushed a fix for retain taking &mut to master.

Also, #44 just added explicit guards to a bunch more methods, which may be a good precursor to the work here.

tests/basic_ref.rs Outdated Show resolved Hide resolved
@cuviper
Copy link
Collaborator Author

cuviper commented Jan 30, 2020

Hmm, maybe I should just cross-link docs, instead of duplicating them...

@jonhoo
Copy link
Owner

jonhoo commented Jan 30, 2020

Yeah, I think that's probably fine, given that they are "feed-through" anyway. I wish there was a way to #[doc(include(path::to::type::or::method))].

@jonhoo
Copy link
Owner

jonhoo commented Jan 30, 2020

Oh, also, you may want to rebase on/merge master given that I merged #47 which included a bunch more guarded methods.

This is created by `HashMap::as_ref(&self)`, and it implements most of
the same public API, just with implicit epoch guards. Additionally, it
implements `IntoIterator` by reference, and a new trick -- `Index`!
src/map_ref.rs Outdated Show resolved Hide resolved
src/map_ref.rs Show resolved Hide resolved
@jonhoo jonhoo added this to the 0.1 milestone Jan 31, 2020
@jonhoo jonhoo mentioned this pull request Jan 31, 2020
@jonhoo jonhoo merged commit 120e06a into jonhoo:master Jan 31, 2020
@jonhoo
Copy link
Owner

jonhoo commented Jan 31, 2020

Fantastic, thank you!

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.

None yet

2 participants