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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits