From e518235aca345ad5dd708dd26c632e06122ffb09 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Sun, 2 Jun 2019 04:00:43 +0000 Subject: [PATCH] Factor out commonality between variable capture initialization and 'this' capture initialization. llvm-svn: 362317 --- clang/include/clang/Sema/Initialization.h | 4 +- clang/include/clang/Sema/Sema.h | 10 +- clang/lib/Sema/SemaLambda.cpp | 153 ++++++++++++++++-------------- clang/lib/Sema/SemaStmt.cpp | 40 ++++---- clang/test/AST/ast-dump-expr-json.cpp | 56 +++++++---- clang/test/AST/ast-dump-expr.cpp | 1 + 6 files changed, 149 insertions(+), 115 deletions(-) diff --git a/clang/include/clang/Sema/Initialization.h b/clang/include/clang/Sema/Initialization.h index 8efa2e7..14d8aa8 100644 --- a/clang/include/clang/Sema/Initialization.h +++ b/clang/include/clang/Sema/Initialization.h @@ -386,6 +386,8 @@ public: } /// Create the initialization entity for a lambda capture. + /// + /// \p VarID The name of the entity being captured, or nullptr for 'this'. static InitializedEntity InitializeLambdaCapture(IdentifierInfo *VarID, QualType FieldType, SourceLocation Loc) { @@ -509,7 +511,7 @@ public: /// For a lambda capture, return the capture's name. StringRef getCapturedVarName() const { assert(getKind() == EK_LambdaCapture && "Not a lambda capture!"); - return Capture.VarID->getName(); + return Capture.VarID ? Capture.VarID->getName() : "this"; } /// Determine the location of the capture when initializing diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 3e128df..a6db2f0 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5291,11 +5291,6 @@ public: const unsigned *const FunctionScopeIndexToStopAt = nullptr, bool ByCopy = false); - /// Initialize the given 'this' capture with a suitable 'this' or '*this' - /// expression. - ExprResult performThisCaptureInitialization(const sema::Capture &Capture, - bool IsImplicit); - /// Determine whether the given type is the type of *this that is used /// outside of the body of a member function for a type that is currently /// being defined. @@ -5808,6 +5803,11 @@ public: /// Build a FieldDecl suitable to hold the given capture. FieldDecl *BuildCaptureField(RecordDecl *RD, const sema::Capture &Capture); + /// Initialize the given capture with a suitable expression. + ExprResult BuildCaptureInit(const sema::Capture &Capture, + SourceLocation ImplicitCaptureLoc, + bool IsOpenMPMapping = false); + /// Complete a lambda-expression having processed and attached the /// lambda body. ExprResult BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index d3f3b60..4b832f56 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/lib/Sema/SemaLambda.cpp @@ -1431,32 +1431,22 @@ static void addBlockPointerConversion(Sema &S, Class->addDecl(Conversion); } -ExprResult Sema::performThisCaptureInitialization(const Capture &Cap, - bool IsImplicit) { - QualType ThisTy = getCurrentThisType(); - SourceLocation Loc = Cap.getLocation(); - Expr *This = BuildCXXThisExpr(Loc, ThisTy, IsImplicit); - if (Cap.isReferenceCapture()) - return This; - - // Capture (by copy) of '*this'. - Expr *StarThis = CreateBuiltinUnaryOp(Loc, UO_Deref, This).get(); - InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture( - nullptr, Cap.getCaptureType(), Loc); - InitializationKind InitKind = - InitializationKind::CreateDirect(Loc, Loc, Loc); - InitializationSequence Init(*this, Entity, InitKind, StarThis); - return Init.Perform(*this, Entity, InitKind, StarThis); -} - -static ExprResult performLambdaVarCaptureInitialization( - Sema &S, const Capture &Capture, FieldDecl *Field, - SourceLocation ImplicitCaptureLoc, bool IsImplicitCapture) { - assert(Capture.isVariableCapture() && "not a variable capture"); - - auto *Var = Capture.getVariable(); +ExprResult Sema::BuildCaptureInit(const Capture &Cap, + SourceLocation ImplicitCaptureLoc, + bool IsOpenMPMapping) { + // VLA captures don't have a stored initialization expression. + if (Cap.isVLATypeCapture()) + return ExprResult(); + + // An init-capture is initialized directly from its stored initializer. + if (Cap.isInitCapture()) + return Cap.getVariable()->getInit(); + + // For anything else, build an initialization expression. For an implicit + // capture, the capture notionally happens at the capture-default, so use + // that location here. SourceLocation Loc = - IsImplicitCapture ? ImplicitCaptureLoc : Capture.getLocation(); + ImplicitCaptureLoc.isValid() ? ImplicitCaptureLoc : Cap.getLocation(); // C++11 [expr.prim.lambda]p21: // When the lambda-expression is evaluated, the entities that @@ -1470,17 +1460,39 @@ static ExprResult performLambdaVarCaptureInitialization( // C++ [expr.prim.lambda]p12: // An entity captured by a lambda-expression is odr-used (3.2) in // the scope containing the lambda-expression. - ExprResult RefResult = S.BuildDeclarationNameExpr( + ExprResult Init; + IdentifierInfo *Name = nullptr; + if (Cap.isThisCapture()) { + QualType ThisTy = getCurrentThisType(); + Expr *This = BuildCXXThisExpr(Loc, ThisTy, ImplicitCaptureLoc.isValid()); + if (Cap.isCopyCapture()) + Init = CreateBuiltinUnaryOp(Loc, UO_Deref, This); + else + Init = This; + } else { + assert(Cap.isVariableCapture() && "unknown kind of capture"); + VarDecl *Var = Cap.getVariable(); + Name = Var->getIdentifier(); + Init = BuildDeclarationNameExpr( CXXScopeSpec(), DeclarationNameInfo(Var->getDeclName(), Loc), Var); - if (RefResult.isInvalid()) + } + + // In OpenMP, the capture kind doesn't actually describe how to capture: + // variables are "mapped" onto the device in a process that does not formally + // make a copy, even for a "copy capture". + if (IsOpenMPMapping) + return Init; + + if (Init.isInvalid()) return ExprError(); - Expr *Ref = RefResult.get(); - auto Entity = InitializedEntity::InitializeLambdaCapture( - Var->getIdentifier(), Field->getType(), Loc); - InitializationKind InitKind = InitializationKind::CreateDirect(Loc, Loc, Loc); - InitializationSequence Init(S, Entity, InitKind, Ref); - return Init.Perform(S, Entity, InitKind, Ref); + Expr *InitExpr = Init.get(); + InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture( + Name, Cap.getCaptureType(), Loc); + InitializationKind InitKind = + InitializationKind::CreateDirect(Loc, Loc, Loc); + InitializationSequence InitSeq(*this, Entity, InitKind, InitExpr); + return InitSeq.Perform(*this, Entity, InitKind, InitExpr); } ExprResult Sema::ActOnLambdaExpr(SourceLocation StartLoc, Stmt *Body, @@ -1647,14 +1659,18 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, assert(!From.isBlockCapture() && "Cannot capture __block variables"); bool IsImplicit = I >= LSI->NumExplicitCaptures; + SourceLocation ImplicitCaptureLoc = + IsImplicit ? CaptureDefaultLoc : SourceLocation(); // Use source ranges of explicit captures for fixits where available. SourceRange CaptureRange = LSI->ExplicitCaptureRanges[I]; // Warn about unused explicit captures. bool IsCaptureUsed = true; - if (!CurContext->isDependentContext() && !IsImplicit && !From.isODRUsed()) { + if (!CurContext->isDependentContext() && !IsImplicit && + !From.isODRUsed()) { // Initialized captures that are non-ODR used may not be eliminated. + // FIXME: Where did the IsGenericLambda here come from? bool NonODRUsedInitCapture = IsGenericLambda && From.isNonODRUsed() && From.isInitCapture(); if (!NonODRUsedInitCapture) { @@ -1682,46 +1698,43 @@ 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, - // because it results in a reference capture. Don't warn prior to - // C++2a; there's nothing that can be done about it before then. - if (getLangOpts().CPlusPlus2a && IsImplicit && - CaptureDefault == LCD_ByCopy) { - Diag(From.getLocation(), diag::warn_deprecated_this_capture); - Diag(CaptureDefaultLoc, diag::note_deprecated_this_capture) - << FixItHint::CreateInsertion( - getLocForEndOfToken(CaptureDefaultLoc), ", this"); + // Map the capture to our AST representation. + LambdaCapture Capture = [&] { + if (From.isThisCapture()) { + // Capturing 'this' implicitly with a default of '[=]' is deprecated, + // because it results in a reference capture. Don't warn prior to + // C++2a; there's nothing that can be done about it before then. + if (getLangOpts().CPlusPlus2a && IsImplicit && + CaptureDefault == LCD_ByCopy) { + Diag(From.getLocation(), diag::warn_deprecated_this_capture); + Diag(CaptureDefaultLoc, diag::note_deprecated_this_capture) + << FixItHint::CreateInsertion( + getLocForEndOfToken(CaptureDefaultLoc), ", this"); + } + return LambdaCapture(From.getLocation(), IsImplicit, + From.isCopyCapture() ? LCK_StarThis : LCK_This); + } else if (From.isVLATypeCapture()) { + return LambdaCapture(From.getLocation(), IsImplicit, LCK_VLAType); + } else { + assert(From.isVariableCapture() && "unknown kind of capture"); + VarDecl *Var = From.getVariable(); + LambdaCaptureKind Kind = + From.isCopyCapture() ? LCK_ByCopy : LCK_ByRef; + return LambdaCapture(From.getLocation(), IsImplicit, Kind, Var, + From.getEllipsisLoc()); } + }(); - ExprResult Init = performThisCaptureInitialization(From, IsImplicit); - Captures.push_back( - LambdaCapture(From.getLocation(), IsImplicit, - From.isCopyCapture() ? LCK_StarThis : LCK_This)); - CaptureInits.push_back(Init.get()); - continue; - } - if (From.isVLATypeCapture()) { - Captures.push_back( - LambdaCapture(From.getLocation(), IsImplicit, LCK_VLAType)); - CaptureInits.push_back(nullptr); - continue; - } + // Form the initializer for the capture field. + ExprResult Init = BuildCaptureInit(From, ImplicitCaptureLoc); - VarDecl *Var = From.getVariable(); - LambdaCaptureKind Kind = From.isCopyCapture() ? LCK_ByCopy : LCK_ByRef; - Captures.push_back(LambdaCapture(From.getLocation(), IsImplicit, Kind, - Var, From.getEllipsisLoc())); + // FIXME: Skip this capture if the capture is not used, the initializer + // has no side-effects, the type of the capture is trivial, and the + // lambda is not externally visible. - ExprResult Init = - From.isInitCapture() - ? Var->getInit() - : performLambdaVarCaptureInitialization( - *this, From, Field, CaptureDefaultLoc, IsImplicit); + // Add a FieldDecl for the capture and form its initializer. + BuildCaptureField(Class, From); + Captures.push_back(Capture); CaptureInits.push_back(Init.get()); } diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 3a7acd2..bc1e8f2 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -4231,39 +4231,35 @@ buildCapturedStmtCaptureList(Sema &S, CapturedRegionScopeInfo *RSI, if (Cap.isInvalid()) continue; + // Form the initializer for the capture. + ExprResult Init = S.BuildCaptureInit(Cap, Cap.getLocation(), + RSI->CapRegionKind == CR_OpenMP); + + // FIXME: Bail out now if the capture is not used and the initializer has + // no side-effects. + // Create a field for this capture. FieldDecl *Field = S.BuildCaptureField(RSI->TheRecordDecl, Cap); + // Add the capture to our list of captures. if (Cap.isThisCapture()) { - ExprResult Init = - S.performThisCaptureInitialization(Cap, /*Implicit*/ true); Captures.push_back(CapturedStmt::Capture(Cap.getLocation(), CapturedStmt::VCK_This)); - CaptureInits.push_back(Init.get()); - continue; } else if (Cap.isVLATypeCapture()) { Captures.push_back( CapturedStmt::Capture(Cap.getLocation(), CapturedStmt::VCK_VLAType)); - CaptureInits.push_back(nullptr); - continue; - } - - if (S.getLangOpts().OpenMP && RSI->CapRegionKind == CR_OpenMP) - S.setOpenMPCaptureKind(Field, Cap.getVariable(), RSI->OpenMPLevel); - - VarDecl *Var = Cap.getVariable(); - SourceLocation Loc = Cap.getLocation(); + } else { + assert(Cap.isVariableCapture() && "unknown kind of capture"); - // FIXME: For a non-reference capture, we need to build an expression to - // perform a copy here! - ExprResult Init = S.BuildDeclarationNameExpr( - CXXScopeSpec(), DeclarationNameInfo(Var->getDeclName(), Loc), Var); + if (S.getLangOpts().OpenMP && RSI->CapRegionKind == CR_OpenMP) + S.setOpenMPCaptureKind(Field, Cap.getVariable(), RSI->OpenMPLevel); - Captures.push_back(CapturedStmt::Capture(Loc, - Cap.isReferenceCapture() - ? CapturedStmt::VCK_ByRef - : CapturedStmt::VCK_ByCopy, - Var)); + Captures.push_back(CapturedStmt::Capture(Cap.getLocation(), + Cap.isReferenceCapture() + ? CapturedStmt::VCK_ByRef + : CapturedStmt::VCK_ByCopy, + Cap.getVariable())); + } CaptureInits.push_back(Init.get()); } return false; diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 3a8e745..d56d79d 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -3924,24 +3924,46 @@ void TestNonADLCall3() { // CHECK-NEXT: ] // CHECK-NEXT: }, // CHECK-NEXT: { -// CHECK-NEXT: "id": "0x{{.*}}", -// CHECK-NEXT: "kind": "CXXThisExpr", -// CHECK-NEXT: "range": { -// CHECK-NEXT: "begin": { -// CHECK-NEXT: "col": 8, -// CHECK-NEXT: "file": "{{.*}}", -// CHECK-NEXT: "line": 98 +// CHECK-NEXT: "id": "0x{{.*}}", +// CHECK-NEXT: "kind": "ParenListExpr", +// CHECK-NEXT: "range": { +// CHECK-NEXT: "begin": { +// CHECK-NEXT: "col": 8, +// CHECK-NEXT: "file": "{{.*}}", +// CHECK-NEXT: "line": 98 +// CHECK-NEXT: }, +// CHECK-NEXT: "end": { +// CHECK-NEXT: "col": 8, +// CHECK-NEXT: "file": "{{.*}}", +// CHECK-NEXT: "line": 98 +// CHECK-NEXT: } // CHECK-NEXT: }, -// CHECK-NEXT: "end": { -// CHECK-NEXT: "col": 8, -// CHECK-NEXT: "file": "{{.*}}", -// CHECK-NEXT: "line": 98 -// CHECK-NEXT: } -// CHECK-NEXT: }, -// CHECK-NEXT: "type": { -// CHECK-NEXT: "qualType": "V *" -// CHECK-NEXT: }, -// CHECK-NEXT: "valueCategory": "rvalue" +// CHECK-NEXT: "type": { +// CHECK-NEXT: "qualType": "NULL TYPE" +// CHECK-NEXT: }, +// CHECK-NEXT: "valueCategory": "rvalue", +// CHECK-NEXT: "inner": [ +// CHECK-NEXT: { +// CHECK-NEXT: "id": "0x{{.*}}", +// CHECK-NEXT: "kind": "CXXThisExpr", +// CHECK-NEXT: "range": { +// CHECK-NEXT: "begin": { +// CHECK-NEXT: "col": 8, +// CHECK-NEXT: "file": "{{.*}}", +// CHECK-NEXT: "line": 98 +// CHECK-NEXT: }, +// CHECK-NEXT: "end": { +// CHECK-NEXT: "col": 8, +// CHECK-NEXT: "file": "{{.*}}", +// CHECK-NEXT: "line": 98 +// CHECK-NEXT: } +// CHECK-NEXT: }, +// CHECK-NEXT: "type": { +// CHECK-NEXT: "qualType": "V *" +// CHECK-NEXT: }, +// CHECK-NEXT: "valueCategory": "rvalue" +// CHECK-NEXT: } +// CHECK-NEXT: ] // CHECK-NEXT: }, // CHECK-NEXT: { // CHECK-NEXT: "id": "0x{{.*}}", diff --git a/clang/test/AST/ast-dump-expr.cpp b/clang/test/AST/ast-dump-expr.cpp index 47f69a8..f04c311 100644 --- a/clang/test/AST/ast-dump-expr.cpp +++ b/clang/test/AST/ast-dump-expr.cpp @@ -255,6 +255,7 @@ void PrimaryExpressions(Ts... a) { // CHECK-NEXT: CXXMethodDecl // CHECK-NEXT: CompoundStmt // CHECK-NEXT: FieldDecl 0x{{[^ ]*}} col:8 implicit 'V *' + // CHECK-NEXT: ParenListExpr // CHECK-NEXT: CXXThisExpr 0x{{[^ ]*}} 'V *' this [*this]{}; -- 2.7.4