This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Don't autodetect pthreads on MinGW
ClosedPublic

Authored by mstorsjo on Oct 1 2021, 2:35 PM.

Details

Reviewers
mati865
EricWF
ldionne
Group Reviewers
Restricted Project
Commits
rGa8d15a926689: [libcxx] Don't autodetect pthreads on MinGW
Summary

e9ee517930f76a972fcda00d9dd0466db80d60f7 added support for using
winpthreads on Windows, enabled if __WINPTHREADS_VERSION was
defined (i.e. if winpthreads headers have been included before
including libcxx __config). This was fragile (libcxx changed
behaviour depending on what headers had been included externally
before), and was changed in a1bc823a59d5b6f310bdf6c7c7b62ec71b87d1aa
to use pthreads on Windows whenever the pthread.h header was
available.

In practice, in every modern setup for building libcxx for Windows
I've seen, users end up manually configuring it with
LIBCXX_HAS_WIN32_THREAD_API=ON, as the users may have winpthreads
installed (for other libraries/projects to use) while wanting to
build libcxx with the default win32 threading.

Don't automatically pick up pthreads on Windows even if the header
is available. Instead require the user to configure the libcxx
build with LIBCXX_HAS_PTHREAD_API=ON if that's desired.

Event Timeline

mstorsjo requested review of this revision.Oct 1 2021, 2:35 PM
mstorsjo created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptOct 1 2021, 2:35 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
mati865 accepted this revision.EditedOct 2 2021, 2:18 AM

Agree, we should promote using working and tested configuration.
Even if pthread build works it's probably affected by threading issues when built with GCC.

Agree, we should promote using working and tested configuration.
Even if pthread build works it's probably affected by threading issues when built with GCC.

FWIW, the main use case for winpthreads in libc++ is to be able to run on versions older than Win7. That’s quite unsupported in libc++, but AFAIK it does work - as threading might be the only API used that requires a newer version.

Oh and another reason why the autodetection in the header is dangerous: If libc++ is built without pthread.h being available, but if pthread.h is available when the user includes libc++ headers, the user would end up with a bad mismatch.

ldionne accepted this revision.Oct 4 2021, 10:23 AM
This revision is now accepted and ready to land.Oct 4 2021, 10:23 AM
mstorsjo updated this revision to Diff 376994.Oct 4 2021, 12:23 PM

Retrigger CI

This revision was automatically updated to reflect the committed changes.