[C++11] Support for capturing of variable length arrays in lambda expression.

[ABataev]

Richard, could please review this patch?
Best regards,
Alexey Bataev

[rsmith]

The C++ Array Extensions draft TS specifies how the C++ committee thinks VLAs in C++ ("Arrays of Runtime Bound") should work, and I think it has different semantics for lambda captures from those provided here =( In particular, it only allows the array bound expression to be evaluated a single time, so the array bound itself must be captured, rather than the variables that it references.

So, we have a choice: we can go with what you have here (which seems reasonable if we're providing the C-style VLAs), and we should do something different if instead we want to align ourselves with the Array Extensions draft TS and ARB semantics. If Array Extensions is going to become a full TS, then we'll want to align with it eventually, and it doesn't make sense to me to differ from it now, but it's not completely clear if that's going to happen (there seemed to be weak consensus at the previous committee meeting to abandon it).

Thoughts?

[hfinkel]

• Original Message -----

From: "Richard Smith" <[e-mail]>
To: "a bataev" <[e-mail]>, [e-mail], [e-mail], [e-mail], [e-mail]
Cc: [e-mail]
Sent: Sunday, July 13, 2014 7:17:59 PM
Subject: Re: [PATCH] [C++11] Support for capturing of variable length arrays in lambda expression.

The C++ Array Extensions draft TS
specifies how the C++ committee thinks VLAs in C++ ("Arrays of
Runtime Bound") should work, and I think it has different semantics
for lambda captures from those provided here =( In particular, it
only allows the array bound expression to be evaluated a single
time, so the array bound itself must be captured, rather than the
variables that it references.

So, we have a choice: we can go with what you have here (which seems
reasonable if we're providing the C-style VLAs), and we should do
something different if instead we want to align ourselves with the
Array Extensions draft TS and ARB semantics. If Array Extensions is
going to become a full TS, then we'll want to align with it
eventually, and it doesn't make sense to me to differ from it now,
but it's not completely clear if that's going to happen (there
seemed to be weak consensus at the previous committee meeting to
abandon it).

Thoughts?

Regarding the committee, I think there is weak consensus to replace dynarray with some other container-compatible stack-based allocation scheme, and no particularly clear idea what that should be. There are also strong objections to having ARBs without some container-compatible scheme. To the question at hand, I think it would be really valuable to have some experience attempting to implement the proposed ARB semantics, and I'm certainly interested in hearing what the downsides are compared to the semantics we have here (and are using for OpenMP).

-Hal

http://reviews.llvm.org/D4368

[ABataev]

Richard,
Then the only possible solution is to create some temp var for array
size and capture it (or make it global) + rebuild the original type to
make it use this temp var. Another solution is to learn the capturing
machinery to capture expressions.

Best regards,

Alexey Bataev

Software Engineer
Intel Compiler Team

14.07.2014 4:17, Richard Smith пишет:

The C++ Array Extensions draft TS specifies how the C++ committee thinks VLAs in C++ ("Arrays of Runtime Bound") should work, and I think it has different semantics for lambda captures from those provided here =( In particular, it only allows the array bound expression to be evaluated a single time, so the array bound itself must be captured, rather than the variables that it references.

So, we have a choice: we can go with what you have here (which seems reasonable if we're providing the C-style VLAs), and we should do something different if instead we want to align ourselves with the Array Extensions draft TS and ARB semantics. If Array Extensions is going to become a full TS, then we'll want to align with it eventually, and it doesn't make sense to me to differ from it now, but it's not completely clear if that's going to happen (there seemed to be weak consensus at the previous committee meeting to abandon it).

Thoughts?

http://reviews.llvm.org/D4368

[ABataev]

Reworked version of the original patch with array dimension size capturing instead of var capturing.

[rsmith]

Please add a serialization / deserialization test.

include/clang/AST/Decl.h

2254–2256

I can't parse this sentence.

2257–2259

I think this should be made more specific: this actually refers specifically to capturing a VLA bound, so it should be named thusly.

include/clang/AST/LambdaCapture.h

81

"... captures an expression."

82

Please make this instead be capturesVLABound, since that's what it's for.

lib/AST/Decl.cpp

3265–3269

Please update the comment for InitializerOrBitWidth to describe what you're doing here.

3333

It's not really safe to track the captured expression like this: we don't guarantee to preserve expression pointer equality across serialization and deserialization if the expression is reachable from multiple parents, as this one will be.

Instead, how about passing in and storing a VariableArrayType here?

[ABataev]

Hi Richard,
Yes, I already found that there are some problems with the
serialization/deserialization (bu after I published the patch). I'll try
to fix it ASAP.

Best regards,

Alexey Bataev

Software Engineer
Intel Compiler Team

17.07.2014 4:09, Richard Smith пишет:

Please add a serialization / deserialization test.

Comment at: include/clang/AST/Decl.h:2253-2255
@@ -2255,1 +2252,5 @@

+ / \brief Determine whether this member has a reference to captured
+ / expression for structs/classes generates for list of variables captured
+ /// in lambda expressions.

+ bool hasCapturedExpr() const;

I can't parse this sentence.

Comment at: include/clang/AST/Decl.h:2256-2258
@@ +2255,5 @@
+ / in lambda expressions.
+ bool hasCapturedExpr() const;
+ / \brief Get the captured expression if any.
+ Expr *getCapturedExpr() const {

+ return hasCapturedExpr() ? InitializerOrBitWidth.getPointer() : nullptr;

I think this should be made more specific: this actually refers specifically to capturing a VLA bound, so it should be named thusly.

Comment at: include/clang/AST/LambdaCapture.h:81
@@ -77,1 +80,3 @@

+ /// \brief Determine whether this captures expression.

+ bool capturesExpression() const {

"... captures an expression."

Comment at: include/clang/AST/LambdaCapture.h:82
@@ +81,3 @@
+ /// \brief Determine whether this captures expression.
+ bool capturesExpression() const {

+ return (DeclAndBits.getPointer() == nullptr) &&

Please make this instead be capturesVLABound, since that's what it's for.

Comment at: lib/AST/Decl.cpp:3265-3269
@@ +3264,7 @@
+ InitializerOrBitWidth.getPointer()) {
+ if (getDeclContext() && getDeclContext()->isRecord()) {
+ if (auto *RD = dyn_cast<CXXRecordDecl>(getParent())) {
+ return !RD->isLambda();
+ }
+ }

+ return true;

Please update the comment for InitializerOrBitWidth to describe what you're doing here.

Comment at: lib/AST/Decl.cpp:3333
@@ +3332,3 @@
+ "bit width, initializer or captured expr already set");
+ InitializerOrBitWidth.setPointer(CapturedExpr);

+}

It's not really safe to track the captured expression like this: we don't guarantee to preserve expression pointer equality across serialization and deserialization if the expression is reachable from multiple parents, as this one will be.

Instead, how about passing in and storing a VariableArrayType here?

http://reviews.llvm.org/D4368

[ABataev]

Fixed serialization/deserialization problems.

[ABataev]

Cosmetic update per Richard Smith comments.

[rsmith]

lib/AST/Decl.cpp

3265

You don't need the isRecord check here, and you shouldn't need the getDeclContext check either. If some buggy code is querying this when there is no DeclContext, it'd be better to crash or assert because you cannot give a correct answer.

3309

Please assert(isBitField()) or similar here to ensure that we don't set a bit width on a lambda capture.

3328–3329

Given the number of times you do this isa<CXXRecordDecl> && isLambda check, I'd prefer for you to add an isLambda member to RecordDecl and have it do the dyn_cast to CXXRecordDecl.

lib/Sema/SemaExpr.cpp

12220–12235

This seems like a hack: could you instead make the FieldDecl directly store the VariableArrayType *, rather than creating a proxy expression and variable to carry it?

[ABataev]

lib/AST/Decl.cpp

3265

I need it, because there are classes ObjCIvarDecl and ObjCAtDefsFieldDecl, derived from FieldDecl, and the parents for instances of these classes are not RecordDecl, but some ObjC specific constructs.

3309

Ok

3328–3329

Ok

lib/Sema/SemaExpr.cpp

12220–12235

I'll try, but I'm not sure that it will work. I'm not sure can we store VLA as the element of the structure in LLVM IR? I'll try to improve it.

[ABataev]

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.

[ABataev]

Capture VariableArrayType * in Lambdas instead of expressions.

[rsmith]

Thanks, this looks much nicer.

This needs more testcases, covering at least:

• serialization,
• generic lambdas,
• capturing multiple array bounds from a single variable,
• referencing the same VLA type multiple times in a single lambda-expression,
• applying sizeof to a captured VLA type within a lambda expression.

include/clang/AST/Decl.h

2258–2264

Can you use VariableArrayType* instead of just Type* here?

2259–2260

Get the Expr from the VariableArrayType here.

2264

Pass in the VariableArrayType instead of its Expr here.

include/clang/Sema/ScopeInfo.h

405–409 ↗

(On Diff #11745)

This seems unnecessary; the Capture does not need to know the type.

433 ↗

(On Diff #11745)

Please call this isVLACapture() so that a reader can tell what it's for.

496 ↗

(On Diff #11745)

There cannot be an ellipsis, because a VLA capture is always implicit.

lib/CodeGen/CGExprCXX.cpp

1781–1791

Directly grab the VariableArrayType from CurField here.

1783

This QBlah naming convention is not one we use. We'd usually call this VAT or similar.

lib/CodeGen/CodeGenFunction.cpp

675–680

... and here

lib/Sema/SemaDecl.cpp

9774

You shouldn't need to pass in an expression here: CodeGen initializes the field itself, and in any case, this expression is wrong, because you're not allowed to evaluate the VLA size expression again at the point where the closure object is created.

lib/Sema/SemaExpr.cpp

12218–12255

You shouldn't do this if LSI has already captured this VLA type.

12220–12227

Delete this. Instead, just set the captured VLA type on the FieldDecl.

12221

This seems like the wrong type; VLA bounds are always implicitly converted to size_t, but we don't model that conversion in the size expression. I'd suggest just always using Context.getSizeType() here. Otherwise you'll get asserts in CodeGen because its widened type doesn't match the captured field type.

12235

This doesn't seem like a good location for the capture; ExprLoc would be better.

12239–12241

Likewise, you shouldn't need to pass in CapExpr here.

test/CodeGenCXX/instantiate-typeof-vla.cpp

2

Please name this test as something to do with VLAs and lambdas. __typeof is incidental here.

[ABataev]

Testcase:

serialization, - done(see head of the new test in CodeGenCXX)
generic lambdas - there is one, I'll add more,
capturing multiple array bounds from a single variable - will be added,
referencing the same VLA type multiple times in a single lambda-expression - will be added,
applying sizeof to a captured VLA type within a lambda expression - will be added.

include/clang/AST/Decl.h

2258–2264

Will do

include/clang/Sema/ScopeInfo.h

405–409 ↗

(On Diff #11745)

Ok, I'll remove it

433 ↗

(On Diff #11745)

Ok

496 ↗

(On Diff #11745)

Ok

lib/CodeGen/CGExprCXX.cpp

1783

Ok

lib/Sema/SemaExpr.cpp

12218–12255

Ok, I'll fix it

12221

Ok, I'll fix it

12235

Ok

test/CodeGenCXX/instantiate-typeof-vla.cpp

2

Ok

[ABataev]

Richard, any comments on the updated patch?
Best regards,

Alexey Bataev

---

Software Engineer
Intel Compiler Team

[ABataev]

Richard?

[rsmith]

include/clang/AST/LambdaCapture.h

67

Have you considered adding an LCK_ value for VLA capture? Calling it LCK_ByCopy doesn't seem quite right.

lib/AST/Decl.cpp

3306–3308

This pair of asserts doesn't make sense. A field can only be a bit-field if it has a bit-width, but you're asserting that it's a bit-field *and* has no bit-width.

This is only passing the tests because setBitWidth is never called. Please remove the assert you added here. (I suspect this function is called by lldb or similar, otherwise I'd just suggest removing it entirely.)

3319

Is this check necessary? You can't have an in-class initializer in a lambda.

3557–3559

No braces here, please.

lib/AST/StmtPrinter.cpp

1626–1627

Is this possible? I would have expected VLA captures to always be implicit.

lib/AST/StmtProfile.cpp

957–958

Likewise.

lib/Sema/SemaExpr.cpp

11670

Update this comment: "Prohibit variably-modified types in blocks; ..." maybe?

12077–12084

Please make this a member of CapturingScopeInfo. Also, drop one of the Iss from its name.

[ABataev]

Richard, here are my comments. The updated patch is ready and I'll upload it soon.

include/clang/AST/LambdaCapture.h

67

Yes, I tried this. But the problem is that there is no more space for another one LCK_ value in field DeclAndBits. It is declared as llvm::PointerIntPair<Decl *, 2> and its integer argument may have values Capture_Implicit = 0x01 or Capture_ByCopy = 0x02. I can't add vallue 0x03v without changing llvm::PointerIntPair<Decl *, 2> to llvm::PointerIntPair<Decl *, 3>. The only way to do it is to split DeclAndBits into 2 independent fields - pointer and flags.

lib/AST/Decl.cpp

3306–3308

Yers, you're right. I'll remove assert. The method is used by some tools and I think we should keep it.

3319

No, it is not neccessary, just double checked some conditions. I think it can be removed.

3557–3559

Removed

lib/AST/StmtPrinter.cpp

1626–1627

You're right, removed.

lib/AST/StmtProfile.cpp

957–958

Removed too.

lib/Sema/SemaExpr.cpp

11670

Done.

12077–12084

Ok

[ABataev]

Improved code after review.

[rsmith]

include/clang/AST/LambdaCapture.h

67

DeclAndBits stores Capture_ values, not LCK_ values. The LCK_ values are defined in Basic/Lambda.h and there are only 3 different values right now.

[ABataev]

Oh, yes, you're right. I'll try to improve it then.

Best regards,

Alexey Bataev

Software Engineer
Intel Compiler Team

26.08.2014 8:33, Richard Smith пишет:

Comment at: include/clang/AST/LambdaCapture.h:67
@@ -66,3 +66,3 @@

/// \brief Determine the kind of capture.
LambdaCaptureKind getCaptureKind() const;

---

ABataev wrote:

rsmith wrote:

Have you considered adding an LCK_ value for VLA capture? Calling it LCK_ByCopy doesn't seem quite right.

Yes, I tried this. But the problem is that there is no more space for another one LCK_ value in field DeclAndBits. It is declared as llvm::PointerIntPair<Decl *, 2> and its integer argument may have values Capture_Implicit = 0x01 or Capture_ByCopy = 0x02. I can't add vallue 0x03v without changing llvm::PointerIntPair<Decl *, 2> to llvm::PointerIntPair<Decl *, 3>. The only way to do it is to split DeclAndBits into 2 independent fields - pointer and flags.

DeclAndBits stores Capture_ values, not LCK_ values. The LCK_ values are defined in Basic/Lambda.h and there are only 3 different values right now.

http://reviews.llvm.org/D4368

[ABataev]

Added LCK_VLAType for capturing variable-length array types.

[ABataev]

Restored some deleted content.

[rsmith]

Thanks! LGTM.

[ABataev]

Closed by commit rL216649 (authored by @ABataev).

include/clang/AST/Decl.h

 /// FieldDecl - An instance of this class is created by Sema::ActOnField to
 /// represent a member of a struct/union/class.
 class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
   // FIXME: This can be packed into the bitfields in Decl.
   bool Mutable : 1;
   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.
   ///
   /// If the InClassInitStyle is not ICIS_NoInit and the initializer is null,
   /// then this field has an in-class initializer which has not yet been parsed
   /// and attached.
   llvm::PointerIntPair<Expr *, 2, unsigned> InitializerOrBitWidth;
public:
   
   /// getFieldIndex - Returns the index of this field within its record,
   /// as appropriate for passing to ASTRecordLayout::getFieldOffset.
   unsigned getFieldIndex() const;
 
   /// 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(); }
 
   /// isAnonymousStructOrUnion - Determines whether this field is a
   /// representative for an anonymous struct or union. Such fields are
   /// unnamed and are implicitly generated by the implementation to
   /// store the data for the anonymous union or struct.
public:
   /// removeInClassInitializer - Remove the C++11 in-class initializer from this
   /// member.
   void removeInClassInitializer() {
     assert(hasInClassInitializer() && "no initializer to remove");
     InitializerOrBitWidth.setPointer(nullptr);
     InitializerOrBitWidth.setInt(ICIS_NoInit);
   }
 
+  /// \brief Determine whether this member captures the expression for variable
+  /// length arrays bounds.
+  bool hasCapturedVLABoundExpr() const;
+  /// \brief Get the captured variable length array bound expression.
+  Expr *getCapturedVLABoundExpr() const {
+    return hasCapturedVLABoundExpr() ? InitializerOrBitWidth.getPointer()
+                                     : nullptr;
+  }
+  /// \brief Set the captured variable length array bound expression for this
+  /// field.
+  void setCapturedVLABoundExpr(Expr *CapturedExpr);
+
   /// getParent - Returns the parent of this field declaration, which
   /// is the struct in which this method is defined.
   const RecordDecl *getParent() const {
     return cast<RecordDecl>(getDeclContext());
   }
 
   RecordDecl *getParent() {
     return cast<RecordDecl>(getDeclContext());

include/clang/AST/LambdaCapture.h

public:
   /// \param EllipsisLoc The location of the ellipsis (...) for a
   /// capture that is a pack expansion, or an invalid source
   /// location to indicate that this is not a pack expansion.
   LambdaCapture(SourceLocation Loc, bool Implicit, LambdaCaptureKind Kind,
                 VarDecl *Var = nullptr,
                 SourceLocation EllipsisLoc = SourceLocation());
 
   /// \brief Determine the kind of capture.
   LambdaCaptureKind getCaptureKind() const;
 
   /// \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
+  /// expression.
+  bool capturesVLABoundExpression() const {
+    return (DeclAndBits.getPointer() == nullptr) &&
+           (DeclAndBits.getInt() & Capture_ByCopy);
+  }
+
   /// \brief Determine whether this is an init-capture.
   bool isInitCapture() const {
     return capturesVariable() && getCapturedVar()->isInitCapture();
   }
 
   /// \brief Retrieve the declaration of the local variable being
   /// captured.
   ///

include/clang/Basic/DiagnosticSemaKinds.td

def err_capture_does_not_name_variable : Error<
     "%0 in capture list does not name a variable">;
   def err_capture_non_automatic_variable : Error<
     "%0 cannot be captured because it does not have automatic storage "
     "duration">;
   def err_this_capture : Error<
     "'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">;
   def err_lambda_impcap : Error<
     "variable %0 cannot be implicitly captured in a lambda with no "
     "capture-default specified">;
   def note_lambda_decl : Note<"lambda expression begins here">;
   def err_lambda_unevaluated_operand : Error<

lib/AST/Decl.cpp

if (!isImplicit() || getDeclName())
     return false;
 
   if (const RecordType *Record = getType()->getAs<RecordType>())
     return Record->getDecl()->isAnonymousStructOrUnion();
 
   return false;
 }
 
+bool FieldDecl::isBitField() const {
+  if (getInClassInitStyle() == ICIS_NoInit &&
+      InitializerOrBitWidth.getPointer()) {
+    if (getDeclContext() && getDeclContext()->isRecord()) {
+      if (auto *RD = dyn_cast<CXXRecordDecl>(getParent())) {
+        return !RD->isLambda();
+      }
+    }
+    return true;
+  }
+  return false;
+}
+
 unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
   assert(isBitField() && "not a bitfield");
   Expr *BitWidth = InitializerOrBitWidth.getPointer();
   return BitWidth->EvaluateKnownConstInt(Ctx).getZExtValue();
 }
 
 unsigned FieldDecl::getFieldIndex() const {
   const FieldDecl *Canonical = getCanonicalDecl();

 
 SourceRange FieldDecl::getSourceRange() const {
   if (const Expr *E = InitializerOrBitWidth.getPointer())
     return SourceRange(getInnerLocStart(), E->getLocEnd());
   return DeclaratorDecl::getSourceRange();
 }
 
 void FieldDecl::setBitWidth(Expr *Width) {
   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::hasCapturedVLABoundExpr() const {
+  if (getDeclContext() && getDeclContext()->isRecord()) {
+    if (auto *RD = dyn_cast<CXXRecordDecl>(getParent())) {
+      return RD->isLambda() && getInClassInitStyle() == ICIS_NoInit &&
+             InitializerOrBitWidth.getPointer();
+    }
+  }
+  return false;
+}
+
+void FieldDecl::setCapturedVLABoundExpr(Expr *CapturedExpr) {
+  assert(isa<CXXRecordDecl>(getParent()) &&
+         cast<CXXRecordDecl>(getParent())->isLambda() &&
+         "capturing expression in non-lambda.");
+  assert(!InitializerOrBitWidth.getPointer() && !hasInClassInitializer() &&
+         "bit width, initializer or captured expr already set");
+  InitializerOrBitWidth.setPointer(CapturedExpr);
+}
+
 //===----------------------------------------------------------------------===//
 // TagDecl Implementation
 //===----------------------------------------------------------------------===//
 
 SourceLocation TagDecl::getOuterLocStart() const {
   return getTemplateOrInnerLocStart(this);
 }
 

 }
 
 bool RecordDecl::isInjectedClassName() const {
   return isImplicit() && getDeclName() && getDeclContext()->isRecord() &&
     cast<RecordDecl>(getDeclContext())->getDeclName() == getDeclName();
 }
 
 RecordDecl::field_iterator RecordDecl::field_begin() const {
   if (hasExternalLexicalStorage() && !LoadedFieldsFromExternalStorage)
     LoadFieldsFromExternalStorage();
 
   return field_iterator(decl_iterator(FirstDecl));
 }
 
 /// completeDefinition - Notes that the definition of this type is now
 /// complete.
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();

lib/AST/Expr.cpp

case CXXTemporaryObjectExprClass: {
     // at its arguments.
     break;
   }
 
   case LambdaExprClass: {
     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;
     return false;
   }
 
   case PseudoObjectExprClass: {
     // Only look for side-effects in the semantic form, and look past

lib/AST/ExprCXX.cpp

LambdaCapture::LambdaCapture(SourceLocation Loc, bool Implicit,
   
   switch (Kind) {
   case LCK_This:
     assert(!Var && "'this' capture cannot have a variable!");
     break;
 
   case LCK_ByCopy:
     Bits |= Capture_ByCopy;
-    // Fall through 
+    break;
   case LCK_ByRef:
     assert(Var && "capture must have a variable!");
     break;
   }
   DeclAndBits.setInt(Bits);
 }
 
 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;
 }
 
 LambdaExpr::LambdaExpr(QualType T,
                        SourceRange IntroducerRange,
                        LambdaCaptureDefault CaptureDefault,

lib/AST/StmtPrinter.cpp

case LCD_ByRef:
     OS << '&';
     NeedComma = true;
     break;
   }
   for (LambdaExpr::capture_iterator C = Node->explicit_capture_begin(),
                                  CEnd = Node->explicit_capture_end();
        C != CEnd;
        ++C) {
+    if (C->capturesVLABoundExpression())
+      continue;
     if (NeedComma)
       OS << ", ";
     NeedComma = true;
 
     switch (C->getCaptureKind()) {
     case LCK_This:
       OS << "this";
       break;

lib/AST/StmtProfile.cpp

 }
 
 void
 StmtProfiler::VisitLambdaExpr(const LambdaExpr *S) {
   VisitExpr(S);
   for (LambdaExpr::capture_iterator C = S->explicit_capture_begin(),
                                  CEnd = S->explicit_capture_end();
        C != CEnd; ++C) {
+    if (C->capturesVLABoundExpression())
+      continue;
     ID.AddInteger(C->getCaptureKind());
     switch (C->getCaptureKind()) {
     case LCK_This:
       break;
     case LCK_ByRef:
     case LCK_ByCopy:
       VisitDecl(C->getCapturedVar());
       ID.AddBoolean(C->isPackExpansion());

lib/CodeGen/CGDebugInfo.cpp

if (C.capturesVariable()) {
         SizeInBitsOverride = Field->getBitWidthValue(CGM.getContext());
         assert(SizeInBitsOverride && "found named 0-width bitfield");
       }
       llvm::DIType fieldType
         = createFieldType(VName, Field->getType(), SizeInBitsOverride,
                           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();
       llvm::DIType fieldType
         = createFieldType("this", type, 0, f->getLocation(), f->getAccess(),
                           layout.getFieldOffset(fieldno), VUnit, RecordTy);
 
       elements.push_back(fieldType);

lib/CodeGen/CGExprCXX.cpp

if (ShouldNullCheckSrcValue) {
     Value = PHI;
   }
 
   return Value;
 }
 
 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);
+    if (CurField->hasCapturedVLABoundExpr()) {
+      assert(isa<BinaryOperator>(*i) &&
+             "Captured expression is not a binary operator.");
+      auto QArrayType = cast<VariableArrayType>(cast<BinaryOperator>(*i)
+                                                    ->getLHS()
+                                                    ->getType()
+                                                    ->getAsArrayTypeUnsafe());
+      EmitStoreThroughLValue(RValue::get(VLASizeMap[QArrayType->getSizeExpr()]),
+                             LV);
+    } else {
       ArrayRef<VarDecl *> ArrayIndexes;
       if (CurField->getType()->isArrayType())
         ArrayIndexes = E->getCaptureInitIndexVars(i);
       EmitInitializerForField(*CurField, LV, *i, ArrayIndexes);
     }
   }
+}

lib/CodeGen/CodeGenFunction.cpp

if (MD->getParent()->isLambda() &&
       MD->getParent()->getCaptureFields(LambdaCaptureFields,
                                         LambdaThisCaptureField);
       if (LambdaThisCaptureField) {
         // If this lambda captures this, load it.
         LValue ThisLValue = EmitLValueForLambdaField(LambdaThisCaptureField);
         CXXThisValue = EmitLoadOfLValue(ThisLValue,
                                         SourceLocation()).getScalarVal();
       }
+      for (auto *FD : MD->getParent()->fields()) {
+        if (FD->hasCapturedVLABoundExpr()) {
+          auto *ExprArg = EmitLoadOfLValue(EmitLValueForLambdaField(FD),
+                                           SourceLocation()).getScalarVal();
+          assert(isa<BinaryOperator>(FD->getCapturedVLABoundExpr()) &&
+                 "Captured expression is not a binary operator.");
+          auto QArrayType = cast<VariableArrayType>(
+              cast<BinaryOperator>(FD->getCapturedVLABoundExpr())
+                  ->getLHS()
+                  ->getType()
+                  ->getAsArrayTypeUnsafe());
+          VLASizeMap[QArrayType->getSizeExpr()] = ExprArg;
+        }
+      }
     } else {
       // Not in a lambda; just use 'this' from the method.
       // FIXME: Should we generate a new load for each use of 'this'?  The
       // fast register allocator would be happier...
       CXXThisValue = CXXABIThisValue;
     }
   }
 

lib/Sema/SemaDecl.cpp

else if (LCD == LCD_ByRef)
     LSI->ImpCaptureStyle = CapturingScopeInfo::ImpCap_LambdaByref;
   DeclarationNameInfo DNI = CallOperator->getNameInfo();
     
   LSI->IntroducerRange = DNI.getCXXOperatorNameRange(); 
   LSI->Mutable = !CallOperator->isConst();
 
   // 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();
       if (VD->isInitCapture())
         S.CurrentInstantiationScope->InstantiatedLocal(VD, VD);
       QualType CaptureType = VD->getType();
       const bool ByRef = C.getCaptureKind() == LCK_ByRef;
       LSI->addCapture(VD, /*IsBlock*/false, ByRef, 
           /*RefersToEnclosingLocal*/true, C.getLocation(),
           /*EllipsisLoc*/C.isPackExpansion() 
                          ? C.getEllipsisLoc() : SourceLocation(),
           CaptureType, /*Expr*/ nullptr);
 
     } else if (C.capturesThis()) {
       LSI->addThisCapture(/*Nested*/ false, C.getLocation(), 
                               S.getCurrentThisType(), /*Expr*/ nullptr);
+    } else {
+      LSI->addCapture(nullptr, /*IsBlock*/ false, /*ByRef*/ false,
+                      /*RefersToEnclosingLocal*/ false, C.getLocation(),
+                      /*EllipsisLoc*/ C.isPackExpansion() ? C.getEllipsisLoc()
+                                                          : SourceLocation(),
+                      I->getType(), I->getCapturedVLABoundExpr());
     }
+    ++I;
   }
 }
 
 Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D) {
   // Clear the last template instantiation error context.
   LastTemplateInstantiationErrorContext = ActiveTemplateInstantiation();
   
   if (!D)

lib/Sema/SemaExpr.cpp

static bool isVariableCapturable(CapturingScopeInfo *CSI, VarDecl *Var,
   if (IsLambda && !Var->getDeclName()) {
     if (Diagnose) {
       S.Diag(Loc, diag::err_lambda_capture_anonymous_var);
       S.Diag(Var->getLocation(), diag::note_declared_at);
     }
     return false;
   }
 
   // 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(Var->getLocation(), diag::note_previous_decl) 
         << Var->getDeclName();
     }
     return false;
   }
   // Prohibit structs with flexible array members too.
   // We cannot capture what is in the tail end of the struct.
   if (const RecordType *VTTy = Var->getType()->getAs<RecordType>()) {
static bool captureInLambda(LambdaScopeInfo *LSI,
   // Add the capture.
   if (BuildAndDiagnose)
     LSI->addCapture(Var, /*IsBlock=*/false, ByRef, RefersToEnclosingLocal, 
                     Loc, EllipsisLoc, CaptureType, CopyExpr);
       
   return true;
 }
 
 
 bool Sema::tryCaptureVariable(VarDecl *Var, SourceLocation ExprLoc, 
                               TryCaptureKind Kind, SourceLocation EllipsisLoc,
                               bool BuildAndDiagnose, 
                               QualType &CaptureType,
                               QualType &DeclRefType,
                             const unsigned *const FunctionScopeIndexToStopAt) {
   bool Nested = false;
   
   DeclContext *DC = CurContext;
   const unsigned MaxFunctionScopesIndex = FunctionScopeIndexToStopAt 
       ? *FunctionScopeIndexToStopAt : FunctionScopes.size() - 1;  
   // We need to sync up the Declaration Context with the
   // FunctionScopeIndexToStopAt
   if (FunctionScopeIndexToStopAt) {
     unsigned FSIndex = FunctionScopes.size() - 1;
#include "clang/AST/TypeNodes.def"
           QTy = cast<ArrayType>(Ty)->getElementType();
           break;
         case Type::VariableArray: {
           // Losing element qualification here is fine.
           const VariableArrayType *Vat = cast<VariableArrayType>(Ty);
 
           // Unknown size indication requires no size computation.
           // Otherwise, evaluate and record it.
-          if (Expr *Size = Vat->getSizeExpr()) {
+          if (auto Size = Vat->getSizeExpr()) {
+            if (auto LSI = dyn_cast<LambdaScopeInfo>(CSI)) {
+              auto Loc = Size->getExprLoc();
+              auto QType = Size->getType();
+              auto QArrayType = QualType(Vat, 0);
+              auto TInfo = Context.getTrivialTypeSourceInfo(QType, Loc);
+              auto QArrayTInfo =
+                  Context.getTrivialTypeSourceInfo(QArrayType, Loc);
+              auto VD = VarDecl::Create(Context, getFunctionLevelDeclContext(),
+                                        SourceLocation(), SourceLocation(),
+                                        &Context.Idents.get(".captured_expr."),
+                                        QArrayType, QArrayTInfo, SC_Auto);
+              auto DRE =
+                  new (Context) DeclRefExpr(VD, /*refersToEnclosingLocal*/ true,
+                                            QArrayType, VK_LValue, Loc);
+              auto BinExpr = new (Context) BinaryOperator(
+                  DRE, Size, BO_Comma, QType, Size->getValueKind(), OK_Ordinary,
+                  Loc, /*fpContractable*/ false);
+              auto Lambda = LSI->Lambda;
+
+              // Build the non-static data member.
+              FieldDecl *Field = FieldDecl::Create(Context, Lambda, Loc, Loc,
+                                                   /*Id*/ nullptr, QType, TInfo,
+                                                   BinExpr, /*Mutable*/ false,
+                                                   /*InitStyle*/ ICIS_NoInit);
+              Field->setImplicit(true);
+              Field->setAccess(AS_private);
+              Lambda->addDecl(Field);
+
+              LSI->addCapture(/*Var=*/nullptr, /*IsBlock=*/false,
+                              /*isByref=*/false, /*isNested=*/false, Loc, Loc,
+                              QType, BinExpr);
+            } else {
+              // Immediately mark all referenced vars for CapturedStatements,
+              // they all are captured by reference.
               MarkDeclarationsReferencedInExpr(Size);
             }
+          }
           QTy = Vat->getElementType();
           break;
         }
         case Type::FunctionProto:
         case Type::FunctionNoProto:
           QTy = cast<FunctionType>(Ty)->getReturnType();
           break;
         case Type::Paren:

lib/Sema/TreeTransform.h

if (!FinishedExplicitCaptures && C->isImplicit()) {
       FinishedExplicitCaptures = true;
     }
 
     // Capturing 'this' is trivial.
     if (C->capturesThis()) {
       getSema().CheckCXXThisCapture(C->getLocation(), C->isExplicit());
       continue;
     }
+    // Captured expression will be recaptured during captured variables
+    // rebuilding.
+    if (C->capturesVLABoundExpression())
+      continue;
 
     // Rebuild init-captures, including the implied field declaration.
     if (C->isInitCapture()) {
       
       InitCaptureInfoTy InitExprTypePair = 
           InitCaptureExprsAndTypes[C - E->capture_begin()];
       ExprResult Init = InitExprTypePair.first;
       QualType InitQualType = InitExprTypePair.second;

test/CodeGenCXX/instantiate-typeof-vla.cpp

+// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -emit-pch -o %t
+// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+
+typedef __INTPTR_TYPE__ intptr_t;
+
+// CHECK-DAG:   [[CAP_TYPE1:%.+]] = type { [[INTPTR_T:i.+]], [[INTPTR_T]]*, [[INTPTR_T]]* }
+// CHECK-DAG:   [[CAP_TYPE2:%.+]] = type { [[INTPTR_T]], [[INTPTR_T]]* }
+
+
+
+// CHECK:       define void [[G:@.+]](
+// CHECK:       [[N_ADDR:%.+]] = alloca [[INTPTR_T]]
+// CHECK:       store [[INTPTR_T]] %{{.+}}, [[INTPTR_T]]* [[N_ADDR]]
+// CHECK:       [[N_VAL:%.+]] = load [[INTPTR_T]]* [[N_ADDR]]
+// CHECK:       [[CAP_EXPR_REF:%.+]] = getelementptr inbounds [[CAP_TYPE1]]* [[CAP_ARG:%.+]], i{{.+}} 0, i{{.+}} 0
+// CHECK:       store [[INTPTR_T]] [[N_VAL]], [[INTPTR_T]]* [[CAP_EXPR_REF]]
+// CHECK:       [[CAP_BUFFER_ADDR:%.+]] = getelementptr inbounds [[CAP_TYPE1]]* [[CAP_ARG]], i{{.+}} 0, i{{.+}} 1
+// CHECK:       store [[INTPTR_T]]* %{{.+}}, [[INTPTR_T]]** [[CAP_BUFFER_ADDR]]
+// CHECK:       [[CAP_N_REF:%.+]] = getelementptr inbounds [[CAP_TYPE1]]* [[CAP_ARG:%.+]], i{{.+}} 0, i{{.+}} 2
+// CHECK:       store [[INTPTR_T]]* [[N_ADDR]], [[INTPTR_T]]** [[CAP_N_REF]]
+// CHECK:       call void [[G_LAMBDA:@.+]]([[CAP_TYPE1]]* [[CAP_ARG]])
+// CHECK:       ret void
+void g(intptr_t n) {
+  intptr_t buffer[n];
+  [&buffer, &n]() {
+    __typeof(buffer) x;
+  }();
+}
+
+// CHECK: void [[G_LAMBDA]]([[CAP_TYPE1]]*
+// CHECK: [[THIS:%.+]] = load [[CAP_TYPE1]]**
+// CHECK: [[N_ADDR:%.+]] = getelementptr inbounds [[CAP_TYPE1]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
+// CHECK: [[N:%.+]] = load [[INTPTR_T]]* [[N_ADDR]]
+// CHECK: [[BUFFER_ADDR:%.+]] = getelementptr inbounds [[CAP_TYPE1]]* [[THIS]], i{{.+}} 0, i{{.+}} 1
+// CHECK: [[BUFFER:%.+]] = load [[INTPTR_T]]** [[BUFFER_ADDR]]
+// CHECK: call i{{.+}}* @llvm.stacksave()
+// CHECK: alloca [[INTPTR_T]], [[INTPTR_T]] [[N]]
+// CHECK: call void @llvm.stackrestore(
+// CHECK: ret void
+
+template <typename T> void f(T n, T m) {
+  intptr_t buffer[n + m];
+  [&buffer]() {
+    __typeof(buffer) x;
+  }();
+}
+
+// CHECK-LABEL: @main
+int main() {
+// CHECK:       call void [[G]]([[INTPTR_T]] 1)
+  g((intptr_t)1);
+// CHECK:       call void [[F_INT:@.+]]([[INTPTR_T]] 1, [[INTPTR_T]] 2)
+  f((intptr_t)1, (intptr_t)2);
+// CHECK:       ret i32 0
+  return 0;
+}
+
+// CHECK: void [[F_INT]]([[INTPTR_T]]
+// CHECK: [[SIZE:%.+]] = add
+// CHECK: call i{{.+}}* @llvm.stacksave()
+// CHECK: [[BUFFER_ADDR:%.+]] = alloca [[INTPTR_T]], [[INTPTR_T]] [[SIZE]]
+// CHECK: [[CAP_SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]]* [[CAP_ARG:%.+]], i{{.+}} 0, i{{.+}} 0
+// CHECK: store [[INTPTR_T]] [[SIZE]], [[INTPTR_T]]* [[CAP_SIZE_REF]]
+// CHECK: [[CAP_BUFFER_ADDR_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]]* [[CAP_ARG]], i{{.+}} 0, i{{.+}} 1
+// CHECK: store [[INTPTR_T]]* [[BUFFER_ADDR]], [[INTPTR_T]]** [[CAP_BUFFER_ADDR_REF]]
+// CHECK: call void [[F_INT_LAMBDA:@.+]]([[CAP_TYPE2]]* [[CAP_ARG]])
+// CHECK: call void @llvm.stackrestore(
+// CHECK: ret void
+
+// CHECK: void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
+// CHECK: [[THIS:%.+]] = load [[CAP_TYPE2]]**
+// CHECK: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
+// CHECK: [[SIZE:%.+]] = load [[INTPTR_T]]* [[SIZE_REF]]
+// CHECK: call i{{.+}}* @llvm.stacksave()
+// CHECK: alloca [[INTPTR_T]], [[INTPTR_T]] [[SIZE]]
+// CHECK: call void @llvm.stackrestore(
+// CHECK: ret void
+#endif

test/SemaTemplate/instantiate-typeof.cpp

 // 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);
 }

tools/libclang/IndexBody.cpp

for (DeclGroupRef::iterator I = DG.begin(), E = DG.end(); I != E; ++I) {
       if (!IndexCtx.isFunctionLocalDecl(D))
         IndexCtx.indexTopLevelDecl(D);
     }
 
     return true;
   }
 
   bool TraverseLambdaCapture(LambdaExpr *LE, const LambdaCapture *C) {
-    if (C->capturesThis())
+    if (C->capturesThis() || C->capturesVLABoundExpression())
       return true;
 
     if (C->capturesVariable() && IndexCtx.shouldIndexFunctionLocalSymbols())
       IndexCtx.handleReference(C->getCapturedVar(), C->getLocation(), Parent,
                                ParentDC);
 
     // FIXME: Lambda init-captures.
     return true;