On Wed, Sep 24, 2014 at 10:42 AM, Arthur O'Dwyer <[email protected]> wrote:
> On Wed, Sep 24, 2014 at 4:22 AM, David Majnemer > <[email protected]> wrote: > > Hi rsmith, rnk, andreadb, > > > > Zero sized arrays are more or less the pre-standard version of flexible > > array members. It makes sense to mark structs which contain them as > > hasFlexibleArrayMember. > > > > Doing this has the side effect of resolving PR21040, a crash involving > > one record inheriting from a base which is terminated with a zero sized > > array field. > > > > http://reviews.llvm.org/D5478 > > Drive-by comments: > > (1) What effect does this have on templates, e.g. > template<int N> struct A { int a[N]; }; > struct B : private A<0> { int b; }; > struct C { A<0> a; int c; }; > ? IIUC, Clang used to produce a GCC-style "array with size zero" in > this case, and now produces a hard error. Your new code is justified, > IMHO, but could you add a test case involving templates or other > sneaky code, just to be explicit that the new behavior is 100% > intended? > I don't think we need a test specifically for templates, instantiation will give the concrete 'A' field a ConstantArrayType. > > (2) Unfortunately unrelated to your patch: The error is "base class > 'A' has a flexible array member", but it doesn't generate a note > pointing to the flexible array member's declaration, nor even mention > the name of the flexible array member. IMHO it should, especially now > that a "flexible array member" may have an innocent-looking > declaration of the form "int a[N]". > 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. > > my $.02, > –Arthur >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
