[cfe-dev] Flexible array fields in C++ classes.

[Douglas Gregor]

On Feb 2, 2010, at 7:10 AM, Enea Zaffanella wrote:

> Hello.
> 
> When compiling the following C++ code
> ==========================================
> struct S {
>  int size;
>  int vec[]; // This is OK.
> };
> 
> class C {
> public:
>  int size;
>  int vec[]; // This triggers diagnostic.
> };
> ==========================================
> 
> clang complains about the flexible array 'vec' that is found at the end of class C, whereby it accepts the very same thing if at the end of struct S. This is different wrt g++ behavior, which accepts both cases.
> 
> $ clang++ -fsyntax-only flex.cc
> flex.cc:9:7: error: field has incomplete type 'int []'
>  int vec[]; // This triggers diagnostic.
>      ^
> 1 diagnostic generated.

Frankly, I'd rather reject both cases in C++. Flexible arrays collide with C++ in a few places, and we need to find those places and address them before enabling this extension. I have the same view toward variable-length arrays in C++ (http://llvm.org/bugs/show_bug.cgi?id=5678): we *can* have the extension, but we need to complain and we need to deal with the various corner cases before enabling the extension. If we aren't strict about this, we'll end up where GCC is now, with a bunch of half-baked extensions. 

For example, I wrote this little test:

struct S {
  int size;
  int vec[];
};

struct S2 {
  S s;
  int x;
};

struct T : S { int x; };

template<typename T>
struct X { T t; int x; };

X<S> xs;

Even under -Wall, g++ doesn't complain about anything here... it doesn't complain about the definition of S2, which has a non-static data member ('x') following a non-static data member ('s') that has a flexible array member, nor does it complain about derivation from a class with a flexible array member, which is the moral equivalent of the previous problem. Clang does better (it warns about the first case, along with case where this happens in templates), but misses derivation.

So, Clang is already in the "half-baked" category with this extension to C++. We should either finish the extension or ban it in C++, but we shouldn't extend it 1% and then leave it there.

	- Doug

> Assuming that the one above is not what was really meant,
> as far as I can see, the relevant code is in SemaDecl.cpp
> (in function Sema::ActOnFields()):
> ==========================================
>    } else if (FDTy->isIncompleteArrayType() && i == NumFields - 1 &&
>               Record && Record->isStruct()) {
>      // Flexible array member.
>      if (NumNamedMembers < 1) {
>        Diag(FD->getLocation(), diag::err_flexible_array_empty_struct)
>          << FD->getDeclName();
>        FD->setInvalidDecl();
>        EnclosingDecl->setInvalidDecl();
>        continue;
>      }
>      // Okay, we have a legal flexible array member at the end of the struct.
>      if (Record)
>        Record->setHasFlexibleArrayMember(true);
> ==========================================
> 
> The impression is that the if-guard condition
>    Record->isStruct()
> is an overkill and could be replaced by the weaker form
>    (Record->isStruct() || Record->isClass())
> 
> Patch is attached.
> 
> Cheers,
> Enea Zaffanella.
> <flex_array_in_class.patch>_______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev