rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Some minor typographical comments.
Please add some tests for the union case, then this LGTM.
================
Comment at: lib/AST/ExprConstant.cpp:6333-6336
@@ +6332,6 @@
+ if (BaseType->isArrayType()) {
+ // Because __builtin_object_size treats arrays as objects, we can ignore
+ // the index iff this is the last array in the Designator.
+ if (I + 1 == E)
+ return true;
+ auto *CAT = cast<ConstantArrayType>(Ctx.getAsArrayType(BaseType));
----------------
OK, but please reflect that this is specific to `__builtin_object_size` in the
name or at least doc comment for this function.
================
Comment at: lib/AST/ExprConstant.cpp:6361
@@ +6360,3 @@
+
+/// Tests to see if the has a designator (that isn't necessarily valid).
+static bool hasDesignator(const LValue &LVal) {
----------------
This is missing a noun.
================
Comment at: lib/AST/ExprConstant.cpp:6362
@@ +6361,3 @@
+/// Tests to see if the has a designator (that isn't necessarily valid).
+static bool hasDesignator(const LValue &LVal) {
+ if (LVal.Designator.Invalid || !LVal.Designator.Entries.empty())
----------------
I think this would be clearer if it were named `hasNonemptyDesignator`... or
perhaps reverse the sense of it and name it `refersToCompleteObject` or similar?
http://reviews.llvm.org/D12821
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
