[PATCH] D111581: Remove checks for old gcc versions for LLVM_ATTRIBUTE_*

Thu Oct 14 00:03:38 PDT 2021

mstorsjo added inline comments.

================
Comment at: llvm/include/llvm/Support/Compiler.h:100
 /// corrected in MSVC 2019 but not MSVC 2017.
-#if __has_feature(cxx_rvalue_references) || LLVM_GNUC_PREREQ(4, 8, 1) ||       \
-    LLVM_MSC_PREREQ(1920)
+#if __has_feature(cxx_rvalue_references) || LLVM_MSC_PREREQ(1920)
 #define LLVM_HAS_RVALUE_REFERENCE_THIS 1
----------------
aeubanks wrote:
> yrouban wrote:
> > For gcc this change results in LLVM_HAS_RVALUE_REFERENCE_THIS set to 0.
> > That is because __ has_feature(x) is always 0 for gcc regardless of the version (I use 7.4.0).
> > //__has_feature// is not and will not be defined for gcc. (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60512). Please consider fixing or reverting this patch.
> > 
> sorry about that, should be fixed with 60605a2b8fa2
FWIW, just for the record, with `__has_feature(cxx_rvalue_references) || defined(__GNUC__)`, this would also end up enabled for Clang (which identifies itself as `__GNUC__` normally) even for older versions of Clang that don't support `__has_feature(cxx_rvalue_references)`. In this particular case, `__has_feature(cxx_rvalue_references)` is true since Clang 3.5 which also is our current minimum supported compiler version, so it's probably ok - but it could have been an issue.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111581