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:
> > > 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.
Even if the committee can make retroactive changes to standards, that doesn't
change the fact that MSVC doesn't support VLAs, and that codebases exist which
need to be compiled with both clang and MSVC.
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
