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

Alexey Bataev a.bataev at hotmail.com
Tue Oct 14 00:43:02 PDT 2014


Hal, John, Many thanks for the review!

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

================
Comment at: include/clang/AST/Stmt.h:2044
@@ -2031,1 +2043,3 @@
+      assert(capturesVariable() &&
+             "No variable available for 'this' or VAT capture");
       return VarAndKind.getPointer();
----------------
hfinkel wrote:
> VAT? Do you mean VLA?
No, VAT. See above: VariableArrayType - VAT.

================
Comment at: lib/AST/Decl.cpp:3274
@@ -3269,1 +3273,3 @@
+    return !getDeclContext()->isRecord() ||
+           !isVLATypeCapturingAllowed(getParent());
   }
----------------
rjmccall wrote:
> 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.
Ok

================
Comment at: lib/AST/Decl.cpp:3564
@@ +3563,3 @@
+bool RecordDecl::isCapturedRecord() const {
+  return getTagKind() == TTK_Struct && getLocStart() == getLocation() &&
+         getIdentifier() == nullptr && isImplicit() &&
----------------
rjmccall wrote:
> 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.
Ok, I'll rework this.

================
Comment at: lib/CodeGen/CGStmt.cpp:2161
@@ +2160,3 @@
+      auto *ExprArg = EmitLoadOfLValue(EmitLValueForField(Base, FD),
+                                       SourceLocation()).getScalarVal();
+      auto VAT = FD->getCapturedVLAType();
----------------
hfinkel wrote:
> Why not use Loc (S.getLocStart()) as the source location?
Ok

http://reviews.llvm.org/D5099






More information about the cfe-commits mailing list