[libcxx-commits] [PATCH] D58201: Make std::memory_order an enum class (P0439R0)

[JF Bastien via Phabricator via libcxx-commits]

jfb marked an inline comment as done.
jfb added inline comments.


================
Comment at: include/atomic:603
+
+#endif
 
----------------
jwakely wrote:
> jfb wrote:
> > I think you want to keep the old `typedef enum memory_order` before C++20, and only enable the new thing in C++20 and later.
> That's what we did for libstdc++.
You did what I'm suggesting, or what @zoecarver did? 🤔


================
Comment at: include/atomic:932
       _LIBCPP_CHECK_STORE_MEMORY_ORDER(__m)
-        {__c11_atomic_store(&__a_, __d, __m);}
+        {__c11_atomic_store(&__a_, __d, static_cast<int>(__m));}
     _LIBCPP_INLINE_VISIBILITY
----------------
jwakely wrote:
> jfb wrote:
> > Hmm this is unfortunate. Can we change the builtins to accept the enum? I guess that would make mixing compiler / stdlib versions harder... So I think what you did here is best.
> And we also use casts to `int` in libstdc++. Changing the builtins wouldn't help if somebody used new libstdc++ headers with an old Clang or icc that only accepts ints.
Right, it makes me slightly sad at how ugly the code is, but it was already ugly to start with 😉


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58201/new/

https://reviews.llvm.org/D58201