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

[Alexey Bataev]

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

================
Comment at: include/clang/AST/LambdaCapture.h:67
@@ -66,3 +66,3 @@
   /// \brief Determine the kind of capture.
   LambdaCaptureKind getCaptureKind() const;
 
----------------
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.

================
Comment at: lib/AST/Decl.cpp:3303-3305
@@ -3292,4 +3302,5 @@
 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 type already set");
   InitializerOrBitWidth.setPointer(Width);
----------------
rsmith wrote:
> 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.)
Yers, you're right. I'll remove assert. The method is used by some tools and I think we should keep it.

================
Comment at: lib/AST/Decl.cpp:3317
@@ +3316,3 @@
+  return getDeclContext()->isRecord() && getParent()->isLambda() &&
+         getInClassInitStyle() == ICIS_NoInit &&
+         InitializerOrBitWidth.getPointer();
----------------
rsmith wrote:
> Is this check necessary? You can't have an in-class initializer in a lambda.
No, it is not neccessary, just double checked some conditions. I think it can be removed.

================
Comment at: lib/AST/Decl.cpp:3549-3551
@@ +3548,5 @@
+bool RecordDecl::isLambda() const {
+  if (auto RD = dyn_cast<CXXRecordDecl>(this)) {
+    return RD->isLambda();
+  }
+  return false;
----------------
rsmith wrote:
> No braces here, please.
Removed

================
Comment at: lib/AST/StmtPrinter.cpp:1711-1712
@@ -1710,2 +1710,4 @@
        ++C) {
+    if (C->capturesVLAType())
+      continue;
     if (NeedComma)
----------------
rsmith wrote:
> Is this possible? I would have expected VLA captures to always be implicit.
You're right, removed.

================
Comment at: lib/AST/StmtProfile.cpp:1020-1021
@@ -1019,2 +1019,4 @@
        C != CEnd; ++C) {
+    if (C->capturesVLAType())
+      continue;
     ID.AddInteger(C->getCaptureKind());
----------------
rsmith wrote:
> Likewise.
Removed too.

================
Comment at: lib/Sema/SemaExpr.cpp:11668
@@ -11667,2 +11667,3 @@
 
   // Prohibit variably-modified types; they're difficult to deal with.
+  if (Var->getType()->isVariablyModifiedType() && IsBlock) {
----------------
rsmith wrote:
> Update this comment: "Prohibit variably-modified types in blocks; ..." maybe?
Done.

================
Comment at: lib/Sema/SemaExpr.cpp:12075-12082
@@ -12077,2 +12074,10 @@
 
+static bool isVLATypeIsCaptured(LambdaScopeInfo *LSI,
+                                const VariableArrayType *VAT) {
+  for (auto *FD : LSI->Lambda->fields()) {
+    if (FD->hasCapturedVLAType() && FD->getCapturedVLAType() == VAT)
+      return true;
+  }
+  return false;
+}
 
----------------
rsmith wrote:
> Please make this a member of `CapturingScopeInfo`. Also, drop one of the `Is`s from its name.
Ok

http://reviews.llvm.org/D4368