flac dev - Feb 2017 - about "cpu.h: Fix compiler detection" patch

After this patch, all FLAC__SSEN_SUPPORTED variables are
undefined for GCC, so intrinsic versions of functions are
not compiled into libFLAC.

Previously, the code was:

#if defined __INTEL_COMPILER
   // definitions for ICC
#elif defined _MSC_VER
   // definitions for MSVC
#elif defined __GNUC__ || defined __clang__
   #if defined __clang__ && __has_attribute(__target__)
     // definitions for newer clang
   #elif !defined __clang__ && (__GNUC__ > 4 || (__GNUC__ == 4
&&
__GNUC_MINOR__ >= 9))
     // definitions for newer GCC
   #else
     // definitions for older GCC and clang
   #endif
#endif



Now, it's basically:



#if defined __INTEL_COMPILER
   // definitions for ICC
#elif defined _MSC_VER
   // definitions for MSVC
#elif defined __GNUC__ && defined __clang__
   // definitions for clang
#endif



By the way, what's the problem with GCC 4.6?
According to <https://gcc.gnu.org/onlinedocs/cpp/If.html>:
"... and logical operations (&& and ||). The latter two obey the
usual
short-circuiting rules of standard C."

So I thought that the directive

    #if defined __clang__ && __has_attribute(__target__)

is ok for GCC because "defined __clang__" is false and
preprocessor shouldn't try to parse "__has_attribute(...)" part.

lvqcl wrote:
> After this patch, all FLAC__SSEN_SUPPORTED variables are
> undefined for GCC, so intrinsic versions of functions are
> not compiled into libFLAC.
Sigh!  The C preprocessor is by far the very worst feature of
the C langauge. Its hard to read, hard to debug, hard to 
verify and just incredibly fragile.
> Now, it's basically:
I think its fixed in:

   https://github.com/xiph/flac/pull/30

> By the way, what's the problem with GCC 4.6?
The travis build log is at:

    https://travis-ci.org/xiph/flac/jobs/201678000
> According to <https://gcc.gnu.org/onlinedocs/cpp/If.html>:
> "... and logical operations (&& and ||). The latter two obey
the usual
> short-circuiting rules of standard C."
> 
> So I thought that the directive
> 
>     #if defined __clang__ && __has_attribute(__target__)
> 
> is ok for GCC because "defined __clang__" is false and
> preprocessor shouldn't try to parse "__has_attribute(...)"
part.
I agree. I would call that error a compiler bug.

Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

lvqcl wrote:
> So I thought that the directive
> 
>     #if defined __clang__ && __has_attribute(__target__)
> 
> is ok for GCC because "defined __clang__" is false and
> preprocessor shouldn't try to parse "__has_attribute(...)"
part.
It seems this is actually a pre-processor parsing bug. It hits
the bug *before* the logic is evaluated. My current solution in
the above PR is to avoid `__has_attribute` and use this:

    #elif defined __clang__ && (__clang_major__ > 3 || \
          (__clang_major__ == 3 && __clang_minor__ >= 6)) /* clang */

which I have tested with clang 3.6. If someone has an earlier version
of clang and can verify that it work, I'll drop the version number.

Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

Erik de Castro Lopo wrote:
> the bug *before* the logic is evaluated. My current solution in
> the above PR is to avoid `__has_attribute` and use this:
>
>     #elif defined __clang__ && (__clang_major__ > 3 || \
>           (__clang_major__ == 3 && __clang_minor__ >= 6)) /*
clang */
>
> which I have tested with clang 3.6. If someone has an earlier version
> of clang and can verify that it work, I'll drop the version number.

Maybe it's simpler to add

#ifndef __has_attribute
#define __has_attribute(x) 0
#endif
#ifndef __has_builtin
#define __has_builtin(x) 0
#endif

somewhere before their use? (see  
http://clang.llvm.org/docs/LanguageExtensions.html )