Page MenuHomePhabricator

Fix threads build on GNU/Hurd
ClosedPublic

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

Details

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
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.

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 ?

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

Please replace defined(BSD).

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

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.

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