<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Sep 24, 2014 at 10:42 AM, Arthur O'Dwyer <span dir="ltr"><<a href="mailto:arthur.j.odwyer@gmail.com" target="_blank">arthur.j.odwyer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Sep 24, 2014 at 4:22 AM, David Majnemer<br>
<<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br>
> Hi rsmith, rnk, andreadb,<br>
><br>
> Zero sized arrays are more or less the pre-standard version of flexible<br>
> array members.  It makes sense to mark structs which contain them as<br>
> hasFlexibleArrayMember.<br>
><br>
> Doing this has the side effect of resolving PR21040, a crash involving<br>
> one record inheriting from a base which is terminated with a zero sized<br>
> array field.<br>
><br>
> <a href="http://reviews.llvm.org/D5478" target="_blank">http://reviews.llvm.org/D5478</a><br>
<br>
</span>Drive-by comments:<br>
<br>
(1) What effect does this have on templates, e.g.<br>
    template<int N> struct A { int a[N]; };<br>
    struct B : private A<0> { int b; };<br>
    struct C { A<0> a; int c; };<br>
? IIUC, Clang used to produce a GCC-style "array with size zero" in<br>
this case, and now produces a hard error.  Your new code is justified,<br>
IMHO, but could you add a test case involving templates or other<br>
sneaky code, just to be explicit that the new behavior is 100%<br>
intended?<br></blockquote><div><br></div><div>I don't think we need a test specifically for templates, instantiation will give the concrete 'A' field a ConstantArrayType.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
(2) Unfortunately unrelated to your patch: The error is "base class<br>
'A' has a flexible array member", but it doesn't generate a note<br>
pointing to the flexible array member's declaration, nor even mention<br>
the name of the flexible array member. IMHO it should, especially now<br>
that a "flexible array member" may have an innocent-looking<br>
declaration of the form "int a[N]".<br></blockquote><div><br></div><div>This is not trivial, RecordDecls don't remember what it is that made them non-flexible.  While improving diagnostic quality here would be nice, I don't think it's vital.  I don't think it's unreasonable for a QoI improvement of that nature to be done separately from this work.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
my $.02,<br>
–Arthur<br>
</blockquote></div><br></div></div>