efriedma added inline comments.

================
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@-2{{variable length arrays are a C99 feature}}
+#endif
 }
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> > inclyc wrote:
> > > aaron.ballman wrote:
> > > > The diagnostic there is rather unfortunate because we're not using a 
> > > > variable-length array in this case.
> > > Emm, I'm not clear about whether we should consider this a VLA, and 
> > > generates `-Wvla-extensions`. Is `v[n]` literally a variable-length 
> > > array? (in source code) So it seems to me that we should still report c89 
> > > incompatibility warnings?
> > > 
> > C89's grammar only allowed for an integer constant expression to 
> > (optionally) appear as the array extent in an array declarator, so there is 
> > a compatibility warning needed for that. But I don't think we should issue 
> > a warning about this being a VLA in C99 or later. The array *is* a VLA in 
> > terms of the form written in the source, but C adjusts the parameter to be 
> > a pointer parameter, so as far as the function's type is concerned, it's 
> > not a VLA (it's just a self-documenting interface).
> > 
> > Because self-documenting code is nice and because people are worried about 
> > accidental use of VLAs that cause stack allocations (which this does not), 
> > I think we don't want to scare people off from this construct. But I'm 
> > curious what others think as well.
> > But I'm curious what others think as well.
> 
> (Specifically, I'm wondering if others agree that the only warning that 
> should be issued there is a C89 compatibility warning under 
> `-Wvla-extensions` when in C89 mode and via a new `CPre99Compat` diagnostic 
> group when enabled in C99 and later modes.)
> 
> 
I imagine people working with codebases that are also built with compilers that 
don't support VLAs would still want some form of warning on any VLA type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to