Skip to content

Extra impl for (not-unsafe) Freeze version of MutexARC accessor methods #7473

New issue

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

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

Already on GitHub? # to your account

Closed
bblum opened this issue Jun 29, 2013 · 10 comments
Closed

Extra impl for (not-unsafe) Freeze version of MutexARC accessor methods #7473

bblum opened this issue Jun 29, 2013 · 10 comments
Labels
A-concurrency Area: Concurrency C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@bblum
Copy link
Contributor

bblum commented Jun 29, 2013

MutexARC and RWARC both make opposite choices for two different design features (safety-vs-nesting and rwlock-vs-mutex), excluding the other two combinations. We could permit the other combinations simply by adding extra impls with stub methods that called out to their counterpart.

@ghost ghost assigned bors and bblum Jun 29, 2013
@bblum
Copy link
Contributor Author

bblum commented Aug 19, 2013

Actually, there cannot be a non-freeze version of RWArc, even if it's labelled "unsafe" because of nesting. The entire point there is that the freeze requirement on RWArc contents enable the immutable read-mode. So, renaming this issue.

However, we could still have dual impls for the MutexArc, as the MutexArc only has a mutable access mode. A safe access mode for Send+Freeze MutexArc contents would be desirable to users who wish to have slightly better performance (mutexes are faster than rwlocks) and don't require a read mode.

I envision the current "access" and "access_cond" methods on MutexArc being changed to "unsafe_access" and "unsafe_access_cond", and having the prior names be used in the safe impl. (The constructors and unwrap method are safe even if the data doesn't freeze.)

Unassigning myself; this should be a pretty approachable code refactor for a newcomer.

@flaper87
Copy link
Contributor

I'd like to take this one!

@bblum
Copy link
Contributor Author

bblum commented Aug 20, 2013

OK

@flaper87
Copy link
Contributor

What would be the advantage of keeping unsafe_access and unsafe_access_cond ?

@bblum
Copy link
Contributor Author

bblum commented Aug 28, 2013

there should be two impls, one for each combination of kinds.

@flaper87
Copy link
Contributor

flaper87 commented Sep 3, 2013

r=?

@huonw
Copy link
Member

huonw commented Sep 3, 2013

@flaper87 have you opened a pull request with that commit?

@flaper87
Copy link
Contributor

flaper87 commented Sep 3, 2013

Just did! Damn, GH failed me before!

Thanks for the comment!

@flaper87
Copy link
Contributor

flaper87 commented Sep 3, 2013

r=?

bors added a commit that referenced this issue Sep 4, 2013
Current access methods are nestable and unsafe. This patch renames
current methods implementation - prepends unsafe_ - and implements 2 new
methods that are both safe and un-nestable.

Fixes #7473
@bors bors closed this as completed in 408367b Sep 4, 2013
@bblum
Copy link
Contributor Author

bblum commented Sep 4, 2013

nice job @flaper87

flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 19, 2021
Fix ICE in `is_integer_const`

fixes: rust-lang#7340
changelog: Fix ICE in `modulo_one` in const contexts
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 19, 2021
Rustup

r? `@ghost`

Out of cycle sync for 2 ICE fixes rust-lang#7470 rust-lang#7471 rust-lang#7473

changelog: none
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-concurrency Area: Concurrency C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants