This is an archive of the discontinued LLVM Phabricator instance.

Differential D92349

[libcxx] remove checks for __STDCPP_THREADS__ as it is defined by compiler
ClosedPublic

Authored by zequanwu on Nov 30 2020, 2:09 PM.

Details

Reviewers
rnk
alanphipps
dmajor
Group Reviewers
Restricted Project
Commits
rG871f96eed379: [libcxx] remove checks for __STDCPP_THREADS__ as it is defined by compiler

Diff Detail

Unit TestsFailed

TimeTest
980 mslibcxx CI -fno-exceptions > libc++.std/thread::macro.pass.cpp
1,340 mslibcxx CI ASAN > libc++.std/thread::macro.pass.cpp
710 mslibcxx CI Apple back-deployment macosx10.9 > libc++.std/thread::macro.pass.cpp
780 mslibcxx CI Apple system > libc++.std/thread::macro.pass.cpp
1,170 mslibcxx CI Apple system -fno-exceptions > libc++.std/thread::macro.pass.cpp
View Full Test Results (18 Failed)

Event Timeline

zequanwu created this revision.Nov 30 2020, 2:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 30 2020, 2:09 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
zequanwu requested review of this revision.Nov 30 2020, 2:09 PM
rnk added inline comments.Nov 30 2020, 2:42 PM
libcxx/include/__config
1159

Let's continue defining __STDCPP_THREADS__ if it's not defined. It's safe. It makes new libc++ work with old clang.

zequanwu updated this revision to Diff 308484.Nov 30 2020, 2:45 PM
zequanwu marked an inline comment as done.

address comment.

Harbormaster completed remote builds in B80581: Diff 308478.Nov 30 2020, 3:02 PM
Harbormaster completed remote builds in B80585: Diff 308484.Nov 30 2020, 3:45 PM
rnk accepted this revision.Nov 30 2020, 4:19 PM

Looks good, but change the commit message: you are changing libc++, not libc++abi.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 30 2020, 4:37 PM
Closed by commit rG871f96eed379: [libcxx] remove checks for __STDCPP_THREADS__ as it is defined by compiler (authored by zequanwu). · Explain Why
This revision was automatically updated to reflect the committed changes.
zequanwu added a commit: rG871f96eed379: [libcxx] remove checks for __STDCPP_THREADS__ as it is defined by compiler.
zequanwu retitled this revision from [libcxxabi] remove checks for __STDCPP_THREADS__ as it is defined by compiler to [libcxx] remove checks for __STDCPP_THREADS__ as it is defined by compiler.Nov 30 2020, 4:37 PM
thakis added a subscriber: thakis.Nov 30 2020, 4:40 PM

(See also https://reviews.llvm.org/D65370 for prior discussion.)

dmajor mentioned this in D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros.Nov 30 2020, 6:01 PM
ldionne added a subscriber: ldionne.Dec 1 2020, 8:31 AM

@zequanwu In the future, please refrain from committing to libc++ or libc++abi unless you have approval of the libcxx or libcxxabi review group. There's a reason why that group is added as a "blocking reviewer", it's because you should wait for it to be green before proceeding.

zequanwu added a comment.Dec 1 2020, 10:31 AM
In D92349#2425794, @ldionne wrote:

@zequanwu In the future, please refrain from committing to libc++ or libc++abi unless you have approval of the libcxx or libcxxabi review group. There's a reason why that group is added as a "blocking reviewer", it's because you should wait for it to be green before proceeding.

Okay.

Revision Contents

PathSize
libcxx/
include/
8 lines

Diff 308478

libcxx/include/__config

Show First 20 Lines • Show All 1,146 Lines • ▼ Show 20 Lines#error _LIBCPP_HAS_THREAD_API_EXTERNAL may not be defined when \
_LIBCPP_HAS_NO_THREADS is defined. _LIBCPP_HAS_NO_THREADS is defined.
#endif #endif
#if defined(_LIBCPP_HAS_NO_MONOTONIC_CLOCK) && !defined(_LIBCPP_HAS_NO_THREADS) #if defined(_LIBCPP_HAS_NO_MONOTONIC_CLOCK) && !defined(_LIBCPP_HAS_NO_THREADS)
#error _LIBCPP_HAS_NO_MONOTONIC_CLOCK may only be defined when \ #error _LIBCPP_HAS_NO_MONOTONIC_CLOCK may only be defined when \
_LIBCPP_HAS_NO_THREADS is defined. _LIBCPP_HAS_NO_THREADS is defined.
#endif #endif
#if defined(__STDCPP_THREADS__) && defined(_LIBCPP_HAS_NO_THREADS)
#error _LIBCPP_HAS_NO_THREADS cannot be set when __STDCPP_THREADS__ is set.
#endif
#if !defined(_LIBCPP_HAS_NO_THREADS) && !defined(__STDCPP_THREADS__)
rnkUnsubmitted
Done
ReplyInline Actions

Let's continue defining __STDCPP_THREADS__ if it's not defined. It's safe. It makes new libc++ work with old clang.

rnk: Let's continue defining `__STDCPP_THREADS__` if it's not defined. It's safe. It makes new…
#define __STDCPP_THREADS__ 1
#endif
// The glibc and Bionic implementation of pthreads implements // The glibc and Bionic implementation of pthreads implements
// pthread_mutex_destroy as nop for regular mutexes. Additionally, Win32 // pthread_mutex_destroy as nop for regular mutexes. Additionally, Win32
// mutexes have no destroy mechanism. // mutexes have no destroy mechanism.
// //
// This optimization can't be performed on Apple platforms, where // This optimization can't be performed on Apple platforms, where
// pthread_mutex_destroy can allow the kernel to release resources. // pthread_mutex_destroy can allow the kernel to release resources.
// See https://llvm.org/D64298 for details. // See https://llvm.org/D64298 for details.
// //
▲ Show 20 Lines • Show All 260 Lines • Show Last 20 Lines