[PATCH] D12821: Allow for C's "writing off the end" idiom in __builtin_object_size

[Richard Smith via cfe-commits]

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