Skip to content

Conversation

@compnerd
Copy link
Member

@compnerd compnerd commented Aug 2, 2017

Newer versions of glibc indicate that they intend to move the major
macro from sys/types.h to sys/sysmacros.h. Add a check for the header
and include that earlier to ensure that the macro is provided by the
newer header when available/possible. This avoids an unnecessary warning
from the system headers.

Because config_ac.h is not available at the right location, we cannot
include the header to check whether the header is available. Rely on
the compiler provided __has_include instead of the configure check.
Adjust the inclusion of sys/cdefs.h accordingly.

#include <unistd.h>
#endif
#if HAVE_SYS_CDEFS_H
#if __has_include(<sys/cdefs.h>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one concern with this approach here we've always had is that __has_include is not supported on all compilers we support at least in the Darwin SDK (that is why dispatch/base.h defines them).

I would suggest replicating the approach at the top of dispatch/dispatch.h here (replacing lines 30-32)

#ifdef __APPLE__
#include <Availability.h>
#include <os/availability.h>
#include <TargetConditionals.h>
#include <os/base.h>
#elif defined(__linux__)
#include <os/linux_base.h>
#endif

and then actually dropping the #include <sys/cdefs.h> completely here (as it will already be covered by the base*.h includes)

@compnerd
Copy link
Member Author

compnerd commented Aug 3, 2017

Updated.

#include <os/availability.h>
#include <TargetConditionals.h>
#include <os/base.h>
#else
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be #elif defined(__linux__) like in dispatch/dispatch.h, these headers are included by other platforms as well...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good idea; I would've hit this the next time I did a windows build!

@das
Copy link
Contributor

das commented Aug 4, 2017

@swift-ci please test

Newer versions of glibc indicate that they intend to move the major
macro from sys/types.h to sys/sysmacros.h. Add a check for the header
and include that earlier to ensure that the macro is provided by the
newer header when available/possible. This avoids an unnecessary warning
from the system headers.

Because `config_ac.h` is not available at the right location, we cannot
include the header to check whether the header is available.  Rely on
the compiler provided `__has_include` instead of the configure check.
Adjust the inclusion of `sys/cdefs.h` accordingly.
@das
Copy link
Contributor

das commented Aug 4, 2017

@das das merged commit 0fd5a69 into swiftlang:master Aug 4, 2017
@compnerd compnerd deleted the inclusivity branch August 4, 2017 23:04
ktopley-apple pushed a commit that referenced this pull request Dec 6, 2018
linux: update header used for `major` macro

Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants