[libcxx-commits] [PATCH] D99515: [libc++] Build and test with -Wundef warning. NFC.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM with my fixes (assuming they fix the macOS CI, which they should unless I messed up somewhere).



================
Comment at: libcxx/include/type_traits:837-839
 // Before Clang 11, __is_pointer didn't work for Objective-C types.
-#if __has_keyword(__is_pointer) && !(defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER < 1100)
+#if __has_keyword(__is_pointer) &&                                             \
+    !(defined(_LIBCPP_CLANG_VER) && _LIBCPP_CLANG_VER < 1100)
----------------
AppleClang 12.0.5 should contain commit b25ec4580. See comment below for more info on why this is needed.


================
Comment at: libcxx/include/type_traits:1458-1460
 // Before Clang 13, __is_unsigned returned true for enums with signed underlying type.
-#if __has_keyword(__is_unsigned) && !(defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER < 1300)
+#if __has_keyword(__is_unsigned) &&                                            \
+    !(defined(_LIBCPP_CLANG_VER) && _LIBCPP_CLANG_VER < 1300)
----------------
We now need to handle AppleClang as well, otherwise we'll be using the intrinsic even on (slightly older) versions of AppleClang where it's broken.

Note that we need to start handling AppleClang "specially" here because we were using `_LIBCPP_COMPILER_CLANG`, and the proper translation of that is `_LIBCPP_COMPILER_CLANG_BASED`, not `_LIBCPP_CLANG_VER`. Previously AppleClang was kept off of this `#if` by means of `_LIBCPP_CLANG_VER < 1300` being considered `true`.

I checked and we don't need this sort of fix in the other similar places where we have workarounds for Clang 10, not because we don't have this bug, but because the versions of AppleClang that would be impacted are not going to be supported for building libc++ for much longer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99515