PR42104: Support instantiations of lambdas that implicitly capture

packs.

Two changes:
 * Track odr-use via FunctionParmPackExprs to properly handle dependent
   odr-uses of packs in generic lambdas.
 * Do not instantiate implicit captures; instead, regenerate them by
   instantiating the body of the lambda. This is necessary to
   distinguish between cases where only one element of a pack is
   captured and cases where the entire pack is captured.

This reinstates r362358 (reverted in r362375) with a fix for an
uninitialized variable use in UpdateMarkingForLValueToRValue.

llvm-svn: 362531

Author: zygoloid

clang/include/clang/Sema/ScopeInfo.h

@@ -15,6 +15,7 @@
 #define LLVM_CLANG_SEMA_SCOPEINFO_H
 
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CapturedStmt.h"
 #include "clang/Basic/LLVM.h"
@@ -913,7 +914,8 @@ class LambdaScopeInfo final : public CapturingScopeInfo {
   ///   };
   /// }
   void addPotentialCapture(Expr *VarExpr) {
-    assert(isa<DeclRefExpr>(VarExpr) || isa<MemberExpr>(VarExpr));
+    assert(isa<DeclRefExpr>(VarExpr) || isa<MemberExpr>(VarExpr) ||
+           isa<FunctionParmPackExpr>(VarExpr));
     PotentiallyCapturingExprs.push_back(VarExpr);
   }
 
@@ -965,13 +967,15 @@ class LambdaScopeInfo final : public CapturingScopeInfo {
   ///  building such a node. So we need a rule that anyone can implement and get
   ///  exactly the same result".
   void markVariableExprAsNonODRUsed(Expr *CapturingVarExpr) {
-    assert(isa<DeclRefExpr>(CapturingVarExpr)
-        || isa<MemberExpr>(CapturingVarExpr));
+    assert(isa<DeclRefExpr>(CapturingVarExpr) ||
+           isa<MemberExpr>(CapturingVarExpr) ||
+           isa<FunctionParmPackExpr>(CapturingVarExpr));
     NonODRUsedCapturingExprs.insert(CapturingVarExpr);
   }
   bool isVariableExprMarkedAsNonODRUsed(Expr *CapturingVarExpr) const {
-    assert(isa<DeclRefExpr>(CapturingVarExpr)
-      || isa<MemberExpr>(CapturingVarExpr));
+    assert(isa<DeclRefExpr>(CapturingVarExpr) ||
+           isa<MemberExpr>(CapturingVarExpr) ||
+           isa<FunctionParmPackExpr>(CapturingVarExpr));
     return NonODRUsedCapturingExprs.count(CapturingVarExpr);
   }
   void removePotentialCapture(Expr *E) {
@@ -993,9 +997,8 @@ class LambdaScopeInfo final : public CapturingScopeInfo {
                                   PotentialThisCaptureLocation.isValid();
   }
 
-  // When passed the index, returns the VarDecl and Expr associated
-  // with the index.
-  void getPotentialVariableCapture(unsigned Idx, VarDecl *&VD, Expr *&E) const;
+  void visitPotentialCaptures(
+      llvm::function_ref<void(VarDecl *, Expr *)> Callback) const;
 };
 
 FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy()

clang/include/clang/Sema/Sema.h

@@ -4181,6 +4181,7 @@ class Sema {
   void MarkVariableReferenced(SourceLocation Loc, VarDecl *Var);
   void MarkDeclRefReferenced(DeclRefExpr *E, const Expr *Base = nullptr);
   void MarkMemberReferenced(MemberExpr *E);
+  void MarkFunctionParmPackReferenced(FunctionParmPackExpr *E);
   void MarkCaptureUsedInEnclosingContext(VarDecl *Capture, SourceLocation Loc,
                                          unsigned CapturingScopeIndex);
 

clang/lib/Sema/ScopeInfo.cpp

@@ -229,20 +229,20 @@ bool CapturingScopeInfo::isVLATypeCaptured(const VariableArrayType *VAT) const {
   return false;
 }
 
-void LambdaScopeInfo::getPotentialVariableCapture(unsigned Idx, VarDecl *&VD,
-                                                  Expr *&E) const {
-  assert(Idx < getNumPotentialVariableCaptures() &&
-         "Index of potential capture must be within 0 to less than the "
-         "number of captures!");
-  E = PotentiallyCapturingExprs[Idx];
-  if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
-    VD = dyn_cast<VarDecl>(DRE->getFoundDecl());
-  else if (MemberExpr *ME = dyn_cast<MemberExpr>(E))
-    VD = dyn_cast<VarDecl>(ME->getMemberDecl());
-  else
-    llvm_unreachable("Only DeclRefExprs or MemberExprs should be added for "
-    "potential captures");
-  assert(VD);
+void LambdaScopeInfo::visitPotentialCaptures(
+    llvm::function_ref<void(VarDecl *, Expr *)> Callback) const {
+  for (Expr *E : PotentiallyCapturingExprs) {
+    if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+      Callback(cast<VarDecl>(DRE->getFoundDecl()), E);
+    } else if (auto *ME = dyn_cast<MemberExpr>(E)) {
+      Callback(cast<VarDecl>(ME->getMemberDecl()), E);
+    } else if (auto *FP = dyn_cast<FunctionParmPackExpr>(E)) {
+      for (VarDecl *VD : *FP)
+        Callback(VD, E);
+    } else {
+      llvm_unreachable("unexpected expression in potential captures list");
+    }
+  }
 }
 
 FunctionScopeInfo::~FunctionScopeInfo() { }

clang/lib/Sema/SemaExpr.cpp

@@ -14610,7 +14610,9 @@ namespace {
     // context so never needs to be transformed.
     // FIXME: Ideally we wouldn't transform the closure type either, and would
     // just recreate the capture expressions and lambda expression.
-    StmtResult TransformLambdaBody(Stmt *Body) { return Body; }
+    StmtResult TransformLambdaBody(LambdaExpr *E, Stmt *Body) {
+      return SkipLambdaBody(E, Body);
+    }
   };
 }
 
@@ -15054,7 +15056,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
 ///    *FunctionScopeIndexToStopAt on the FunctionScopeInfo stack.
 static void
 MarkVarDeclODRUsed(VarDecl *Var, SourceLocation Loc, Sema &SemaRef,
-                   const unsigned *const FunctionScopeIndexToStopAt) {
+                   const unsigned *const FunctionScopeIndexToStopAt = nullptr) {
   // Keep track of used but undefined variables.
   // FIXME: We shouldn't suppress this warning for static data members.
   if (Var->hasDefinition(SemaRef.Context) == VarDecl::DeclarationOnly &&
@@ -15735,14 +15737,21 @@ void Sema::UpdateMarkingForLValueToRValue(Expr *E) {
   // variable.
   if (LambdaScopeInfo *LSI = getCurLambda()) {
     Expr *SansParensExpr = E->IgnoreParens();
-    VarDecl *Var = nullptr;
+    VarDecl *Var;
+    ArrayRef<VarDecl *> Vars(&Var, &Var + 1);
     if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(SansParensExpr))
       Var = dyn_cast<VarDecl>(DRE->getFoundDecl());
     else if (MemberExpr *ME = dyn_cast<MemberExpr>(SansParensExpr))
       Var = dyn_cast<VarDecl>(ME->getMemberDecl());
+    else if (auto *FPPE = dyn_cast<FunctionParmPackExpr>(SansParensExpr))
+      Vars = llvm::makeArrayRef(FPPE->begin(), FPPE->end());
+    else
+      Vars = None;
 
-    if (Var && IsVariableNonDependentAndAConstantExpression(Var, Context))
-      LSI->markVariableExprAsNonODRUsed(SansParensExpr);
+    for (VarDecl *VD : Vars) {
+      if (VD && IsVariableNonDependentAndAConstantExpression(VD, Context))
+        LSI->markVariableExprAsNonODRUsed(SansParensExpr);
+    }
   }
 }
 
@@ -15767,20 +15776,18 @@ void Sema::CleanupVarDeclMarking() {
   std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
 
   for (Expr *E : LocalMaybeODRUseExprs) {
-    VarDecl *Var;
-    SourceLocation Loc;
-    if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
-      Var = cast<VarDecl>(DRE->getDecl());
-      Loc = DRE->getLocation();
-    } else if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
-      Var = cast<VarDecl>(ME->getMemberDecl());
-      Loc = ME->getMemberLoc();
+    if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+      MarkVarDeclODRUsed(cast<VarDecl>(DRE->getDecl()),
+                         DRE->getLocation(), *this);
+    } else if (auto *ME = dyn_cast<MemberExpr>(E)) {
+      MarkVarDeclODRUsed(cast<VarDecl>(ME->getMemberDecl()), ME->getMemberLoc(),
+                         *this);
+    } else if (auto *FP = dyn_cast<FunctionParmPackExpr>(E)) {
+      for (VarDecl *VD : *FP)
+        MarkVarDeclODRUsed(VD, FP->getParameterPackLocation(), *this);
     } else {
       llvm_unreachable("Unexpected expression");
     }
-
-    MarkVarDeclODRUsed(Var, Loc, *this,
-                       /*MaxFunctionScopeIndex Pointer*/ nullptr);
   }
 
   assert(MaybeODRUseExprs.empty() &&
@@ -15789,7 +15796,8 @@ void Sema::CleanupVarDeclMarking() {
 
 static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc,
                                     VarDecl *Var, Expr *E) {
-  assert((!E || isa<DeclRefExpr>(E) || isa<MemberExpr>(E)) &&
+  assert((!E || isa<DeclRefExpr>(E) || isa<MemberExpr>(E) ||
+          isa<FunctionParmPackExpr>(E)) &&
          "Invalid Expr argument to DoMarkVarDeclReferenced");
   Var->setReferenced();
 
@@ -16022,6 +16030,12 @@ void Sema::MarkMemberReferenced(MemberExpr *E) {
   MarkExprReferenced(*this, Loc, E->getMemberDecl(), E, MightBeOdrUse);
 }
 
+/// Perform reference-marking and odr-use handling for a FunctionParmPackExpr.
+void Sema::MarkFunctionParmPackReferenced(FunctionParmPackExpr *E) {
+  for (VarDecl *VD : *E)
+    MarkExprReferenced(*this, E->getParameterPackLocation(), VD, E, true);
+}
+
 /// Perform marking for a reference to an arbitrary declaration.  It
 /// marks the declaration referenced, and performs odr-use checking for
 /// functions and variables. This method should not be used when building a

clang/lib/Sema/SemaExprCXX.cpp

@@ -7427,12 +7427,7 @@ static void CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures(
   // All the potentially captureable variables in the current nested
   // lambda (within a generic outer lambda), must be captured by an
   // outer lambda that is enclosed within a non-dependent context.
-  const unsigned NumPotentialCaptures =
-      CurrentLSI->getNumPotentialVariableCaptures();
-  for (unsigned I = 0; I != NumPotentialCaptures; ++I) {
-    Expr *VarExpr = nullptr;
-    VarDecl *Var = nullptr;
-    CurrentLSI->getPotentialVariableCapture(I, Var, VarExpr);
+  CurrentLSI->visitPotentialCaptures([&] (VarDecl *Var, Expr *VarExpr) {
     // If the variable is clearly identified as non-odr-used and the full
     // expression is not instantiation dependent, only then do we not
     // need to check enclosing lambda's for speculative captures.
@@ -7446,7 +7441,7 @@ static void CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures(
     // }
     if (CurrentLSI->isVariableExprMarkedAsNonODRUsed(VarExpr) &&
         !IsFullExprInstantiationDependent)
-      continue;
+      return;
 
     // If we have a capture-capable lambda for the variable, go ahead and
     // capture the variable in that lambda (and all its enclosing lambdas).
@@ -7478,7 +7473,7 @@ static void CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures(
                           DeclRefType, nullptr);
       }
     }
-  }
+  });
 
   // Check if 'this' needs to be captured.
   if (CurrentLSI->hasPotentialThisCapture()) {

clang/lib/Sema/SemaTemplateInstantiate.cpp

@@ -1368,9 +1368,11 @@ TemplateInstantiator::TransformFunctionParmPackExpr(FunctionParmPackExpr *E) {
     Vars.push_back(D);
   }
 
-  return FunctionParmPackExpr::Create(getSema().Context, T,
-                                      E->getParameterPack(),
-                                      E->getParameterPackLocation(), Vars);
+  auto *PackExpr =
+      FunctionParmPackExpr::Create(getSema().Context, T, E->getParameterPack(),
+                                   E->getParameterPackLocation(), Vars);
+  getSema().MarkFunctionParmPackReferenced(PackExpr);
+  return PackExpr;
 }
 
 ExprResult
@@ -1389,8 +1391,10 @@ TemplateInstantiator::TransformFunctionParmPackRefExpr(DeclRefExpr *E,
       QualType T = TransformType(E->getType());
       if (T.isNull())
         return ExprError();
-      return FunctionParmPackExpr::Create(getSema().Context, T, PD,
-                                          E->getExprLoc(), *Pack);
+      auto *PackExpr = FunctionParmPackExpr::Create(getSema().Context, T, PD,
+                                                    E->getExprLoc(), *Pack);
+      getSema().MarkFunctionParmPackReferenced(PackExpr);
+      return PackExpr;
     }
 
     TransformedDecl = (*Pack)[getSema().ArgumentPackSubstitutionIndex];

clang/lib/Sema/TreeTransform.h

@@ -660,7 +660,10 @@ class TreeTransform {
                                           bool ExpectParameterPack);
 
   /// Transform the body of a lambda-expression.
-  StmtResult TransformLambdaBody(Stmt *Body);
+  StmtResult TransformLambdaBody(LambdaExpr *E, Stmt *Body);
+  /// Alternative implementation of TransformLambdaBody that skips transforming
+  /// the body.
+  StmtResult SkipLambdaBody(LambdaExpr *E, Stmt *Body);
 
   QualType TransformReferenceType(TypeLocBuilder &TLB, ReferenceTypeLoc TL);
 
@@ -11370,16 +11373,13 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
   bool Invalid = false;
 
   // Transform captures.
-  bool FinishedExplicitCaptures = false;
   for (LambdaExpr::capture_iterator C = E->capture_begin(),
                                  CEnd = E->capture_end();
        C != CEnd; ++C) {
     // When we hit the first implicit capture, tell Sema that we've finished
     // the list of explicit captures.
-    if (!FinishedExplicitCaptures && C->isImplicit()) {
-      getSema().finishLambdaExplicitCaptures(LSI);
-      FinishedExplicitCaptures = true;
-    }
+    if (C->isImplicit())
+      break;
 
     // Capturing 'this' is trivial.
     if (C->capturesThis()) {
@@ -11488,17 +11488,16 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
     getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind,
                                  EllipsisLoc);
   }
-  if (!FinishedExplicitCaptures)
-    getSema().finishLambdaExplicitCaptures(LSI);
+  getSema().finishLambdaExplicitCaptures(LSI);
 
-  // Enter a new evaluation context to insulate the lambda from any
-  // cleanups from the enclosing full-expression.
+  // FIXME: Sema's lambda-building mechanism expects us to push an expression
+  // evaluation context even if we're not transforming the function body.
   getSema().PushExpressionEvaluationContext(
       Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
 
   // Instantiate the body of the lambda expression.
   StmtResult Body =
-      Invalid ? StmtError() : getDerived().TransformLambdaBody(E->getBody());
+      Invalid ? StmtError() : getDerived().TransformLambdaBody(E, E->getBody());
 
   // ActOnLambda* will pop the function scope for us.
   FuncScopeCleanup.disable();
@@ -11524,10 +11523,50 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
 
 template<typename Derived>
 StmtResult
-TreeTransform<Derived>::TransformLambdaBody(Stmt *S) {
+TreeTransform<Derived>::TransformLambdaBody(LambdaExpr *E, Stmt *S) {
   return TransformStmt(S);
 }
 
+template<typename Derived>
+StmtResult
+TreeTransform<Derived>::SkipLambdaBody(LambdaExpr *E, Stmt *S) {
+  // Transform captures.
+  for (LambdaExpr::capture_iterator C = E->capture_begin(),
+                                 CEnd = E->capture_end();
+       C != CEnd; ++C) {
+    // When we hit the first implicit capture, tell Sema that we've finished
+    // the list of explicit captures.
+    if (!C->isImplicit())
+      continue;
+
+    // Capturing 'this' is trivial.
+    if (C->capturesThis()) {
+      getSema().CheckCXXThisCapture(C->getLocation(), C->isExplicit(),
+                                    /*BuildAndDiagnose*/ true, nullptr,
+                                    C->getCaptureKind() == LCK_StarThis);
+      continue;
+    }
+    // Captured expression will be recaptured during captured variables
+    // rebuilding.
+    if (C->capturesVLAType())
+      continue;
+
+    assert(C->capturesVariable() && "unexpected kind of lambda capture");
+    assert(!E->isInitCapture(C) && "implicit init-capture?");
+
+    // Transform the captured variable.
+    VarDecl *CapturedVar = cast_or_null<VarDecl>(
+        getDerived().TransformDecl(C->getLocation(), C->getCapturedVar()));
+    if (!CapturedVar || CapturedVar->isInvalidDecl())
+      return StmtError();
+
+    // Capture the transformed variable.
+    getSema().tryCaptureVariable(CapturedVar, C->getLocation());
+  }
+
+  return S;
+}
+
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformCXXUnresolvedConstructExpr(