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

Making RWLock::new const fn #66823

Closed
wants to merge 1 commit into from

Conversation

elichai
Copy link
Contributor

@elichai elichai commented Nov 27, 2019

Related #66806
The only problem I saw was the fact that sys::RWLock was on the heap, but I couldn't find any reason why so I removed the Box.

If there's a reason why it was boxed that I missed please tell me.

I also didn't see any good reason to put this behind a feature, so if this will be accepted as is it will be instant stable.

@rust-highfive
Copy link
Collaborator

r? @KodrAus

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 27, 2019
@Mark-Simulacrum
Copy link
Member

r? @alexcrichton

But I believe we can't remove the box here -- there's a comment on the mutex impl that indicates that the underlying system primitives can't move after creation (https://github.com/rust-lang/rust/blob/master/src/libstd/sync/mutex.rs#L112-L116) -- I suspect the same is true of rwlock, we just aren't documenting that today (though probably should do so).

@alexcrichton
Copy link
Member

Thanks for the PR! @Mark-Simulacrum is right though in that the system primitives are required to be boxed up here for safety, so technically exposing the underlying API as safe is actually incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants