[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