[PATCH] D116280: [clang] adds unary type trait checks as compiler built-ins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:4889-4890
     return T->isArrayType();
+  case UTT_IsBoundedArray:
+    return T->isArrayType() && !T->isIncompleteArrayType();
+  case UTT_IsUnboundedArray:
----------------
Is a VLA a bounded array? (We support those as an extension in C++, unfortunately.)


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5282
+      return false;
+    if (!T.isReferenceable(LangStandard::getLangStandardForKind(Self.LangOpts.LangStd).isCPlusPlus()))
+      return false;
----------------
Why this dance and not `Self.LangOpts.CPlusPlus`? (Same below.)


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5289
+    if (UTT == UTT_IsCopyAssignable) {
+      return EvaluateBinaryTypeTrait(Self, BTT_IsAssignable, Self.BuildReferenceType(T, true, KeyLoc, T.getBaseTypeIdentifier()), AssigneeType, KeyLoc);
+    }
----------------
It looks like this file could stand to be clang formatted.


================
Comment at: clang/test/SemaCXX/type-traits.cpp:737
 
+void is_bounded_array()
+{
----------------
Please add a test for VLAs to whichever function it should be tested under.


================
Comment at: clang/test/SemaCXX/type-traits.cpp:764
+  int t30[F(__is_bounded_array(void*))];
+  int t31[F(__is_bounded_array(cvoid*))];
+}
----------------
Any reason there's not a test with something like `int[10]` and `int[]`? (Same below.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116280