[PATCH] Improved capturing variable-length array types in CapturedStmt.

John McCall rjmccall at gmail.com
Fri Oct 10 11:59:26 PDT 2014


Generally looks good.  A few comments inline.

================
Comment at: include/clang/AST/Stmt.h:1985
@@ -1984,1 +1984,3 @@
+    VCK_ByRef,
+    VCK_VLAType
   };
----------------
Leave a comma after the last enumerator, please.

================
Comment at: lib/AST/Decl.cpp:3274
@@ -3269,1 +3273,3 @@
+    return !getDeclContext()->isRecord() ||
+           !isVLATypeCapturingAllowed(getParent());
   }
----------------
hfinkel wrote:
> Calling this function here seems confusing because this has nothing to do with VLAs.
It's because we're reusing the same member to store
  - C++11 in-class initializer expressions
  - bitfield width expressions
  - lambda and captured-statement captured VariableArrayType*s

However, this is getting out of hand.  Querying this should be efficient.  I'm committing a patch to fix this; you'll have to rebase some of this patch on top of that.

================
Comment at: lib/AST/Decl.cpp:3564
@@ +3563,3 @@
+bool RecordDecl::isCapturedRecord() const {
+  return getTagKind() == TTK_Struct && getLocStart() == getLocation() &&
+         getIdentifier() == nullptr && isImplicit() &&
----------------
hfinkel wrote:
> Is testing the source location really necessary here? That seems fragile.
I agree, there is absolutely no reason to be doing this.  If you need a bit to store that this is a capture record, find a bit to use.  Or set an attribute.

http://reviews.llvm.org/D5099






More information about the cfe-commits mailing list