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

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius added inline comments.


================
Comment at: libcxx/include/type_traits:840
+    !(defined(_LIBCPP_COMPILER_CLANG) && defined(_LIBCPP_CLANG_VER) &&         \
+      _LIBCPP_CLANG_VER < 1100)
 
----------------
Quuxplusone wrote:
> ldionne wrote:
> > Quuxplusone wrote:
> > > These are pretty icky. I would tentatively suggest at least breaking the line as
> > > ```
> > > #if __has_keyword(__is_pointer) &&                                             \
> > >     !(defined(_LIBCPP_COMPILER_CLANG) &&                                       \
> > >       defined(_LIBCPP_CLANG_VER) && _LIBCPP_CLANG_VER < 1100)
> > > ```
> > > but I'd also want to wait and see if anyone has a better idea (e.g. pulling out a `_LIBCPP_CLANG_VER_LESS_THAN(x)` macro or something).
> > I think this is equivalent:
> > 
> > ```
> > !(defined(_LIBCPP_CLANG_VER) && _LIBCPP_CLANG_VER < 1100)
> > ```
> > 
> > In other words, I think checking for `_LIBCPP_COMPILER_CLANG` is redundant if you already check for `_LIBCPP_CLANG_VER` unless I'm mistaken.
> @ldionne: Sadly no; see D98720. Apple Clang defines `_LIBCPP_COMPILER_CLANG` without `_LIBCPP_CLANG_VER` (because their version numbers are all screwed up), and so we need to treat Apple Clang essentially as "Clang version 0.0" for the purposes of these feature tests.
> 
> Actually, is there any appetite to just `#define _LIBCPP_CLANG_VER 0` on Apple Clang?
> 
> Double actually, @curdeius, isn't this change wrong? It should be
> ```
> #if __has_keyword(__is_pointer) &&                                             \
>     !(defined(_LIBCPP_COMPILER_CLANG) &&                                       \
>       (!defined(_LIBCPP_CLANG_VER) || _LIBCPP_CLANG_VER < 1100))
> ```
> 
> So I am now more confident in my suggestion that we need **either** `_LIBCPP_CLANG_VER_LESS_THAN` **or** simply to define `_LIBCPP_CLANG_VER` as `0` on Apple Clang.
Indeed, that was too-quick-a-change from my part.
So, now I `#define _LIBCPP_CLANG_VER 0` on AppleClang. It seems to be cleaner this way. It preserves the current behaviour as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99515