This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Fix <atomic> failures on GCC
ClosedPublic

Authored by ldionne on Mar 5 2019, 7:35 AM.

Details

Summary

In https://reviews.llvm.org/D58201, we turned memory_order into an enum
class in C++20 mode. However, we were not casting memory_order to its
underlying type correctly for the GCC implementation, which broke the
build bots. I also fixed a test that was failing in C++17 mode on GCC 5.

Diff Detail

Repository
rL LLVM

Event Timeline

ldionne created this revision.Mar 5 2019, 7:35 AM
Herald added a project: Restricted Project. · View Herald Transcript
ldionne changed the repository for this revision from rC Clang to rCXX libc++.Mar 5 2019, 7:44 AM
ldionne removed a project: Restricted Project.
ldionne edited subscribers, added: libcxx-commits; removed: cfe-commits.

Will commit to fix the build, please comment if you would rather fix the problem differently.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 5 2019, 7:49 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2019, 7:49 AM

@ldionne Sorry for my delayed reply. Looks good to me.

libcxx/trunk/include/atomic
1458

Some reason these arent static_casted?

ldionne marked an inline comment as done.Mar 5 2019, 8:05 AM
ldionne added inline comments.
libcxx/trunk/include/atomic
1458

Yes -- all the __cxx_atomic_foo functions take a memory_order argument, not an int.

zoecarver added inline comments.Mar 5 2019, 8:06 AM
libcxx/trunk/include/atomic
1458

Oh, my bad. How did this pass the tests then..?

ldionne marked an inline comment as done.Mar 5 2019, 10:54 AM
ldionne added inline comments.
libcxx/trunk/include/atomic
1458

The mismatch was only under the GCC implementation.

zoecarver added inline comments.Mar 5 2019, 10:55 AM
libcxx/trunk/include/atomic
1458

Ah, I see, thanks for the clarification.