[Openmp-commits] [PATCH] D57851: Guard a feature that unsupported by old GCC

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

In D57851#1387935 <https://reviews.llvm.org/D57851#1387935>, @twoh wrote:

> @kkwli0 Thanks for letting me know! But wouldn't https://reviews.llvm.org/D57848 always fall to "else" case added if the compiler is not GCC?


I agree, Clang defines `__GNUC__` to value `4` for compatibility reasons. Likewise, this change doesn't handle compilers other than GCC that don't support `__has_cpp_attribute` (yet).

I'd propose to take the solution that libcxx <https://github.com/llvm/llvm-project/blob/master/libcxx/include/__config#L147>, LLVMSupport <https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/Compiler.h#L38>, and LLVMDemangle <https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Demangle/DemangleConfig.h#L29> use:

  #ifndef __has_cpp_attribute
  #define __has_cpp_attribute(__x) 0
  #endif

Also Kelvin is right that `_GNUC_VER` is not defined here (it's from libcxx <https://github.com/llvm/llvm-project/blob/master/libcxx/include/__config#L26>). Maybe we should take the macros from LLVM (except the non-C++ case)? That would also give us a correct checking for `gnu::fallthrough`, but remove `__attribute__((__fallthrough__))` completely.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57851/new/

https://reviews.llvm.org/D57851