[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:
----------------
cjdb wrote:
> aaron.ballman wrote:
> > Is a VLA a bounded array? (We support those as an extension in C++, unfortunately.)
> `__is_bounded_array(T)` is true when "`T` is an array type of known bound". Do VLAs meet this criteria?
Heh, that's just it -- I don't know what constitutes "known". A VLA that tracks its extent "knows" the extent at runtime but possibly not compile time. So it's known-ish.

I think there's a few ways we could go about it.

1) all VLAs are unbounded because some VLAs may not have their size known until runtime
2) check the VLA's size expression and if we can find any way to constant evaluate it to a known value, then it's a bounded array
3) make it an error to call this function with a VLA

My intuition is that #3 is the safest approach, but that #1 probably is sufficiently defensible as well. I worry a little bit that #2 will give surprising results without more investigation/effort (we sometimes fold VLAs into constant arrays, and I think users would expect this API to mirror that behavior).


================
Comment at: clang/test/SemaCXX/type-traits.cpp:764
+  int t30[F(__is_bounded_array(void*))];
+  int t31[F(__is_bounded_array(cvoid*))];
+}
----------------
cjdb wrote:
> aaron.ballman wrote:
> > Any reason there's not a test with something like `int[10]` and `int[]`? (Same below.)
> Those are `IntAr` and `IntArNB` at the very top. I can rename those if you'd like.
Oh, yeah, I totally missed that, thank you!

Up to you on the rename, so long as we have the coverage, that's the crucial bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116280