aaron.ballman 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
}
----------------
efriedma wrote:
> 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.)
In a strict sense, yes, this is a C23 change. There are no changes to older
standards as far as ISO is concerned (and there never have been).
In a practical sense, no, this is clarifying how VLAs have always been intended
to work. C23 is in a really weird spot in that we had to stop our "defect
report" process at the end of C17 because the ISO definition of a defect did
not match the WG14 definition of a defect, and ISO got upset. Towards the tail
end of the C23 cycle, we introduced a list of extensions to obsolete versions
of C (https://www.open-std.org/jtc1/sc22/wg14/www/previous.html) as a stopgap,
but the convener would not allow us to retroactively add papers to it. The
committee still hasn't gotten used to this new process and as a result, not
many papers have made it to the list. We've also not had the chance to discuss
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3002.pdf on having a more
reasonable issue tracking process.
The end result is that there's a whole pile of papers in C23 that we would have
normally treated via the old defect report process but couldn't, and if we had
treated them via the old DR process, it would have been more clear why I think
this should be a retroactive change back to C99 instead of a C23-only change.
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
