From c716e5d6dea2dec44e2f54da5a7ec41b1237011b Mon Sep 17 00:00:00 2001 From: Simon Pilgrim Date: Mon, 3 Jun 2019 09:56:09 +0000 Subject: [PATCH] Revert rL362358 : 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. ........ Fixes http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win buildbot failures llvm-svn: 362375 --- clang/include/clang/Sema/ScopeInfo.h | 19 +++---- clang/include/clang/Sema/Sema.h | 1 - clang/lib/Sema/ScopeInfo.cpp | 28 +++++----- clang/lib/Sema/SemaExpr.cpp | 50 +++++++---------- clang/lib/Sema/SemaExprCXX.cpp | 11 ++-- clang/lib/Sema/SemaTemplateInstantiate.cpp | 14 ++--- clang/lib/Sema/TreeTransform.h | 63 +++++----------------- .../SemaCXX/cxx1y-generic-lambdas-capturing.cpp | 27 ++-------- clang/test/SemaTemplate/lambda-capture-pack.cpp | 17 ------ 9 files changed, 69 insertions(+), 161 deletions(-) delete mode 100644 clang/test/SemaTemplate/lambda-capture-pack.cpp diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h index ea25951..177c88d 100644 --- a/clang/include/clang/Sema/ScopeInfo.h +++ b/clang/include/clang/Sema/ScopeInfo.h @@ -15,7 +15,6 @@ #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" @@ -914,8 +913,7 @@ public: /// }; /// } void addPotentialCapture(Expr *VarExpr) { - assert(isa(VarExpr) || isa(VarExpr) || - isa(VarExpr)); + assert(isa(VarExpr) || isa(VarExpr)); PotentiallyCapturingExprs.push_back(VarExpr); } @@ -967,15 +965,13 @@ public: /// 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(CapturingVarExpr) || - isa(CapturingVarExpr) || - isa(CapturingVarExpr)); + assert(isa(CapturingVarExpr) + || isa(CapturingVarExpr)); NonODRUsedCapturingExprs.insert(CapturingVarExpr); } bool isVariableExprMarkedAsNonODRUsed(Expr *CapturingVarExpr) const { - assert(isa(CapturingVarExpr) || - isa(CapturingVarExpr) || - isa(CapturingVarExpr)); + assert(isa(CapturingVarExpr) + || isa(CapturingVarExpr)); return NonODRUsedCapturingExprs.count(CapturingVarExpr); } void removePotentialCapture(Expr *E) { @@ -997,8 +993,9 @@ public: PotentialThisCaptureLocation.isValid(); } - void visitPotentialCaptures( - llvm::function_ref Callback) const; + // When passed the index, returns the VarDecl and Expr associated + // with the index. + void getPotentialVariableCapture(unsigned Idx, VarDecl *&VD, Expr *&E) const; }; FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy() diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 096bebf..b4f721c 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4179,7 +4179,6 @@ public: 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); diff --git a/clang/lib/Sema/ScopeInfo.cpp b/clang/lib/Sema/ScopeInfo.cpp index b2a26af..e84e592 100644 --- a/clang/lib/Sema/ScopeInfo.cpp +++ b/clang/lib/Sema/ScopeInfo.cpp @@ -229,20 +229,20 @@ bool CapturingScopeInfo::isVLATypeCaptured(const VariableArrayType *VAT) const { return false; } -void LambdaScopeInfo::visitPotentialCaptures( - llvm::function_ref Callback) const { - for (Expr *E : PotentiallyCapturingExprs) { - if (auto *DRE = dyn_cast(E)) { - Callback(cast(DRE->getFoundDecl()), E); - } else if (auto *ME = dyn_cast(E)) { - Callback(cast(ME->getMemberDecl()), E); - } else if (auto *FP = dyn_cast(E)) { - for (VarDecl *VD : *FP) - Callback(VD, E); - } else { - llvm_unreachable("unexpected expression in potential captures list"); - } - } +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(E)) + VD = dyn_cast(DRE->getFoundDecl()); + else if (MemberExpr *ME = dyn_cast(E)) + VD = dyn_cast(ME->getMemberDecl()); + else + llvm_unreachable("Only DeclRefExprs or MemberExprs should be added for " + "potential captures"); + assert(VD); } FunctionScopeInfo::~FunctionScopeInfo() { } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index d0b2760..72b61b8 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -14610,9 +14610,7 @@ 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(LambdaExpr *E, Stmt *Body) { - return SkipLambdaBody(E, Body); - } + StmtResult TransformLambdaBody(Stmt *Body) { return Body; } }; } @@ -15056,7 +15054,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 = nullptr) { + const unsigned *const FunctionScopeIndexToStopAt) { // 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 && @@ -15737,19 +15735,14 @@ void Sema::UpdateMarkingForLValueToRValue(Expr *E) { // variable. if (LambdaScopeInfo *LSI = getCurLambda()) { Expr *SansParensExpr = E->IgnoreParens(); - VarDecl *Var; - ArrayRef Vars = None; + VarDecl *Var = nullptr; if (DeclRefExpr *DRE = dyn_cast(SansParensExpr)) - Vars = Var = dyn_cast(DRE->getFoundDecl()); + Var = dyn_cast(DRE->getFoundDecl()); else if (MemberExpr *ME = dyn_cast(SansParensExpr)) - Vars = Var = dyn_cast(ME->getMemberDecl()); - else if (auto *FPPE = dyn_cast(SansParensExpr)) - Vars = llvm::makeArrayRef(FPPE->begin(), FPPE->end()); + Var = dyn_cast(ME->getMemberDecl()); - for (VarDecl *VD : Vars) { - if (Var && IsVariableNonDependentAndAConstantExpression(VD, Context)) - LSI->markVariableExprAsNonODRUsed(SansParensExpr); - } + if (Var && IsVariableNonDependentAndAConstantExpression(Var, Context)) + LSI->markVariableExprAsNonODRUsed(SansParensExpr); } } @@ -15774,18 +15767,20 @@ void Sema::CleanupVarDeclMarking() { std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs); for (Expr *E : LocalMaybeODRUseExprs) { - if (auto *DRE = dyn_cast(E)) { - MarkVarDeclODRUsed(cast(DRE->getDecl()), - DRE->getLocation(), *this); - } else if (auto *ME = dyn_cast(E)) { - MarkVarDeclODRUsed(cast(ME->getMemberDecl()), ME->getMemberLoc(), - *this); - } else if (auto *FP = dyn_cast(E)) { - for (VarDecl *VD : *FP) - MarkVarDeclODRUsed(VD, FP->getParameterPackLocation(), *this); + VarDecl *Var; + SourceLocation Loc; + if (DeclRefExpr *DRE = dyn_cast(E)) { + Var = cast(DRE->getDecl()); + Loc = DRE->getLocation(); + } else if (MemberExpr *ME = dyn_cast(E)) { + Var = cast(ME->getMemberDecl()); + Loc = ME->getMemberLoc(); } else { llvm_unreachable("Unexpected expression"); } + + MarkVarDeclODRUsed(Var, Loc, *this, + /*MaxFunctionScopeIndex Pointer*/ nullptr); } assert(MaybeODRUseExprs.empty() && @@ -15794,8 +15789,7 @@ void Sema::CleanupVarDeclMarking() { static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc, VarDecl *Var, Expr *E) { - assert((!E || isa(E) || isa(E) || - isa(E)) && + assert((!E || isa(E) || isa(E)) && "Invalid Expr argument to DoMarkVarDeclReferenced"); Var->setReferenced(); @@ -16028,12 +16022,6 @@ 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 diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 5884cf9..ac050fa 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -7427,7 +7427,12 @@ 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. - CurrentLSI->visitPotentialCaptures([&] (VarDecl *Var, Expr *VarExpr) { + const unsigned NumPotentialCaptures = + CurrentLSI->getNumPotentialVariableCaptures(); + for (unsigned I = 0; I != NumPotentialCaptures; ++I) { + Expr *VarExpr = nullptr; + VarDecl *Var = nullptr; + CurrentLSI->getPotentialVariableCapture(I, Var, 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. @@ -7441,7 +7446,7 @@ static void CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures( // } if (CurrentLSI->isVariableExprMarkedAsNonODRUsed(VarExpr) && !IsFullExprInstantiationDependent) - return; + continue; // If we have a capture-capable lambda for the variable, go ahead and // capture the variable in that lambda (and all its enclosing lambdas). @@ -7473,7 +7478,7 @@ static void CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures( DeclRefType, nullptr); } } - }); + } // Check if 'this' needs to be captured. if (CurrentLSI->hasPotentialThisCapture()) { diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 973f564..ba54d50 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -1368,11 +1368,9 @@ TemplateInstantiator::TransformFunctionParmPackExpr(FunctionParmPackExpr *E) { Vars.push_back(D); } - auto *PackExpr = - FunctionParmPackExpr::Create(getSema().Context, T, E->getParameterPack(), - E->getParameterPackLocation(), Vars); - getSema().MarkFunctionParmPackReferenced(PackExpr); - return PackExpr; + return FunctionParmPackExpr::Create(getSema().Context, T, + E->getParameterPack(), + E->getParameterPackLocation(), Vars); } ExprResult @@ -1391,10 +1389,8 @@ TemplateInstantiator::TransformFunctionParmPackRefExpr(DeclRefExpr *E, QualType T = TransformType(E->getType()); if (T.isNull()) return ExprError(); - auto *PackExpr = FunctionParmPackExpr::Create(getSema().Context, T, PD, - E->getExprLoc(), *Pack); - getSema().MarkFunctionParmPackReferenced(PackExpr); - return PackExpr; + return FunctionParmPackExpr::Create(getSema().Context, T, PD, + E->getExprLoc(), *Pack); } TransformedDecl = (*Pack)[getSema().ArgumentPackSubstitutionIndex]; diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index a1a9aae..592787a 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -660,10 +660,7 @@ public: bool ExpectParameterPack); /// Transform the body of a lambda-expression. - StmtResult TransformLambdaBody(LambdaExpr *E, Stmt *Body); - /// Alternative implementation of TransformLambdaBody that skips transforming - /// the body. - StmtResult SkipLambdaBody(LambdaExpr *E, Stmt *Body); + StmtResult TransformLambdaBody(Stmt *Body); QualType TransformReferenceType(TypeLocBuilder &TLB, ReferenceTypeLoc TL); @@ -11361,13 +11358,16 @@ TreeTransform::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 (C->isImplicit()) - break; + if (!FinishedExplicitCaptures && C->isImplicit()) { + getSema().finishLambdaExplicitCaptures(LSI); + FinishedExplicitCaptures = true; + } // Capturing 'this' is trivial. if (C->capturesThis()) { @@ -11476,16 +11476,17 @@ TreeTransform::TransformLambdaExpr(LambdaExpr *E) { getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind, EllipsisLoc); } - getSema().finishLambdaExplicitCaptures(LSI); + if (!FinishedExplicitCaptures) + getSema().finishLambdaExplicitCaptures(LSI); - // FIXME: Sema's lambda-building mechanism expects us to push an expression - // evaluation context even if we're not transforming the function body. + // Enter a new evaluation context to insulate the lambda from any + // cleanups from the enclosing full-expression. getSema().PushExpressionEvaluationContext( Sema::ExpressionEvaluationContext::PotentiallyEvaluated); // Instantiate the body of the lambda expression. StmtResult Body = - Invalid ? StmtError() : getDerived().TransformLambdaBody(E, E->getBody()); + Invalid ? StmtError() : getDerived().TransformLambdaBody(E->getBody()); // ActOnLambda* will pop the function scope for us. FuncScopeCleanup.disable(); @@ -11511,51 +11512,11 @@ TreeTransform::TransformLambdaExpr(LambdaExpr *E) { template StmtResult -TreeTransform::TransformLambdaBody(LambdaExpr *E, Stmt *S) { +TreeTransform::TransformLambdaBody(Stmt *S) { return TransformStmt(S); } template -StmtResult -TreeTransform::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( - getDerived().TransformDecl(C->getLocation(), C->getCapturedVar())); - if (!CapturedVar || CapturedVar->isInvalidDecl()) - return StmtError(); - - // Capture the transformed variable. - getSema().tryCaptureVariable(CapturedVar, C->getLocation()); - } - - return S; -} - -template ExprResult TreeTransform::TransformCXXUnresolvedConstructExpr( CXXUnresolvedConstructExpr *E) { diff --git a/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp b/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp index a98366c..eaed45a 100644 --- a/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp +++ b/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -emit-llvm-only %s -// RUN: %clang_cc1 -std=c++2a -verify -fsyntax-only -fblocks -emit-llvm-only %s // DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s -DDELAYED_TEMPLATE_PARSING // DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fms-extensions %s -DMS_EXTENSIONS // DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fdelayed-template-parsing -fms-extensions %s -DMS_EXTENSIONS -DDELAYED_TEMPLATE_PARSING @@ -177,13 +176,7 @@ void doit() { sample::X cx{5}; auto L = [=](auto a) { const int z = 3; - // FIXME: The warning below is correct but for some reason doesn't show - // up in C++17 mode. return [&,a](auto b) { -#if __cplusplus > 201702L - // expected-warning@-2 {{address of stack memory associated with local variable 'z' returned}} - // expected-note@#call {{in instantiation of}} -#endif const int y = 5; return [=](auto c) { int d[sizeof(a) == sizeof(c) || sizeof(c) == sizeof(b) ? 2 : 1]; @@ -196,7 +189,7 @@ void doit() { }; }; }; - auto M = L(3)(3.5); // #call + auto M = L(3)(3.5); M(3.14); } } @@ -1526,20 +1519,6 @@ void test() { } // end ns5 -} // end PR34266 -namespace capture_pack { -#if __cplusplus >= 201702L - constexpr -#endif - auto v = - [](auto ...a) { - [&](auto ...b) { - ((a = b), ...); // expected-warning 0-1{{extension}} - }(100, 20, 3); - return (a + ...); // expected-warning 0-1{{extension}} - }(400, 50, 6); -#if __cplusplus >= 201702L - static_assert(v == 123); -#endif -} + +} // end PR34266 diff --git a/clang/test/SemaTemplate/lambda-capture-pack.cpp b/clang/test/SemaTemplate/lambda-capture-pack.cpp deleted file mode 100644 index 2fe5767..0000000 --- a/clang/test/SemaTemplate/lambda-capture-pack.cpp +++ /dev/null @@ -1,17 +0,0 @@ -// RUN: %clang_cc1 -std=c++2a -verify %s -// expected-no-diagnostics - -template void check_sizes(Lambda ...L) { - static_assert(((sizeof(T) == sizeof(Lambda)) && ...)); -} - -template void f(T ...v) { - // Pack expansion of lambdas: each lambda captures only one pack element. - check_sizes([=] { (void)&v; } ...); - - // Pack expansion inside lambda: captures all pack elements. - auto l = [=] { ((void)&v, ...); }; - static_assert(sizeof(l) >= (sizeof(T) + ...)); -} - -template void f(int, char, double); -- 2.7.4