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

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

Merged
merged 8 commits into from
Jan 31, 2020
Merged

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

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 •
edited
Loading

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.

jonhoo
jonhoo reviewed Jan 29, 2020
View reviewed changes
src/map_ref.rs Show resolved Hide resolved
jonhoo
jonhoo reviewed Jan 29, 2020
View reviewed changes
src/map_ref.rs Show resolved Hide resolved
jonhoo
jonhoo reviewed Jan 29, 2020
View reviewed changes
src/map_ref.rs Outdated Show resolved Hide resolved
@jonhoo jonhoo mentioned this pull request Jan 29, 2020
Closed
@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.

@jonhoo jonhoo mentioned this pull request Jan 30, 2020
Closed
cuviper
cuviper commented Jan 30, 2020
View reviewed changes
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.

@jonhoo jonhoo mentioned this pull request Jan 30, 2020
Merged
@cuviper
Add a struct HashMapRef which holds a reference and guard
82363e9 
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
Add retain to HashMapRef
3f00f93
@cuviper
Add tests for HashMapRef
16b510a
@cuviper
Rename as_ref() to guarded() and add with_guard(&guard)
7be0533
@cuviper
Add HashMapRef::compute_if_present
8e6804d
@cuviper
Cut back HashMapRef docs
d5a7d57
@cuviper
Note that HashMapRef prevents garbage collection
be751ba
jonhoo
jonhoo reviewed Jan 31, 2020
View reviewed changes
src/map_ref.rs Outdated Show resolved Hide resolved
jonhoo
jonhoo reviewed Jan 31, 2020
View reviewed changes
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
Closed
@jonhoo jonhoo merged commit 120e06a into jonhoo:master Jan 31, 2020
@jonhoo
Copy link
Owner

jonhoo commented Jan 31, 2020

Fantastic, thank you!

@cuviper cuviper mentioned this pull request Jan 31, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonhoo jonhoo jonhoo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.1
Development

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