[PATCH] D21453: Add support for attribute "overallocated"

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: include/clang/AST/Decl.h:3249
@@ -3248,1 +3248,3 @@
 
+  /// This is true if this struct ends with an array marked 'flexible_array'.
+  bool HasFlexibleArrayAttr : 1;
----------------
ahatanak wrote:
> Probably it can be looked up although it would require incrementing the field_iterator until it reaches the last non-static data member of the record.
Ah shoot, I forgot that these are forward iterators, not bidirectional ones. I think the bit is probably the better way to go; we have bits to spare here.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2170
@@ +2169,3 @@
+  "'flexible_array' attribute only applies to %select{"
+  "the last member of a struct|members of structs or classes|"
+  "fixed sized array members|array members that have at least one element}0">;
----------------
I think "members of structs or classes" and "the last member of a struct" could be combined into "the last member of a non-union class", instead of using separate diagnostic text. What do you think?

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2171
@@ +2170,3 @@
+  "the last member of a struct|members of structs or classes|"
+  "fixed sized array members|array members that have at least one element}0">;
+def err_flexible_array_nested : Error<
----------------
Since you can only have one such member, I think we want to drop the plural from both of these. I think the first may read better as "a non-flexible array member" (since there are "arrays" and "flexible arrays", but not "fixed-sized arrays" in our diagnostics), and the second may read better as "an array member that has at least one element".

================
Comment at: lib/AST/ExprConstant.cpp:170
@@ +169,3 @@
+    /// Indicator of whether the last array added is marked flexible_array.
+    bool IsFlexibleArray : 1;
+
----------------
ahatanak wrote:
> I don't think there is a way to do that reliably. The FieldDecl for the array isn't always available in struct LValue, as far as I can tell, so it looks like we'll need a bit here.
Okay, that seems reasonable to me -- I don't think anyone is relying on that bit being stolen from `MostDerivedPathLength`.


http://reviews.llvm.org/D21453