aaron.ballman 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}};
+
----------------
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.


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

Reply via email to