[PATCH] D135175: [clang] adds `__is_bounded_array` and `__is_unbounded_array` as builtins

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:24
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
----------------
cjdb wrote:
> shafik wrote:
> > Why also `Type.h`?
> The most likely reason is that it's come from D116280, and is actually important for one of the child patches (I'm not sure which off the top of my head). I can investigate and move it to the correct one before merging.
I'd say it isn't particularly important to figure out whichone.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:4910
+    if (T->isVariableArrayType())
+      Self.Diag(KeyLoc, diag::err_vla_unsupported)
+          << 1 << tok::kw___is_bounded_array;
----------------
This should make this return 'false' in this case, rather than a success?  Not sure about this, but wanted to make sure you were thinking about this.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:4915
+    if (T->isVariableArrayType())
+      Self.Diag(KeyLoc, diag::err_vla_unsupported)
+          << 1 << tok::kw___is_unbounded_array;
----------------
Same here, not clear if this is the logic you meant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135175