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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
