[PATCH] D156565: Diagnose use of VLAs in C++ by default

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:147-149
+def ext_vla_cxx_static_assert : ExtWarn<
+  "variable length arrays in C++ are a Clang extension; did you mean to use "
+  "'static_assert'?">, InGroup<VLAUseStaticAssert>;
----------------
I find the "did you mean to use 'static_assert'" phrasing awkward here since it is highly unlikely that someone intended to write `static_assert(x)` and instead wrote `int my_assert[x ? 1 : -1]`. Perhaps something like this?

"variable length arrays in C++ are a Clang extension; 'static_assert' can be used in this case".


================
Comment at: clang/test/SemaCXX/vla-ext-diag.cpp:34
+                                   off-note {{function parameter 'n' with unknown value cannot be used in a constant expression}}
+}
----------------
Perhaps add a test where the conditional expression is parenthesized.
  int array4[(n ? 1 : -1)]; 

Adding tests for one side being of size 0 might be useful to demonstrate that those are not intended to trigger the "use 'static_assert'" diagnostic. I know some compilers don't diagnose on a size of 0 and so the static assert idiom generally requires a negative number.
  int array5[n ? 1 : 0]; // ok?


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

https://reviews.llvm.org/D156565