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
Add a const-constructors for sync primitives #2790
Add a const-constructors for sync primitives #2790
Conversation
There are some CI tests that I don't believe will work, due to a using
I'm not sure how to specify the cfg this PR so that these changes are only available on nightly. |
Co-authored-by: Mikail Bagishov <bagishov.mikail@yandex.ru>
never mind, figured it out 😅 |
they require feature(const_fn) which is not available on stable.
a330a94
to
3d53e15
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'd really prefer not to add a nightly feature flag to Tokio. The problem with using a feature flag for nightly-only code is that another dependency could enable the feature flag, resulting it being enabled in the downstream application's build regardless of whether or not the nightly compiler is actually being used. This would result in a broken build.
For stuff like this, I think we'd prefer to add a RUSTFLAGS
cfg. Unlike a feature flag, these must be passed as an environment variable at build-time, and cannot be set in Cargo.toml
, so only the top level build can enable them. However, I'd rather not require nightly at all, and I think if we're willing to make a somewhat more substantial change, it's possible to do this without needing nightly.
I'd like to hear what other maintainers think about this, but that's my perspective.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
I'm not married to using a nightly feature flag or nightly at all for that matter so currently I'm open to any kind change :) As for moving forward with this, I'm not quite sure what the next steps are.. Currently I'm watching this thread waiting for other maintainers opinions before doing anything so I feel like this is just blocked still atm. |
Can you open a new PR that makes the change we described to |
@mental32 btw, feel free to ping me if you get stuck on the |
Sure thing! It may take a little while however so bear with me :) |
All ready 👍 only blocked on #2805 for the moment |
You should be able to merge in master now with the new |
251d910
to
687c687
Compare
687c687
to
39b8a30
Compare
I think everything's good to go here 😄 |
a800dc2
to
92dc9c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will merge it once CI passes.
Motivation
#2756 describes that the use of const-constructors with sync primitives and that there are currently no const-constructors for them.
Closes #2756
Solution
I've added a
const_new
public function to:loom::std::atomic_usize::AtomicUsize
loom::std::parking_lot::Mutex
sync::batch_semaphore::Semaphore
sync::mutex::Mutex
util::linked_list::LinkedList
All of the
const_new
constructors have the following cfg:As specified by the concern in this comment
Disclaimer
I have not modified or added any new tests as I'm not sure what needs to be done on the testing side for these changes. also since the cfg specifies that the constructors should not be present when
cfg(test)