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: > efriedma wrote: > > 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. > The tricky part is: that's precisely what WG14 N2992 > (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2992.pdf) is clarifying. > If your implementation doesn't support VLAs, it's still required to support > this syntactic form. So the question becomes: do we want a portability > warning to compilers that don't conform to the standard? Maybe we do (if we > can find evidence of compilers in such a state), but probably under a > specific diagnostic flag rather than -Wvla. That only applies to C23, though, right? None of that wording is there for C11. (In particular, MSVC claims C11 conformance without any support for VLA types.) 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