bloomberg / bde

Basic Development Environment - a set of foundational C++ libraries used at Bloomberg.
Apache License 2.0
1.67k stars 316 forks source link

Infer -Dunix on Darwin #97

Closed frutiger closed 10 years ago

frutiger commented 10 years ago

bsls_platform is able to infer BSLS_PLATFORM_OS_UNIX for Darwin, but still requires one of unix, __unix or __unix__ to be defined. This does not apply for any of the other *nixes.

The check is done in the following snippet from bsls_platform;

// Unix flag must be set by the compiler if Unix detected (except for AIX).
#if defined(BSLS_PLATFORM_OS_UNIX) && !defined(BSLS_PLATFORM_OS_AIX)
    #if !defined(unix) && !defined(__unix__) && !defined(__unix)
        #error "Unix platform assumed, but unix flag not set by compiler"
        BSLS_PLATFORM_COMPILER_ERROR;
    #endif
#endif

However, the issue arises from the fact that g++ and clang++ both define __GNUC__ but never lead to the definition of BSLS_PLATFORM_OS_WINDOWS. As a result, we define BSLS_PLATFORM_OS_UNIX and so we expect one of unix, __unix or __unix__ to be defined during the validation stage.

OS X (and Darwin) should not require any special definitions if it is to be a supported platform.

Incidentally, xlC also does not define unix, __unix nor __unix__, but it is explicitly excluded in the validation step.

che2 commented 10 years ago

This page shows the ways that OSX and darwin can be detected using preprocessor macros:

http://nadeausoftware.com/articles/2012/01/c_c_tip_how_use_compiler_predefined_macros_detect_operating_system

mversche commented 10 years ago

This check is in a block marked for validating our assumptions about how we've configured various flags. We can certainly exclude OSX -- but there may be some alternative validation that we want to perform to verify that we are not running xlc/clang on a non-unix system.

kpfleming commented 10 years ago

XL C is only available for AIX, Linux, z/OS and Blue Gene supercomputers. I suspect we can safely avoid worrying about the latter two :-)

Clang is supported on Windows, but I suspect that logic earlier in the header file will have already figured out that the platform is Windows.

gstrauss commented 10 years ago

OS X (and Darwin) should not require any special definitions if it is to be a supported platform.

I agree with Masud. The check for

#if defined(BSLS_PLATFORM_OS_UNIX) && !defined(BSLS_PLATFORM_OS_AIX)

should be extended to:

#if defined(BSLS_PLATFORM_OS_UNIX) && !defined(BSLS_PLATFORM_OS_AIX) \
    && !defined(BSLS_PLATFORM_OS_DARWIN)

Some notes follow below from: https://github.com/gstrauss/mcdb/blob/master/plasma/plasma_feature.h

unix or unix__ not defined by all compilers on all unix-like platforms (e.g. gcc 4.7.0 defines __unix on _AIX but not earlier gcc http://gcc.gnu.org/bugzilla/show%5Fbug.cgi?id=39950)

#if defined(__unix) || defined(__unix__) \
 || (defined(__APPLE__) && defined(__MACH__)) || defined(_AIX)
abeels commented 10 years ago

Internal ticket #46631049 created for tracking.

gstrauss commented 10 years ago

@frutiger : I am curious: how did you run into this issue? common.gypi defines __unix on Mac OSX. Perhaps you were building other code that included bsl headers, and using a different build system?

frutiger commented 10 years ago

@gstrauss: I was indeed using a custom build system: https://github.com/frutiger/bde-meta. It's a very simple tool that does very few things.

abeels commented 10 years ago

This change is under review at https://review.openbloomberg.com/D58

abeels commented 10 years ago

Landed in 76420335