[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;
----------------
Does this require a bit, or can this simply be looked up as needed?

================
Comment at: include/clang/Basic/Attr.td:18
@@ -17,2 +17,3 @@
 def DocCatVariable : DocumentationCategory<"Variable Attributes">;
+def DocCatField : DocumentationCategory<"Field Attributes">;
 def DocCatType : DocumentationCategory<"Type Attributes">;
----------------
Instead of "field", how about "non-static data member"? That should make it more clear as to what it pertains to.

================
Comment at: include/clang/Basic/Attr.td:2282
@@ +2281,3 @@
+  let Subjects = SubjectList<[Field], ErrorDiag,
+                             "ExpectedField">;
+  let Documentation = [FlexibleArrayDocs];
----------------
Should be able to drop the string literal here; it should work by default. If it doesn't, then ClangAttrEmitter.cpp should be updated to properly handle it.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2165
@@ +2164,3 @@
+def err_flexible_array_attribute : Error<
+  "'flexible_array' attribute only applies to %0">;
+def err_flexible_array_nested : Error<
----------------
This should be updated to use a %select instead of passing string literals (for eventual localization purposes).

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2520
@@ -2516,1 +2519,3 @@
+  "variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members|"
+  "fields}1">,
   InGroup<IgnoredAttributes>;
----------------
non-static data members instead of fields.

================
Comment at: lib/AST/ExprConstant.cpp:170
@@ +169,3 @@
+    /// Indicator of whether the last array added is marked flexible_array.
+    bool IsFlexibleArray : 1;
+
----------------
Is the bit required here as well, or can this be queried as-needed?

================
Comment at: lib/AST/ExprConstant.cpp:1133
@@ -1128,1 +1132,3 @@
+    void addArray(EvalInfo &Info, const Expr *E, const ConstantArrayType *CAT,
+                  bool HasFlexibleArrayAttr) {
       if (checkSubobject(Info, E, CSK_ArrayToPointer))
----------------
Perhaps this should be a default argument, since it's false almost everywhere?

================
Comment at: lib/AST/ExprConstant.cpp:5184-5185
@@ +5183,4 @@
+      bool HasFlexibleArrayAttr = false;
+      if (auto *ME = dyn_cast<MemberExpr>(SubExpr)) {
+        auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl());
+        HasFlexibleArrayAttr = FD && FD->hasAttr<FlexibleArrayAttr>();
----------------
Should be `const` qualified.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:1808-1809
@@ +1807,4 @@
+                                    const AttributeList &Attr) {
+  auto *FD = cast<FieldDecl>(D);
+  auto *CAT = dyn_cast<ConstantArrayType>(FD->getType().getTypePtr());
+
----------------
Should be `const`


http://reviews.llvm.org/D21453