Avoid std::atomic_init deprecated by P0883.#140
Avoid std::atomic_init deprecated by P0883.#140lewissbaker merged 2 commits intolewissbaker:masterfrom BillyONeal:remove_atomic_init
Conversation
The build system is currently only looking for VS2017 compilers. |
lewissbaker
left a comment
There was a problem hiding this comment.
Thanks for the PR!
If you can fix up the multi_producer_sequencer constructor it should be good to merge.
| constexpr unsigned_diff_t maxSize = static_cast<unsigned_diff_t>(std::numeric_limits<diff_t>::max()); | ||
| assert(bufferSize <= maxSize); | ||
|
|
||
| #ifndef __cpp_lib_atomic_value_initialization |
There was a problem hiding this comment.
This loop still needs to run to initialise the elements of the array.
Each element is initialised to a different value (notice that seq is incremented inside the while condition).
Maybe something like this:
SEQUENCE seq = initialSequence - (bufferSize - 1);
do
{
+#if __cpp_lib_atomic_value_initialization
+ m_published[seq & m_sequenceMask].store(seq, std::memory_order_relaxed);
+#else
std::atomic_init(&m_published[seq & m_sequenceMask], seq);
+#endif
} while (seq++ != initialSequence);There was a problem hiding this comment.
Fixed up, thank you!
Right, I put the VS2017 compiler on the PATH. (That's the
Derp, stand by. |
There was a problem hiding this comment.
But I guess it might be looking in the 2017 installation directory or something like that.
It uses vswhere to find the install-location of msvc.
I try to avoid relying on environment variables to help make the build a bit more repeatable.
You can modify the config.cake file to set the vcInstallDir variable explicitly as a last resort (it's just a Python script). I should probably add a command-line flag to allow setting this, as it does for clang.
|
FYI, this ran against our compiler nightly builds last night and passed. |
Resolves #139
I don't think I've been successful in actually getting this to build; I apologize if there are typos here: