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

Make TryLock::new const #7

Closed
wants to merge 1 commit into from
Closed

Conversation

Chris--B
Copy link

I was playing with allocators and wanted a lightweight lock to protect some state. I found this crate, works great. Only complaint is that to initialize a GlobalAllocator, I needed ::new() to be const. I made a small change here and it works!

TryLock::new calls two other functions, but is otherwise already const-able.

Since both functions have been const for a long time, I would be surprised if this change breaks anyone's code. Most projects track the latest stable Rust, so in my opinion this is fine.

I also tried consting other functions, but it does not work on stable. The big blocker are the panic!() calls in TryLock::try_lock_explicit_unchecked. I didn't need anything else to be const, so I left everything else alone.

@Chris--B
Copy link
Author

Well, I see 1.21 in the PR checks, so I can already tell this will fail that build. This change bumps the minimum needed to 1.32, from January 2019.

@Chris--B
Copy link
Author

Oh, this is embarrassing. I thought I checked but completely missed PR #2! They hit on the same issues and even update the tests & code.

@Chris--B
Copy link
Author

Chris--B commented Dec 3, 2021

I also tried consting other functions, but it does not work on stable. The big blocker are the panic!() calls in TryLock::try_lock_explicit_unchecked.

This has landed in stable, without formatting. So more of these might be const-able now or soon.

@Finomnis
Copy link

Finomnis commented Dec 4, 2023

If this PR is a duplicate, then it might be a good idea to close it.

@seanmonstar
Copy link
Owner

Done in #2

@seanmonstar seanmonstar closed this Dec 7, 2023
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.

3 participants