efriedma added inline comments.
================
Comment at: clang/test/Sema/vla.c:134
+ // expected-warning@+1{{variable length array folded to constant array as an
extension}}
+ char (*a9)[] = (char[2][ksize]) {{1,2,3,4},{4,3,2,1}};
+
----------------
aaron.ballman wrote:
> efriedma wrote:
> > aaron.ballman wrote:
> > > efriedma wrote:
> > > > aaron.ballman wrote:
> > > > > Doesn't this violate the constraints in C17 6.5.2.5p1, "The type name
> > > > > shall specify a complete object type or an array of unknown size, but
> > > > > not a variable-length array type"?
> > > > Yes, this is a constraint violation. This patch downgrades the error
> > > > to a warning, for compatibility with older versions of clang.
> > > It was an error in older versions of Clang, so downgrading it to a
> > > warning can't make code *more* compatible with older versions. This also
> > > makes it harder to port C code to other compilers because Clang happily
> > > accepts code that the C standard says should be rejected.
> > >
> > > I'm not strongly opposed to the change, but I also don't like ignoring
> > > constraint violations in the standard as that comes awfully close to what
> > > `-fpermissive` does (and at least with that awful flag you have to opt
> > > *into* the less strictly conforming behavior instead of getting it by
> > > default as with extensions).
> > >
> > > I'm curious if @rsmith has thoughts here?
> > > It was an error in older versions of Clang
> >
> > https://godbolt.org/z/rvbffY
> Oh, I hadn't realized this changed *that* recently! Is this breaking some
> significant amounts of code now that we err on it (or regressing performance
> by not folding the VLA)?
>
> My point still stands about disliking when we ignore constraint violations.
> To expound a bit, in this particular case, I think the standard is being
> over-constraining because we reasonably *can* fold this to a more efficient
> form. Normally, I'd suggest filing a DR with WG14 over this and treating the
> constraint as UB we can extend. However, I think the "solution" that comes
> out of WG14 would (likely) be to make compound literal expressions of VLA
> types be undefined behavior rather than a constraint violation (because I
> don't see many other options in the standard wording for a more targeted fix
> to allow *just* this safe usage). Given the security concerns around misuse
> of VLAs already, I think that would be a worse world to live in even if it
> gets us what we want with this specific case in mind. This is where my
> caution is coming from. Knowing a bit more about the experience in the
> proprietary code bases would be helpful, if it's something you can share more
> details about.
The pattern I saw looked something like the following, at global scope, where
it couldn't really be confused for a VLA:
```
const int x = 2;
int *p = (int[x]){0};
```
As for the frequency of it showing up, I think I got confused. Looking again,
there were multiple codebases with new -Wgnu-folding-constant warnings, but
only one codebase with this specific construct. So that makes the argument a
bit weaker.
That said, we're doing this folding for variables already, so extending it to
compound literals doesn't seem like a big deal. And the warning is on by
default.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98363/new/
https://reviews.llvm.org/D98363
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
