[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! Can you be sure to add more information to the description so that reviewers more quickly understand why the changes are being made?

In terms of the changes themselves, I don't think they're heading in the right direction just yet. The changes are because we want to implement https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2992.pdf, which clarifies what constitutes a VLA, which is a problem with our type system or how we use it. We have the `VariableArrayType` to represent a VLA type and I think we're using that type when we should be using a variably modified type (see `Type::isVariablyModifiedType()`).



================
Comment at: clang/include/clang/Sema/Sema.h:1760
+    // Special builder emitting no diagnostics
+    SemaDiagnosticBuilder(Sema &S) : S(S) {}
     SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
----------------
erichkeane wrote:
> I haven't looked particularly closely, but this doesn't seem right to me.  There is already the 'K_Nop' diagnostic Kind (see the line below), that we should look at the uses of.
+1, I think it's dangerous to add this interface (especially because it's not an explicit ctor).


================
Comment at: clang/test/Sema/warn-vla.c:8-12
+void test2(int n, int v[n]) { // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning at -2{{variable length arrays are a C99 feature}}
+#endif
 }
----------------
The diagnostic there is rather unfortunate because we're not using a variable-length array in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952