This is an archive of the discontinued LLVM Phabricator instance.

Differential D54338

Fix threads build on GNU/Hurd
ClosedPublic

Authored by sthibaul on Nov 9 2018, 11:24 AM.

Details

Reviewers
EricWF
howard.hinnant
joerg
krytarowski
ldionne
Commits
rL346763: [libcxx] GNU/Hurd uses BSD-based interfaces, but does not (and won't) provide…
Summary

GNU/Hurd uses BSD-based interfaces, but does not (and won't) provide <sys/sysctl.h>

Diff Detail

Repository
rCXX libc++

Event Timeline

sthibaul created this revision.Nov 9 2018, 11:24 AM
Herald added subscribers: libcxx-commits, ldionne, krytarowski. · View Herald TranscriptNov 9 2018, 11:24 AM
sthibaul added reviewers: EricWF, howard.hinnant, joerg.Nov 9 2018, 11:26 AM
joerg added a comment.Nov 9 2018, 2:13 PM

I think I would enumerate the BSDs here (and Apple) explicitly and not depend on BSD.

krytarowski added a comment.Nov 9 2018, 2:20 PM

Yes, please iterate over explicit BSDs. We don't support BSD4.4 or BSD/OS or such.

sthibaul updated this revision to Diff 173502.Nov 10 2018, 2:36 AM

So that'd be it? I don't know if libcxx supports more than free/net/open bsd and apple ?

krytarowski added a comment.Nov 10 2018, 4:04 AM
In D54338#1294141, @sthibaul wrote:

So that'd be it? I don't know if libcxx supports more than free/net/open bsd and apple ?

There is also __DragonFly__.

src/thread.cpp
24

Please fix this line too.

sthibaul updated this revision to Diff 173509.Nov 10 2018, 4:42 AM
krytarowski added a comment.Nov 10 2018, 1:49 PM

Please replace defined(BSD).

sthibaul added a comment.Nov 10 2018, 1:51 PM

Well, that's what I did the in the latest version?

krytarowski added a comment.Nov 10 2018, 1:52 PM

It's not complete, there is remaining L24.

sthibaul updated this revision to Diff 173529.Nov 10 2018, 1:56 PM

Ah, right, sorry, done so now.

krytarowski accepted this revision.Nov 10 2018, 2:58 PM
This revision is now accepted and ready to land.Nov 10 2018, 2:58 PM
sthibaul marked an inline comment as done.Nov 11 2018, 2:17 PM

(I can't commit this myself)

ldionne requested changes to this revision.Nov 11 2018, 7:12 PM

I’m OK with the change but please don’t repeat the condition on L24.

Re-upload and I will commit.

src/thread.cpp
24

I really don’t feel like it is useful to repeat the condition given the #if spans only one line.

This revision now requires changes to proceed.Nov 11 2018, 7:12 PM
kristina added a subscriber: kristina.EditedNov 11 2018, 9:04 PM

Doesn't __APPLE__ imply __MACH__ anyway outside of targets that are way out of scope of libc++? Since IIRC libc++ only runs on Darwin and only in usermode, which has the __APPLE__ predefined macro even for stuff like PureDarwin. XNU itself lacks a real libc++, and so do other non-usermode things (providing either simple stubs or very minimal more suited implementations).

sthibaul updated this revision to Diff 173629.Nov 12 2018, 1:34 AM

I have dropped the endif comment as requested.

sthibaul added a comment.Nov 12 2018, 1:36 AM

Doesn't APPLE imply MACH anyway outside of targets that are way out of scope of libc++?

Well, possibly, it is just not directly related to my patch :)
(also it makes sense to keep it at least as documenting that it's the mach-powered apple OS that provides sys/param.h + sys/sysctl.h).

ldionne accepted this revision.Nov 13 2018, 9:02 AM

Committed as r346763.

This revision is now accepted and ready to land.Nov 13 2018, 9:02 AM
krytarowski closed this revision.Nov 13 2018, 9:46 AM
kristina added a commit: rL346763: [libcxx] GNU/Hurd uses BSD-based interfaces, but does not (and won't) provide….Nov 13 2018, 11:50 PM

Revision Contents

PathSize
src/
2 lines

Diff 173393

src/thread.cpp

Show All 13 Lines
#include "exception" #include "exception"
#include "vector" #include "vector"
#include "future" #include "future"
#include "limits" #include "limits"
#include <sys/types.h> #include <sys/types.h>
#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
# include <sys/param.h> # include <sys/param.h>
# if defined(BSD) # if defined(BSD) && !defined(__GNU__)
# include <sys/sysctl.h> # include <sys/sysctl.h>
# endif // defined(BSD) # endif // defined(BSD)
krytarowskiUnsubmitted
Done
ReplyInline Actions

Please fix this line too.

krytarowski: Please fix this line too.
ldionneUnsubmitted
Not Done
ReplyInline Actions

I really don’t feel like it is useful to repeat the condition given the #if spans only one line.

ldionne: I really don’t feel like it is useful to repeat the condition given the #if spans only one line.
#endif // defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) #endif // defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) || defined(__CloudABI__) || defined(__Fuchsia__) #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) || defined(__CloudABI__) || defined(__Fuchsia__)
# include <unistd.h> # include <unistd.h>
#endif // defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) || defined(__CloudABI__) || defined(__Fuchsia__) #endif // defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) || defined(__CloudABI__) || defined(__Fuchsia__)
#if defined(__NetBSD__) #if defined(__NetBSD__)
#pragma weak pthread_create // Do not create libpthread dependency #pragma weak pthread_create // Do not create libpthread dependency
▲ Show 20 Lines • Show All 190 Lines • Show Last 20 Lines