Defer creating fields for captures until we finish building the

capturing expression or statement.

No functionality change yet. The intent is that we will also delay
building the initialization expression until the enclosing context, so
that:
a) we build the initialization expression in the right context, and
b) we can elide captures that are not odr-used, as suggested by P0588R1.

This also consolidates some duplicated code building capture fields into
a single place.

llvm-svn: 361893

7 files changed, 103 insertions, 150 deletions

diff --git a/clang/lib/Sema/ScopeInfo.cpp b/clang/lib/Sema/ScopeInfo.cpp
index 1003d2639c0..dd309a28118 100644
--- a/clang/lib/Sema/ScopeInfo.cpp
+++ b/clang/lib/Sema/ScopeInfo.cpp
@@ -113,17 +113,9 @@ FunctionScopeInfo::WeakObjectProfileTy::getBaseInfo(const Expr *E) {
 }
 
 bool CapturingScopeInfo::isVLATypeCaptured(const VariableArrayType *VAT) const {
-  RecordDecl *RD = nullptr;
-  if (auto *LSI = dyn_cast<LambdaScopeInfo>(this))
-    RD = LSI->Lambda;
-  else if (auto CRSI = dyn_cast<CapturedRegionScopeInfo>(this))
-    RD = CRSI->TheRecordDecl;
-
-  if (RD)
-    for (auto *FD : RD->fields()) {
-      if (FD->hasCapturedVLAType() && FD->getCapturedVLAType() == VAT)
-        return true;
-    }
+  for (auto &Cap : Captures)
+    if (Cap.isVLATypeCapture() && Cap.getCapturedVLAType() == VAT)
+      return true;
   return false;
 }
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 188e801b4c4..fbc410f014d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12942,11 +12942,11 @@ static void RebuildLambdaScopeInfo(CXXMethodDecl *CallOperator,
           CaptureType, /*Expr*/ nullptr, /*Invalid*/false);
 
     } else if (C.capturesThis()) {
-      LSI->addThisCapture(/*Nested*/ false, C.getLocation(),
-                              /*Expr*/ nullptr,
-                              C.getCaptureKind() == LCK_StarThis);
+      LSI->addThisCapture(/*Nested*/ false, C.getLocation(), I->getType(),
+                          /*Expr*/ nullptr, C.getCaptureKind() == LCK_StarThis);
     } else {
-      LSI->addVLATypeCapture(C.getLocation(), I->getType());
+      LSI->addVLATypeCapture(C.getLocation(), I->getCapturedVLAType(),
+                             I->getType());
     }
     ++I;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 95be7af8b68..8eccb4b0c5d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4050,32 +4050,11 @@ static void captureVariablyModifiedType(ASTContext &Context, QualType T,
 
       // Unknown size indication requires no size computation.
       // Otherwise, evaluate and record it.
-      if (auto Size = VAT->getSizeExpr()) {
-        if (!CSI->isVLATypeCaptured(VAT)) {
-          RecordDecl *CapRecord = nullptr;
-          if (auto LSI = dyn_cast<LambdaScopeInfo>(CSI)) {
-            CapRecord = LSI->Lambda;
-          } else if (auto CRSI = dyn_cast<CapturedRegionScopeInfo>(CSI)) {
-            CapRecord = CRSI->TheRecordDecl;
-          }
-          if (CapRecord) {
-            auto ExprLoc = Size->getExprLoc();
-            auto SizeType = Context.getSizeType();
-            // Build the non-static data member.
-            auto Field =
-                FieldDecl::Create(Context, CapRecord, ExprLoc, ExprLoc,
-                                  /*Id*/ nullptr, SizeType, /*TInfo*/ nullptr,
-                                  /*BW*/ nullptr, /*Mutable*/ false,
-                                  /*InitStyle*/ ICIS_NoInit);
-            Field->setImplicit(true);
-            Field->setAccess(AS_private);
-            Field->setCapturedVLAType(VAT);
-            CapRecord->addDecl(Field);
-
-            CSI->addVLATypeCapture(ExprLoc, SizeType);
-          }
-        }
-      }
+      auto Size = VAT->getSizeExpr();
+      if (Size && !CSI->isVLATypeCaptured(VAT) &&
+          (isa<CapturedRegionScopeInfo>(CSI) || isa<LambdaScopeInfo>(CSI)))
+        CSI->addVLATypeCapture(Size->getExprLoc(), VAT, Context.getSizeType());
+
       T = VAT->getElementType();
       break;
     }
@@ -15367,18 +15346,6 @@ static bool captureInCapturedRegion(CapturedRegionScopeInfo *RSI,
     // The current implementation assumes that all variables are captured
     // by references. Since there is no capture by copy, no expression
     // evaluation will be needed.
-    RecordDecl *RD = RSI->TheRecordDecl;
-
-    FieldDecl *Field
-      = FieldDecl::Create(S.Context, RD, Loc, Loc, nullptr, CaptureType,
-                          S.Context.getTrivialTypeSourceInfo(CaptureType, Loc),
-                          nullptr, false, ICIS_NoInit);
-    Field->setImplicit(true);
-    Field->setAccess(AS_private);
-    RD->addDecl(Field);
-    if (S.getLangOpts().OpenMP && RSI->CapRegionKind == CR_OpenMP)
-      S.setOpenMPCaptureKind(Field, Var, RSI->OpenMPLevel);
-
     CopyExpr = new (S.Context) DeclRefExpr(
         S.Context, Var, RefersToCapturedVariable, DeclRefType, VK_LValue, Loc);
     Var->setReferenced(true);
@@ -15394,39 +15361,6 @@ static bool captureInCapturedRegion(CapturedRegionScopeInfo *RSI,
   return !Invalid;
 }
 
-/// Create a field within the lambda class for the variable
-/// being captured.
-static void addAsFieldToClosureType(Sema &S, LambdaScopeInfo *LSI,
-                                    QualType FieldType, QualType DeclRefType,
-                                    SourceLocation Loc,
-                                    bool RefersToCapturedVariable) {
-  CXXRecordDecl *Lambda = LSI->Lambda;
-
-  // Build the non-static data member.
-  FieldDecl *Field
-    = FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
-                        S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
-                        nullptr, false, ICIS_NoInit);
-  // If the variable being captured has an invalid type, mark the lambda class
-  // as invalid as well.
-  if (!FieldType->isDependentType()) {
-    if (S.RequireCompleteType(Loc, FieldType, diag::err_field_incomplete)) {
-      Lambda->setInvalidDecl();
-      Field->setInvalidDecl();
-    } else {
-      NamedDecl *Def;
-      FieldType->isIncompleteType(&Def);
-      if (Def && Def->isInvalidDecl()) {
-        Lambda->setInvalidDecl();
-        Field->setInvalidDecl();
-      }
-    }
-  }
-  Field->setImplicit(true);
-  Field->setAccess(AS_private);
-  Lambda->addDecl(Field);
-}
-
 /// Capture the given variable in the lambda.
 static bool captureInLambda(LambdaScopeInfo *LSI,
                             VarDecl *Var,
@@ -15504,11 +15438,6 @@ static bool captureInLambda(LambdaScopeInfo *LSI,
     }
   }
 
-  // Capture this variable in the lambda.
-  if (BuildAndDiagnose && !Invalid)
-    addAsFieldToClosureType(S, LSI, CaptureType, DeclRefType, Loc,
-                            RefersToCapturedVariable);
-
   // Compute the type of a reference to this captured variable.
   if (ByRef)
     DeclRefType = CaptureType.getNonReferenceType();
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index ef27fc2d719..00b158debc5 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1135,39 +1135,15 @@ Sema::CXXThisScopeRAII::~CXXThisScopeRAII() {
 }
 
 static Expr *captureThis(Sema &S, ASTContext &Context, RecordDecl *RD,
-                         QualType ThisTy, SourceLocation Loc,
-                         const bool ByCopy) {
-
-  QualType AdjustedThisTy = ThisTy;
-  // The type of the corresponding data member (not a 'this' pointer if 'by
-  // copy').
-  QualType CaptureThisFieldTy = ThisTy;
-  if (ByCopy) {
-    // If we are capturing the object referred to by '*this' by copy, ignore any
-    // cv qualifiers inherited from the type of the member function for the type
-    // of the closure-type's corresponding data member and any use of 'this'.
-    CaptureThisFieldTy = ThisTy->getPointeeType();
-    CaptureThisFieldTy.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-    AdjustedThisTy = Context.getPointerType(CaptureThisFieldTy);
-  }
-
-  FieldDecl *Field = FieldDecl::Create(
-      Context, RD, Loc, Loc, nullptr, CaptureThisFieldTy,
-      Context.getTrivialTypeSourceInfo(CaptureThisFieldTy, Loc), nullptr, false,
-      ICIS_NoInit);
-
-  Field->setImplicit(true);
-  Field->setAccess(AS_private);
-  RD->addDecl(Field);
-  Expr *This =
-      new (Context) CXXThisExpr(Loc, ThisTy, /*isImplicit*/ true);
+                         QualType ThisTy, QualType CaptureType,
+                         SourceLocation Loc, const bool ByCopy) {
+  Expr *This = new (Context) CXXThisExpr(Loc, ThisTy, /*isImplicit*/ true);
   if (ByCopy) {
-    Expr *StarThis =  S.CreateBuiltinUnaryOp(Loc,
-                                      UO_Deref,
-                                      This).get();
-    InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture(
-      nullptr, CaptureThisFieldTy, Loc);
-    InitializationKind InitKind = InitializationKind::CreateDirect(Loc, Loc, Loc);
+    Expr *StarThis = S.CreateBuiltinUnaryOp(Loc, UO_Deref, This).get();
+    InitializedEntity Entity =
+        InitializedEntity::InitializeLambdaCapture(nullptr, CaptureType, Loc);
+    InitializationKind InitKind =
+        InitializationKind::CreateDirect(Loc, Loc, Loc);
     InitializationSequence Init(S, Entity, InitKind, StarThis);
     ExprResult ER = Init.Perform(S, Entity, InitKind, StarThis);
     if (ER.isInvalid()) return nullptr;
@@ -1273,21 +1249,32 @@ bool Sema::CheckCXXThisCapture(SourceLocation Loc, const bool Explicit,
     CapturingScopeInfo *CSI = cast<CapturingScopeInfo>(FunctionScopes[idx]);
     Expr *ThisExpr = nullptr;
 
+    // The type of the corresponding data member (not a 'this' pointer if 'by
+    // copy').
+    QualType CaptureType = ThisTy;
+    if (ByCopy) {
+      // If we are capturing the object referred to by '*this' by copy, ignore
+      // any cv qualifiers inherited from the type of the member function for
+      // the type of the closure-type's corresponding data member and any use
+      // of 'this'.
+      CaptureType = ThisTy->getPointeeType();
+      CaptureType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
+    }
+
     if (LambdaScopeInfo *LSI = dyn_cast<LambdaScopeInfo>(CSI)) {
       // For lambda expressions, build a field and an initializing expression,
       // and capture the *enclosing object* by copy only if this is the first
       // iteration.
-      ThisExpr = captureThis(*this, Context, LSI->Lambda, ThisTy, Loc,
-                             ByCopy && idx == MaxFunctionScopesIndex);
+      ThisExpr = captureThis(*this, Context, LSI->Lambda, ThisTy, CaptureType,
+                             Loc, ByCopy && idx == MaxFunctionScopesIndex);
 
     } else if (CapturedRegionScopeInfo *RSI
         = dyn_cast<CapturedRegionScopeInfo>(FunctionScopes[idx]))
-      ThisExpr =
-          captureThis(*this, Context, RSI->TheRecordDecl, ThisTy, Loc,
-                      false/*ByCopy*/);
+      ThisExpr = captureThis(*this, Context, RSI->TheRecordDecl, ThisTy,
+                             CaptureType, Loc, false /*ByCopy*/);
 
     bool isNested = NumCapturingClosures > 1;
-    CSI->addThisCapture(isNested, Loc, ThisExpr, ByCopy);
+    CSI->addThisCapture(isNested, Loc, CaptureType, ThisExpr, ByCopy);
   }
   return false;
 }
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index f6c9dee2a08..6d487cc8325 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -843,19 +843,10 @@ VarDecl *Sema::createLambdaInitCaptureVarDecl(SourceLocation Loc,
   return NewVD;
 }
 
-FieldDecl *Sema::buildInitCaptureField(LambdaScopeInfo *LSI, VarDecl *Var) {
-  FieldDecl *Field = FieldDecl::Create(
-      Context, LSI->Lambda, Var->getLocation(), Var->getLocation(),
-      nullptr, Var->getType(), Var->getTypeSourceInfo(), nullptr, false,
-      ICIS_NoInit);
-  Field->setImplicit(true);
-  Field->setAccess(AS_private);
-  LSI->Lambda->addDecl(Field);
-
+void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var) {
   LSI->addCapture(Var, /*isBlock*/false, Var->getType()->isReferenceType(),
                   /*isNested*/false, Var->getLocation(), SourceLocation(),
                   Var->getType(), Var->getInit(), /*Invalid*/false);
-  return Field;
 }
 
 void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
@@ -1182,7 +1173,7 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
     }
 
     if (C->Init.isUsable()) {
-      buildInitCaptureField(LSI, Var);
+      addInitCapture(LSI, Var);
     } else {
       TryCaptureKind Kind = C->Kind == LCK_ByRef ? TryCapture_ExplicitByRef :
                                                    TryCapture_ExplicitByVal;
@@ -1539,6 +1530,54 @@ bool Sema::DiagnoseUnusedLambdaCapture(SourceRange CaptureRange,
   return true;
 }
 
+/// Create a field within the lambda class or captured statement record for the
+/// given capture.
+FieldDecl *Sema::BuildCaptureField(RecordDecl *RD,
+                                   const sema::Capture &Capture) {
+  SourceLocation Loc = Capture.getLocation();
+  QualType FieldType = Capture.getCaptureType();
+
+  TypeSourceInfo *TSI = nullptr;
+  if (Capture.isVariableCapture()) {
+    auto *Var = Capture.getVariable();
+    if (Var->isInitCapture())
+      TSI = Capture.getVariable()->getTypeSourceInfo();
+  }
+
+  // FIXME: Should we really be doing this? A null TypeSourceInfo seems more
+  // appropriate, at least for an implicit capture.
+  if (!TSI)
+    TSI = Context.getTrivialTypeSourceInfo(FieldType, Loc);
+
+  // Build the non-static data member.
+  FieldDecl *Field =
+      FieldDecl::Create(Context, RD, Loc, Loc, nullptr, FieldType, TSI, nullptr,
+                        false, ICIS_NoInit);
+  // If the variable being captured has an invalid type, mark the class as
+  // invalid as well.
+  if (!FieldType->isDependentType()) {
+    if (RequireCompleteType(Loc, FieldType, diag::err_field_incomplete)) {
+      RD->setInvalidDecl();
+      Field->setInvalidDecl();
+    } else {
+      NamedDecl *Def;
+      FieldType->isIncompleteType(&Def);
+      if (Def && Def->isInvalidDecl()) {
+        RD->setInvalidDecl();
+        Field->setInvalidDecl();
+      }
+    }
+  }
+  Field->setImplicit(true);
+  Field->setAccess(AS_private);
+  RD->addDecl(Field);
+
+  if (Capture.isVLATypeCapture())
+    Field->setCapturedVLAType(Capture.getCapturedVLAType());
+
+  return Field;
+}
+
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
                                  LambdaScopeInfo *LSI) {
   // Collect information from the lambda scope.
@@ -1576,14 +1615,12 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
 
     PopExpressionEvaluationContext();
 
-    // Translate captures.
-    auto CurField = Class->field_begin();
     // True if the current capture has a used capture or default before it.
     bool CurHasPreviousCapture = CaptureDefault != LCD_None;
     SourceLocation PrevCaptureLoc = CurHasPreviousCapture ?
         CaptureDefaultLoc : IntroducerRange.getBegin();
 
-    for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I, ++CurField) {
+    for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I) {
       const Capture &From = LSI->Captures[I];
 
       if (From.isInvalid())
@@ -1626,6 +1663,9 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
         PrevCaptureLoc = CaptureRange.getEnd();
       }
 
+      // Add a FieldDecl for the capture.
+      FieldDecl *Field = BuildCaptureField(Class, From);
+
       // Handle 'this' capture.
       if (From.isThisCapture()) {
         // Capturing 'this' implicitly with a default of '[=]' is deprecated,
@@ -1659,7 +1699,7 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
       Expr *Init = From.getInitExpr();
       if (!Init) {
         auto InitResult = performLambdaVarCaptureInitialization(
-            *this, From, *CurField, CaptureDefaultLoc, IsImplicit);
+            *this, From, Field, CaptureDefaultLoc, IsImplicit);
         if (InitResult.isInvalid())
           return ExprError();
         Init = InitResult.get();
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 51a72c618b7..357e257abe0 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -4224,13 +4224,16 @@ Sema::CreateCapturedStmtRecordDecl(CapturedDecl *&CD, SourceLocation Loc,
 }
 
 static void
-buildCapturedStmtCaptureList(SmallVectorImpl<CapturedStmt::Capture> &Captures,
-                             SmallVectorImpl<Expr *> &CaptureInits,
-                             ArrayRef<sema::Capture> Candidates) {
-  for (const sema::Capture &Cap : Candidates) {
+buildCapturedStmtCaptureList(Sema &S, CapturedRegionScopeInfo *RSI,
+                             SmallVectorImpl<CapturedStmt::Capture> &Captures,
+                             SmallVectorImpl<Expr *> &CaptureInits) {
+  for (const sema::Capture &Cap : RSI->Captures) {
     if (Cap.isInvalid())
       continue;
 
+    // Create a field for this capture.
+    FieldDecl *Field = S.BuildCaptureField(RSI->TheRecordDecl, Cap);
+
     if (Cap.isThisCapture()) {
       Captures.push_back(CapturedStmt::Capture(Cap.getLocation(),
                                                CapturedStmt::VCK_This));
@@ -4243,6 +4246,8 @@ buildCapturedStmtCaptureList(SmallVectorImpl<CapturedStmt::Capture> &Captures,
       continue;
     }
 
+    if (S.getLangOpts().OpenMP && RSI->CapRegionKind == CR_OpenMP)
+      S.setOpenMPCaptureKind(Field, Cap.getVariable(), RSI->OpenMPLevel);
     Captures.push_back(CapturedStmt::Capture(Cap.getLocation(),
                                              Cap.isReferenceCapture()
                                                  ? CapturedStmt::VCK_ByRef
@@ -4360,7 +4365,7 @@ StmtResult Sema::ActOnCapturedRegionEnd(Stmt *S) {
 
   SmallVector<CapturedStmt::Capture, 4> Captures;
   SmallVector<Expr *, 4> CaptureInits;
-  buildCapturedStmtCaptureList(Captures, CaptureInits, RSI->Captures);
+  buildCapturedStmtCaptureList(*this, RSI, Captures, CaptureInits);
 
   CapturedDecl *CD = RSI->TheCapturedDecl;
   RecordDecl *RD = RSI->TheRecordDecl;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index b5114eeef30..6620885f239 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -11382,7 +11382,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
           break;
         }
         NewVDs.push_back(NewVD);
-        getSema().buildInitCaptureField(LSI, NewVD);
+        getSema().addInitCapture(LSI, NewVD);
       }
 
       if (Invalid)