From d4e462f6541fec1c58aba92c5ef538821feef4f6 Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Thu, 13 Sep 2018 23:28:25 +0000 Subject: [PATCH] [Sema] Remove location from implicit capture init expr A lambda's closure is initialized when the lambda is declared. For implicit captures, the initialization code emitted from EmitLambdaExpr references source locations *within the lambda body* in the function containing the lambda. This results in a poor debugging experience: we step to the line containing the lambda, then into lambda, out again, over and over, until every capture's field is initialized. To improve stepping behavior, assign the starting location of the lambda to expressions which initialize an implicit capture within it. rdar://39807527 Differential Revision: https://reviews.llvm.org/D50927 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@342194 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaLambda.cpp | 13 +++++++------ test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp | 4 ++-- .../expr.prim/expr.prim.lambda/templates.cpp | 4 ++-- test/CodeGenCXX/debug-info-lambda.cpp | 16 ++++++++++++++++ test/SemaCXX/uninitialized.cpp | 6 ++++-- .../RecursiveASTVisitorTests/DeclRefExpr.cpp | 10 +++++----- 6 files changed, 36 insertions(+), 17 deletions(-) create mode 100644 test/CodeGenCXX/debug-info-lambda.cpp diff --git a/lib/Sema/SemaLambda.cpp b/lib/Sema/SemaLambda.cpp index 3391567658..8000cb4fbf 100644 --- a/lib/Sema/SemaLambda.cpp +++ b/lib/Sema/SemaLambda.cpp @@ -1392,13 +1392,14 @@ static void addBlockPointerConversion(Sema &S, Class->addDecl(Conversion); } -static ExprResult performLambdaVarCaptureInitialization(Sema &S, - const Capture &Capture, - FieldDecl *Field) { +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(); - SourceLocation Loc = Capture.getLocation(); + SourceLocation Loc = + IsImplicitCapture ? ImplicitCaptureLoc : Capture.getLocation(); // C++11 [expr.prim.lambda]p21: // When the lambda-expression is evaluated, the entities that @@ -1607,8 +1608,8 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, Var, From.getEllipsisLoc())); Expr *Init = From.getInitExpr(); if (!Init) { - auto InitResult = - performLambdaVarCaptureInitialization(*this, From, *CurField); + auto InitResult = performLambdaVarCaptureInitialization( + *this, From, *CurField, CaptureDefaultLoc, IsImplicit); if (InitResult.isInvalid()) return ExprError(); Init = InitResult.get(); diff --git a/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp b/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp index 551c100ff7..7fc86e8109 100644 --- a/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp +++ b/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp @@ -15,8 +15,8 @@ public: void capture_by_copy(NonCopyable nc, NonCopyable &ncr, const NonConstCopy nco) { (void)[nc] { }; // expected-error{{capture of variable 'nc' as type 'NonCopyable' calls private copy constructor}} - (void)[=] { - ncr.foo(); // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} + (void)[=] { // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} + ncr.foo(); }(); [nco] {}(); // expected-error{{no matching constructor for initialization of 'const NonConstCopy'}} diff --git a/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp b/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp index 57e555e8d9..2bb75df7ac 100644 --- a/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp +++ b/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp @@ -88,8 +88,8 @@ namespace p2 { template void odr_used2(R &r, Boom boom) { const std::type_info &ti - = typeid([=,&r] () -> R& { - boom.tickle(); // expected-note{{in instantiation of member function}} + = typeid([=,&r] () -> R& { // expected-note{{in instantiation of member function 'p2::Boom::Boom' requested here}} + boom.tickle(); return r; }()); } diff --git a/test/CodeGenCXX/debug-info-lambda.cpp b/test/CodeGenCXX/debug-info-lambda.cpp new file mode 100644 index 0000000000..1b8f4e3cd3 --- /dev/null +++ b/test/CodeGenCXX/debug-info-lambda.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm \ +// RUN: -debug-info-kind=line-tables-only -dwarf-column-info -std=c++11 %s -o - | FileCheck %s + +// CHECK-LABEL: define{{.*}}lambda_in_func +void lambda_in_func(int &ref) { + // CHECK: [[ref_slot:%.*]] = getelementptr inbounds %class.anon, %class.anon* {{.*}}, i32 0, i32 0, !dbg [[lambda_decl_loc:![0-9]+]] + // CHECK-NEXT: %1 = load i32*, i32** %ref.addr, align 8, !dbg [[capture_init_loc:![0-9]+]] + // CHECK-NEXT: store i32* %1, i32** %0, align 8, !dbg [[lambda_decl_loc]] + // CHECK-NEXT: call void {{.*}}, !dbg [[lambda_call_loc:![0-9]+]] + + auto helper = [ // CHECK: [[lambda_decl_loc]] = !DILocation(line: [[@LINE]], column: 17 + &]() { // CHECK: [[capture_init_loc]] = !DILocation(line: [[@LINE]], column: 18 + ++ref; + }; + helper(); // CHECK: [[lambda_call_loc]] = !DILocation(line: [[@LINE]] +} diff --git a/test/SemaCXX/uninitialized.cpp b/test/SemaCXX/uninitialized.cpp index f620e1014c..331a948eb6 100644 --- a/test/SemaCXX/uninitialized.cpp +++ b/test/SemaCXX/uninitialized.cpp @@ -884,8 +884,10 @@ namespace lambdas { int x; }; A a0([] { return a0.x; }); // ok - void f() { - A a1([=] { return a1.x; }); // expected-warning{{variable 'a1' is uninitialized when used within its own initialization}} + void f() { + A a1([=] { // expected-warning{{variable 'a1' is uninitialized when used within its own initialization}} + return a1.x; + }); A a2([&] { return a2.x; }); // ok } } diff --git a/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp b/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp index 67640486ef..cd0e4260a8 100644 --- a/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp +++ b/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp @@ -75,11 +75,11 @@ TEST(RecursiveASTVisitor, VisitsUseOfImplicitLambdaCapture) { TEST(RecursiveASTVisitor, VisitsImplicitLambdaCaptureInit) { DeclRefExprVisitor Visitor; Visitor.setShouldVisitImplicitCode(true); - // We're expecting the "i" in the lambda to be visited twice: - // - Once for the DeclRefExpr in the lambda capture initialization (whose - // source code location is set to the first use of the variable). - // - Once for the DeclRefExpr for the use of "i" inside the lambda. - Visitor.ExpectMatch("i", 1, 24, /*Times=*/2); + // We're expecting "i" to be visited twice: once for the initialization expr + // for the captured variable "i" outside of the lambda body, and again for + // the use of "i" inside the lambda. + Visitor.ExpectMatch("i", 1, 20, /*Times=*/1); + Visitor.ExpectMatch("i", 1, 24, /*Times=*/1); EXPECT_TRUE(Visitor.runOver( "void f() { int i; [=]{ i; }; }", DeclRefExprVisitor::Lang_CXX11)); -- 2.50.1