[PATCH] [C++11] Support for capturing of variable length arrays in lambda expression.
Richard Smith
richard at metafoo.co.uk
Mon Jul 21 23:58:11 PDT 2014
On Mon, Jul 21, 2014 at 10:58 PM, Bataev, Alexey <a.bataev at hotmail.com>
wrote:
> Richard, yes, I understood that. We have troubles in CodeGen in this case.
> Yes, I can call getSizeExpr() for VariableArrayType, by it is impossible to
> get llvm::Value* for the expression returned by getSizeExpr() in lambda
> operator().
You can do this in exactly the same way you do it in your current patch;
see attached patch for example.
> This value is calculated in parent function for lambda expr and we have to
> pass it somehow to lambda. Passing VariableArrayType* does not allow us to
> get an access to this value.
> We must not recalculate the value of expression from getSizeExpr(), we
> must use the value, calculated in parent function. That's why it is not
> enough just to pass VariableArrayType*, also we need to pass llvm::Value*
> for getSizeExpr(), calculated in parent function.
>
>
> Best regards,
> Alexey Bataev
> =============
> Software Engineer
> Intel Compiler Team
>
> 22 Июль 2014 г. 9:41:43, Richard Smith писал:
>
>> On Mon, Jul 21, 2014 at 10:14 PM, Bataev, Alexey <a.bataev at hotmail.com
>> <mailto:a.bataev at hotmail.com>> wrote:
>>
>> Richard, I don't think this is possible. If we storing
>> VariableLengthArray* on FieldDecl, then in Lambda operator we can
>> access VariableLengthArray->__getSizeExpr(). But! We won't be able
>>
>> to get the llvm::Value* for this
>> VariableLengthArray->__getSizeExpr(), calculated in another
>>
>> function. We should pass this llvm::Value* as a member of
>> RecordDecl for LambdaExpr! Here is a scheme:
>>
>> foo() {
>> int vla[n+m];
>> ....
>> some_lambda {vla....;}
>> ....
>> }
>>
>> What we have in CodeGen for this example:
>> CodeGenFunction(foo), llvm::DenseMap<const Expr*, llvm::Value*>
>> VLASizeMap[n+m]=value; VLASizeMap stores pre-calculated size of
>> each VLA.
>> {
>> FD->field_for_m+n_expr = (int [n+m]*)0;
>> CodeGenFunction CGF(some_lambda.operator(), FD);
>> }
>>
>> CodeGenFunction(some_lambda.__operator()), llvm::DenseMap<const
>>
>> Expr*, llvm::Value*> VLASizeMap[n+m]=????? - llvm::Value* for
>> 'n+m' is not captured here, we don't have an access to 'value',
>> calculated in CodeGenFunction(foo) for expression 'n+m'.
>>
>> To solve this problem we can try to capture 2 fields instead - one
>> for VariableLengthArray*, and another one for llvm::Value*
>> calculated for VariableLengthArray->__getSizeExpr().
>>
>>
>>
>> I'm sorry, I still think we're miscommunicating. What I'm suggesting
>> is a mechanical transformation: store the VariableArrayType* on the
>> FieldDecl, and then when you need the size expression that you're
>> currently storing there, call getSizeExpr() on the type.
>>
>> Best regards,
>> Alexey Bataev
>> =============
>> Software Engineer
>> Intel Compiler Team
>>
>> 22 Июль 2014 г. 8:57:54, Richard Smith писал:
>>
>> On Mon, Jul 21, 2014 at 5:43 AM, Alexey Bataev
>> <a.bataev at hotmail.com <mailto:a.bataev at hotmail.com>
>> <mailto:a.bataev at hotmail.com <mailto:a.bataev at hotmail.com>>>
>>
>> wrote:
>>
>> Updated version after last review. Unfortunately, I don't
>> think it
>> is possible to pass captured expression as a
>> VariableLengthArray
>> *. I have to capture expr of VariableLengthArray * and
>> then cast
>> it to SizeExpr->getType() type to make lambda capture this
>> type,
>> not VariableLengthArray *. I have to pass actual value of
>> SizeExpr
>> to the Lambda in this field, because it is defined only in
>> calling
>> function and in Lambda it can be received only in one of
>> captured
>> fields.
>>
>>
>> I don't understand what you're saying. What I'm suggesting is
>> storing
>> the VariableLengthArray* in the InitializerOrBitWidth field on
>> FieldDecl, instead of storing the array bound there.
>>
>> http://reviews.llvm.org/D4368
>>
>> Files:
>> include/clang/AST/Decl.h
>> include/clang/AST/__LambdaCapture.h
>> include/clang/Basic/__DiagnosticSemaKinds.td
>>
>> lib/AST/Decl.cpp
>> lib/AST/Expr.cpp
>> lib/AST/ExprCXX.cpp
>> lib/AST/StmtPrinter.cpp
>> lib/AST/StmtProfile.cpp
>> lib/CodeGen/CGDebugInfo.cpp
>> lib/CodeGen/CGExprCXX.cpp
>> lib/CodeGen/CodeGenFunction.__cpp
>> lib/Sema/SemaDecl.cpp
>> lib/Sema/SemaExpr.cpp
>> lib/Sema/TreeTransform.h
>> test/CodeGenCXX/instantiate-__typeof-vla.cpp
>> test/SemaTemplate/instantiate-__typeof.cpp
>> tools/libclang/IndexBody.cpp
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140721/d0f77638/attachment.html>
-------------- next part --------------
Index: test/SemaTemplate/instantiate-typeof.cpp
===================================================================
--- test/SemaTemplate/instantiate-typeof.cpp (revision 213614)
+++ test/SemaTemplate/instantiate-typeof.cpp (working copy)
@@ -1,10 +1,11 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// expected-no-diagnostics
// Make sure we correctly treat __typeof as potentially-evaluated when appropriate
template<typename T> void f(T n) {
- int buffer[n]; // expected-note {{declared here}}
- [] { __typeof(buffer) x; }(); // expected-error {{variable 'buffer' with variably modified type cannot be captured in a lambda expression}}
+ int buffer[n];
+ [&buffer] { __typeof(buffer) x; }();
}
int main() {
- f<int>(1); // expected-note {{in instantiation}}
+ f<int>(1);
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 213614)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -5320,9 +5320,6 @@
"'this' cannot be %select{implicitly |}0captured in this context">;
def err_lambda_capture_anonymous_var : Error<
"unnamed variable cannot be implicitly captured in a lambda expression">;
- def err_lambda_capture_vm_type : Error<
- "variable %0 with variably modified type cannot be captured in "
- "a lambda expression">;
def err_lambda_capture_flexarray_type : Error<
"variable %0 with flexible array member cannot be captured in "
"a lambda expression">;
Index: include/clang/Sema/ScopeInfo.h
===================================================================
--- include/clang/Sema/ScopeInfo.h (revision 213614)
+++ include/clang/Sema/ScopeInfo.h (working copy)
@@ -481,6 +481,11 @@
void addThisCapture(bool isNested, SourceLocation Loc, QualType CaptureType,
Expr *Cpy);
+ void addVLACapture(SourceLocation Loc, QualType SizeType) {
+ addCapture(nullptr, false, false, false, Loc, SourceLocation(), SizeType,
+ nullptr);
+ }
+
/// \brief Determine whether the C++ 'this' is captured.
bool isCXXThisCaptured() const { return CXXThisCaptureIndex != 0; }
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h (revision 213614)
+++ include/clang/AST/Decl.h (working copy)
@@ -2155,8 +2155,9 @@
mutable unsigned CachedFieldIndex : 31;
/// \brief An InClassInitStyle value, and either a bit width expression (if
- /// the InClassInitStyle value is ICIS_NoInit), or a pointer to the in-class
- /// initializer for this field (otherwise).
+ /// the InClassInitStyle value is ICIS_NoInit) in struct/class, or a captured
+ /// variable length array bound in a lambda expression, or a pointer to the
+ /// in-class initializer for this field (otherwise).
///
/// We can safely combine these two because in-class initializers are not
/// permitted for bit-fields.
@@ -2192,11 +2193,8 @@
/// isMutable - Determines whether this field is mutable (C++ only).
bool isMutable() const { return Mutable; }
- /// isBitfield - Determines whether this field is a bitfield.
- bool isBitField() const {
- return getInClassInitStyle() == ICIS_NoInit &&
- InitializerOrBitWidth.getPointer();
- }
+ /// \brief Determines whether this field is a bitfield.
+ bool isBitField() const;
/// @brief Determines whether this is an unnamed bitfield.
bool isUnnamedBitfield() const { return isBitField() && !getDeclName(); }
@@ -2252,6 +2250,18 @@
InitializerOrBitWidth.setInt(ICIS_NoInit);
}
+ /// \brief Determine whether this field is an implicit lambda capture for the
+ /// bound in a variable length array type.
+ bool hasCapturedVLABound() const;
+ /// \brief Get the captured variable length array.
+ const VariableArrayType *getCapturedVLABound() const {
+ return hasCapturedVLABound() ? reinterpret_cast<const VariableArrayType *>(
+ InitializerOrBitWidth.getPointer())
+ : nullptr;
+ }
+ /// \brief Set the captured variable length array for this field.
+ void setCapturedVLABound(const VariableArrayType *Captured);
+
/// getParent - Returns the parent of this field declaration, which
/// is the struct in which this method is defined.
const RecordDecl *getParent() const {
@@ -3136,6 +3146,10 @@
/// \endcode
bool isInjectedClassName() const;
+ /// \brief Determine whether this record is a class describing a lambda
+ /// function object.
+ bool isLambda() const;
+
/// getDefinition - Returns the RecordDecl that actually defines
/// this struct/union/class. When determining whether or not a
/// struct/union/class is completely defined, one should use this
Index: include/clang/AST/LambdaCapture.h
===================================================================
--- include/clang/AST/LambdaCapture.h (revision 213614)
+++ include/clang/AST/LambdaCapture.h (working copy)
@@ -68,13 +68,22 @@
/// \brief Determine whether this capture handles the C++ \c this
/// pointer.
- bool capturesThis() const { return DeclAndBits.getPointer() == nullptr; }
+ bool capturesThis() const {
+ return (DeclAndBits.getPointer() == nullptr) &&
+ !(DeclAndBits.getInt() & Capture_ByCopy);
+ }
/// \brief Determine whether this capture handles a variable.
bool capturesVariable() const {
return dyn_cast_or_null<VarDecl>(DeclAndBits.getPointer());
}
+ /// \brief Determine whether this captures a variable length array bound.
+ bool capturesVLABound() const {
+ return (DeclAndBits.getPointer() == nullptr) &&
+ (DeclAndBits.getInt() & Capture_ByCopy);
+ }
+
/// \brief Determine whether this is an init-capture.
bool isInitCapture() const {
return capturesVariable() && getCapturedVar()->isInitCapture();
Index: tools/libclang/IndexBody.cpp
===================================================================
--- tools/libclang/IndexBody.cpp (revision 213614)
+++ tools/libclang/IndexBody.cpp (working copy)
@@ -150,7 +150,7 @@
}
bool TraverseLambdaCapture(LambdaExpr *LE, const LambdaCapture *C) {
- if (C->capturesThis())
+ if (C->capturesThis() || C->capturesVLABound())
return true;
if (C->capturesVariable() && IndexCtx.shouldIndexFunctionLocalSymbols())
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 213614)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -11666,12 +11666,9 @@
}
// Prohibit variably-modified types; they're difficult to deal with.
- if (Var->getType()->isVariablyModifiedType() && (IsBlock || IsLambda)) {
+ if (Var->getType()->isVariablyModifiedType() && IsBlock) {
if (Diagnose) {
- if (IsBlock)
- S.Diag(Loc, diag::err_ref_vm_type);
- else
- S.Diag(Loc, diag::err_lambda_capture_vm_type) << Var->getDeclName();
+ S.Diag(Loc, diag::err_ref_vm_type);
S.Diag(Var->getLocation(), diag::note_previous_decl)
<< Var->getDeclName();
}
@@ -12212,14 +12209,33 @@
break;
case Type::VariableArray: {
// Losing element qualification here is fine.
- const VariableArrayType *Vat = cast<VariableArrayType>(Ty);
+ const VariableArrayType *VAT = cast<VariableArrayType>(Ty);
// Unknown size indication requires no size computation.
- // Otherwise, evaluate and record it.
- if (Expr *Size = Vat->getSizeExpr()) {
- MarkDeclarationsReferencedInExpr(Size);
+ // Otherwise, record it.
+ if (auto Size = VAT->getSizeExpr()) {
+ if (auto LSI = dyn_cast<LambdaScopeInfo>(CSI)) {
+ // Build the non-static data member.
+ auto Field = FieldDecl::Create(Context, LSI->Lambda,
+ ExprLoc, ExprLoc,
+ /*Id*/ nullptr,
+ Context.getSizeType(),
+ /*TInfo*/ nullptr,
+ /*BW*/ nullptr, /*Mutable*/ false,
+ /*InitStyle*/ ICIS_NoInit);
+ Field->setImplicit(true);
+ Field->setAccess(AS_private);
+ Field->setCapturedVLABound(VAT);
+ LSI->Lambda->addDecl(Field);
+
+ LSI->addVLACapture(ExprLoc, Field->getType());
+ } else {
+ // Immediately mark all referenced vars for CapturedStatements,
+ // they all are captured by reference.
+ MarkDeclarationsReferencedInExpr(Size);
+ }
}
- QTy = Vat->getElementType();
+ QTy = VAT->getElementType();
break;
}
case Type::FunctionProto:
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h (revision 213614)
+++ lib/Sema/TreeTransform.h (working copy)
@@ -8954,6 +8954,10 @@
getSema().CheckCXXThisCapture(C->getLocation(), C->isExplicit());
continue;
}
+ // Captured expression will be recaptured during captured variables
+ // rebuilding.
+ if (C->capturesVLABound())
+ continue;
// Rebuild init-captures, including the implied field declaration.
if (C->isInitCapture()) {
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp (revision 213614)
+++ lib/Sema/SemaDecl.cpp (working copy)
@@ -9833,6 +9833,7 @@
// Add the captures to the LSI so they can be noted as already
// captured within tryCaptureVar.
+ auto I = LambdaClass->field_begin();
for (const auto &C : LambdaClass->captures()) {
if (C.capturesVariable()) {
VarDecl *VD = C.getCapturedVar();
@@ -9849,7 +9850,10 @@
} else if (C.capturesThis()) {
LSI->addThisCapture(/*Nested*/ false, C.getLocation(),
S.getCurrentThisType(), /*Expr*/ nullptr);
+ } else {
+ LSI->addVLACapture(C.getLocation(), I->getType());
}
+ ++I;
}
}
Index: lib/AST/Decl.cpp
===================================================================
--- lib/AST/Decl.cpp (revision 213614)
+++ lib/AST/Decl.cpp (working copy)
@@ -3259,6 +3259,15 @@
return false;
}
+bool FieldDecl::isBitField() const {
+ if (getInClassInitStyle() == ICIS_NoInit &&
+ InitializerOrBitWidth.getPointer()) {
+ assert(getDeclContext() && "No parent context for FieldDecl");
+ return !getDeclContext()->isRecord() || !getParent()->isLambda();
+ }
+ return false;
+}
+
unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
assert(isBitField() && "not a bitfield");
Expr *BitWidth = InitializerOrBitWidth.getPointer();
@@ -3290,17 +3299,32 @@
}
void FieldDecl::setBitWidth(Expr *Width) {
+ assert(isBitField() && "not a bitfield");
assert(!InitializerOrBitWidth.getPointer() && !hasInClassInitializer() &&
- "bit width or initializer already set");
+ "bit width, initializer or captured expr already set");
InitializerOrBitWidth.setPointer(Width);
}
void FieldDecl::setInClassInitializer(Expr *Init) {
assert(!InitializerOrBitWidth.getPointer() && hasInClassInitializer() &&
- "bit width or initializer already set");
+ "bit width, initializer or captured expr already set");
InitializerOrBitWidth.setPointer(Init);
}
+bool FieldDecl::hasCapturedVLABound() const {
+ return getDeclContext()->isRecord() && getParent()->isLambda() &&
+ getInClassInitStyle() == ICIS_NoInit &&
+ InitializerOrBitWidth.getPointer();
+}
+
+void FieldDecl::setCapturedVLABound(const VariableArrayType *CapturedVLA) {
+ assert(getParent()->isLambda() && "capturing expression in non-lambda.");
+ assert(!InitializerOrBitWidth.getPointer() && !hasInClassInitializer() &&
+ "bit width, initializer or captured expr already set");
+ InitializerOrBitWidth.setPointer(
+ const_cast<Expr *>(reinterpret_cast<const Expr *>(CapturedVLA)));
+}
+
//===----------------------------------------------------------------------===//
// TagDecl Implementation
//===----------------------------------------------------------------------===//
@@ -3521,6 +3545,13 @@
cast<RecordDecl>(getDeclContext())->getDeclName() == getDeclName();
}
+bool RecordDecl::isLambda() const {
+ if (auto RD = dyn_cast<CXXRecordDecl>(this)) {
+ return RD->isLambda();
+ }
+ return false;
+}
+
RecordDecl::field_iterator RecordDecl::field_begin() const {
if (hasExternalLexicalStorage() && !LoadedFieldsFromExternalStorage)
LoadFieldsFromExternalStorage();
Index: lib/AST/StmtProfile.cpp
===================================================================
--- lib/AST/StmtProfile.cpp (revision 213614)
+++ lib/AST/StmtProfile.cpp (working copy)
@@ -1009,6 +1009,8 @@
for (LambdaExpr::capture_iterator C = S->explicit_capture_begin(),
CEnd = S->explicit_capture_end();
C != CEnd; ++C) {
+ if (C->capturesVLABound())
+ continue;
ID.AddInteger(C->getCaptureKind());
switch (C->getCaptureKind()) {
case LCK_This:
Index: lib/AST/ExprCXX.cpp
===================================================================
--- lib/AST/ExprCXX.cpp (revision 213614)
+++ lib/AST/ExprCXX.cpp (working copy)
@@ -905,7 +905,7 @@
case LCK_ByCopy:
Bits |= Capture_ByCopy;
- // Fall through
+ break;
case LCK_ByRef:
assert(Var && "capture must have a variable!");
break;
@@ -915,7 +915,8 @@
LambdaCaptureKind LambdaCapture::getCaptureKind() const {
Decl *D = DeclAndBits.getPointer();
- if (!D)
+ bool CapByCopy = DeclAndBits.getInt() & Capture_ByCopy;
+ if (!D && !CapByCopy)
return LCK_This;
return (DeclAndBits.getInt() & Capture_ByCopy) ? LCK_ByCopy : LCK_ByRef;
Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp (revision 213614)
+++ lib/AST/Expr.cpp (working copy)
@@ -2992,7 +2992,7 @@
const LambdaExpr *LE = cast<LambdaExpr>(this);
for (LambdaExpr::capture_iterator I = LE->capture_begin(),
E = LE->capture_end(); I != E; ++I)
- if (I->getCaptureKind() == LCK_ByCopy)
+ if (I->getCaptureKind() == LCK_ByCopy && I->capturesVariable())
// FIXME: Only has a side-effect if the variable is volatile or if
// the copy would invoke a non-trivial copy constructor.
return true;
Index: lib/AST/StmtPrinter.cpp
===================================================================
--- lib/AST/StmtPrinter.cpp (revision 213614)
+++ lib/AST/StmtPrinter.cpp (working copy)
@@ -1698,6 +1698,8 @@
CEnd = Node->explicit_capture_end();
C != CEnd;
++C) {
+ if (C->capturesVLABound())
+ continue;
if (NeedComma)
OS << ", ";
NeedComma = true;
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp (revision 213614)
+++ lib/CodeGen/CGExprCXX.cpp (working copy)
@@ -1800,19 +1800,23 @@
void CodeGenFunction::EmitLambdaExpr(const LambdaExpr *E, AggValueSlot Slot) {
RunCleanupsScope Scope(*this);
- LValue SlotLV = MakeAddrLValue(Slot.getAddr(), E->getType(),
- Slot.getAlignment());
+ LValue SlotLV =
+ MakeAddrLValue(Slot.getAddr(), E->getType(), Slot.getAlignment());
CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin();
for (LambdaExpr::capture_init_iterator i = E->capture_init_begin(),
e = E->capture_init_end();
i != e; ++i, ++CurField) {
// Emit initialization
-
LValue LV = EmitLValueForFieldInitialization(SlotLV, *CurField);
- ArrayRef<VarDecl *> ArrayIndexes;
- if (CurField->getType()->isArrayType())
- ArrayIndexes = E->getCaptureInitIndexVars(i);
- EmitInitializerForField(*CurField, LV, *i, ArrayIndexes);
+ if (CurField->hasCapturedVLABound()) {
+ auto *VAT = CurField->getCapturedVLABound();
+ EmitStoreThroughLValue(RValue::get(VLASizeMap[VAT->getSizeExpr()]), LV);
+ } else {
+ ArrayRef<VarDecl *> ArrayIndexes;
+ if (CurField->getType()->isArrayType())
+ ArrayIndexes = E->getCaptureInitIndexVars(i);
+ EmitInitializerForField(*CurField, LV, *i, ArrayIndexes);
+ }
}
}
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp (revision 213614)
+++ lib/CodeGen/CodeGenFunction.cpp (working copy)
@@ -663,6 +663,12 @@
CXXThisValue = EmitLoadOfLValue(ThisLValue,
SourceLocation()).getScalarVal();
}
+ for (auto *FD : MD->getParent()->fields()) {
+ if (FD->hasCapturedVLABound())
+ VLASizeMap[FD->getCapturedVLABound()->getSizeExpr()] =
+ EmitLoadOfLValue(EmitLValueForLambdaField(FD), SourceLocation())
+ .getScalarVal();
+ }
} else {
// Not in a lambda; just use 'this' from the method.
// FIXME: Should we generate a new load for each use of 'this'? The
Index: lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- lib/CodeGen/CGDebugInfo.cpp (revision 213614)
+++ lib/CodeGen/CGDebugInfo.cpp (working copy)
@@ -850,12 +850,11 @@
C.getLocation(), Field->getAccess(),
layout.getFieldOffset(fieldno), VUnit, RecordTy);
elements.push_back(fieldType);
- } else {
+ } else if (C.capturesThis()) {
// TODO: Need to handle 'this' in some way by probably renaming the
// this of the lambda class and having a field member of 'this' or
// by using AT_object_pointer for the function and having that be
// used as 'this' for semantic references.
- assert(C.capturesThis() && "Field that isn't captured and isn't this?");
FieldDecl *f = *Field;
llvm::DIFile VUnit = getOrCreateFile(f->getLocation());
QualType type = f->getType();
More information about the cfe-commits
mailing list