zoecarver marked 2 inline comments as done. zoecarver added inline comments.
================ Comment at: clang/test/SemaObjCXX/type-traits-is-pointer.mm:1 +// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s +// expected-no-diagnostics ---------------- ldionne wrote: > ldionne wrote: > > Why do you run this through `-verify`? Can't you just drop that and the `// > > expected-no-diagnostics` bit too? > Actually, never mind this. Someone pointed to me offline that using > `clang-verify` has the benefit that no other diagnostics are going to be > emitted, which is true. This is fine by me. Great, thanks for the review. Most of these tests seem to use `-verify` so that's what I did. ================ Comment at: libcxx/include/type_traits:901 +// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types. +#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000 + ---------------- ldionne wrote: > zoecarver wrote: > > ldionne wrote: > > > Doesn't `__has_keyword` return a number that can be compared against? > > > `#if __has_keyword(__is_pointer) > some-number` would be better if > > > feasible, because it would handle Clang, AppleClang and any other > > > potential derivative. > > I don't think `__has_keyword` returns a comparable number. Libc++ defines > > `__has_keyword ` using `__is_identifier` [1] and, it seems like all the > > feature checking macros from clang (including `__is_identifier`) have bool > > return types [2]. I can add an AppleClang check too if you want. > > > > [1]: > > ``` > > #define __has_keyword(__x) !(__is_identifier(__x)) > > ``` > > > > [2]: > > https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros > Ah, you're right. There's no version of AppleClang to put here right now, so > I'd say status quo is OK. I can add one later. Want me to add a TODO for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77519/new/ https://reviews.llvm.org/D77519 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
