Skip to content

Use of raw RWLock instead of RwLock #60576

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
hellow554 opened this issue May 6, 2019 · 5 comments
Closed

Use of raw RWLock instead of RwLock #60576

hellow554 opened this issue May 6, 2019 · 5 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@hellow554
Copy link
Contributor

hellow554 commented May 6, 2019

static HOOK_LOCK: RWLock = RWLock::new();
static mut HOOK: Hook = Hook::Default;

looking at these two lines of code is very frightening.
Is there a specific reason to use RWLock instead of RwLock? I would open a PR with either using RwLock or a comment describing why RWLock is needed here.

@rustbot modify labels: T-libs

@jonas-schievink jonas-schievink added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels May 6, 2019
@Lonami
Copy link
Contributor

Lonami commented May 6, 2019

Just another thought, isn't the naming itself a bit dangerous? RW vs. Rw seems like an easy typo to make. Perhaps renaming RW as RawRw could help?

@sfackler
Copy link
Member

sfackler commented May 6, 2019

RwLock can't be statically initialized, or at least couldn't be when that code was written. That'll change once the parking_lot PR lands.

@hellow554
Copy link
Contributor Author

hellow554 commented May 25, 2021

Hey @sfackler
has the parking_lot PR already landed? Can you maybe tell the PR number? I would like to give it a shot to clean this up.

@mati865
Copy link
Member

mati865 commented May 25, 2021

It was not merged: #56410 (comment)

@hellow554
Copy link
Contributor Author

hellow554 commented May 25, 2021

I will close this issue as I don't see that this can and will be addressed any time soon. Thank you for your answer mati :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants