PR24164, PR39336: init-captures are not distinct full-expressions.

Rather, they are subexpressions of the enclosing lambda-expression, and any temporaries in them are destroyed at the end of that full-expression, or when the corresponding lambda-expression is destroyed if they are lifetime-extended. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@344801 91177308-0d34-0410-b5e6-96231b3b80d8
dylansturg · Oct 19, 2018 · 3c43b80 · 3c43b80
1 parent 396dc16
commit 3c43b80
Show file tree

Hide file tree

Showing 9 changed files with 95 additions and 50 deletions.
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
@@ -5315,8 +5315,7 @@ class Sema {
   }
   ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
                                  bool DiscardedValue = false,
-                                 bool IsConstexpr = false,
-                                 bool IsLambdaInitCaptureInitializer = false);
+                                 bool IsConstexpr = false);
   StmtResult ActOnFinishFullStmt(Stmt *Stmt);
 
   // Marks SS invalid if it represents an incomplete type.

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
@@ -3255,11 +3255,8 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
 
   case LambdaExprClass: {
     const LambdaExpr *LE = cast<LambdaExpr>(this);
-    for (LambdaExpr::capture_iterator I = LE->capture_begin(),
-                                      E = LE->capture_end(); I != E; ++I)
-      if (I->getCaptureKind() == LCK_ByCopy)
-        // FIXME: Only has a side-effect if the variable is volatile or if
-        // the copy would invoke a non-trivial copy constructor.
+    for (Expr *E : LE->capture_inits())
+      if (E->HasSideEffects(Ctx, IncludePossibleEffects))
         return true;
     return false;
   }

diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
@@ -2252,7 +2252,6 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr,
 }
 
 void CodeGenFunction::EmitLambdaExpr(const LambdaExpr *E, AggValueSlot Slot) {
-  RunCleanupsScope Scope(*this);
   LValue SlotLV = MakeAddrLValue(Slot.getAddress(), E->getType());
 
   CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin();

diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
@@ -7764,41 +7764,24 @@ Sema::CorrectDelayedTyposInExpr(Expr *E, VarDecl *InitDecl,
 
 ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
                                      bool DiscardedValue,
-                                     bool IsConstexpr,
-                                     bool IsLambdaInitCaptureInitializer) {
+                                     bool IsConstexpr) {
   ExprResult FullExpr = FE;
 
   if (!FullExpr.get())
     return ExprError();
 
-  // If we are an init-expression in a lambdas init-capture, we should not
-  // diagnose an unexpanded pack now (will be diagnosed once lambda-expr
-  // containing full-expression is done).
-  // template<class ... Ts> void test(Ts ... t) {
-  //   test([&a(t)]() { <-- (t) is an init-expr that shouldn't be diagnosed now.
-  //     return a;
-  //   }() ...);
-  // }
-  // FIXME: This is a hack. It would be better if we pushed the lambda scope
-  // when we parse the lambda introducer, and teach capturing (but not
-  // unexpanded pack detection) to walk over LambdaScopeInfos which don't have a
-  // corresponding class yet (that is, have LambdaScopeInfo either represent a
-  // lambda where we've entered the introducer but not the body, or represent a
-  // lambda where we've entered the body, depending on where the
-  // parser/instantiation has got to).
-  if (!IsLambdaInitCaptureInitializer &&
-      DiagnoseUnexpandedParameterPack(FullExpr.get()))
+  if (DiagnoseUnexpandedParameterPack(FullExpr.get()))
     return ExprError();
 
-  // Top-level expressions default to 'id' when we're in a debugger.
-  if (DiscardedValue && getLangOpts().DebuggerCastResultToId &&
-      FullExpr.get()->getType() == Context.UnknownAnyTy) {
-    FullExpr = forceUnknownAnyToType(FullExpr.get(), Context.getObjCIdType());
-    if (FullExpr.isInvalid())
-      return ExprError();
-  }
-
   if (DiscardedValue) {
+    // Top-level expressions default to 'id' when we're in a debugger.
+    if (getLangOpts().DebuggerCastResultToId &&
+        FullExpr.get()->getType() == Context.UnknownAnyTy) {
+      FullExpr = forceUnknownAnyToType(FullExpr.get(), Context.getObjCIdType());
+      if (FullExpr.isInvalid())
+        return ExprError();
+    }
+
     FullExpr = CheckPlaceholderExpr(FullExpr.get());
     if (FullExpr.isInvalid())
       return ExprError();

diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
@@ -6786,6 +6786,20 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
     return;
   }
 
+  // The lifetime of an init-capture is that of the closure object constructed
+  // by a lambda-expression.
+  if (auto *LE = dyn_cast<LambdaExpr>(Init)) {
+    for (Expr *E : LE->capture_inits()) {
+      if (!E)
+        continue;
+      if (E->isGLValue())
+        visitLocalsRetainedByReferenceBinding(Path, E, RK_ReferenceBinding,
+                                              Visit);
+      else
+        visitLocalsRetainedByInitializer(Path, E, Visit, true);
+    }
+  }
+
   if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init))
     return visitLifetimeBoundArguments(Path, Init, Visit);
 

diff --git a/lib/Sema/SemaLambda.cpp b/lib/Sema/SemaLambda.cpp
@@ -773,16 +773,6 @@ QualType Sema::buildLambdaInitCaptureInitialization(SourceLocation Loc,
   InitializationSequence InitSeq(*this, Entity, Kind, Args);
   ExprResult Result = InitSeq.Perform(*this, Entity, Kind, Args, &DclT);
 
-  if (Result.isInvalid())
-    return QualType();
-  Init = Result.getAs<Expr>();
-
-  // The init-capture initialization is a full-expression that must be
-  // processed as one before we enter the declcontext of the lambda's
-  // call-operator.
-  Result = ActOnFinishFullExpr(Init, Loc, /*DiscardedValue*/ false,
-                               /*IsConstexpr*/ false,
-                               /*IsLambdaInitCaptureInitializer*/ true);
   if (Result.isInvalid())
     return QualType();
 

diff --git a/test/CXX/special/class.temporary/p6.cpp b/test/CXX/special/class.temporary/p6.cpp
@@ -1,4 +1,15 @@
-// RUN: %clang_cc1 -std=c++17 %s -emit-llvm -o - | FileCheck %s --implicit-check-not='call{{.*}}dtor'
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s --implicit-check-not='call{{.*}}dtor'
+
+namespace std {
+  typedef decltype(sizeof(int)) size_t;
+
+  template <class E>
+  struct initializer_list {
+    const E *begin;
+    size_t   size;
+    initializer_list() : begin(nullptr), size(0) {}
+  };
+}
 
 void then();
 
@@ -8,6 +19,14 @@ struct dtor {
 
 dtor ctor();
 
+auto &&lambda = [a = {ctor()}] {};
+// CHECK-LABEL: define
+// CHECK: call {{.*}}ctor
+// CHECK: call {{.*}}atexit{{.*}}global_array_dtor
+
+// CHECK-LABEL: define{{.*}}global_array_dtor
+// CHECK: call {{.*}}dtor
+
 // [lifetime extension occurs if the object was obtained by]
 //  -- a temporary materialization conversion
 // CHECK-LABEL: ref_binding
@@ -188,3 +207,34 @@ void comma() {
   // CHECK: call {{.*}}dtor
   // CHECK: }
 }
+
+
+// This applies recursively: if an object is lifetime-extended and contains a
+// reference, the referent is also extended.
+// CHECK-LABEL: init_capture_ref
+void init_capture_ref() {
+  // CHECK: call {{.*}}ctor
+  auto x = [&a = (const dtor&)ctor()] {};
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+// CHECK-LABEL: init_capture_ref_indirect
+void init_capture_ref_indirect() {
+  // CHECK: call {{.*}}ctor
+  auto x = [&a = (const dtor&)ctor()] {};
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+// CHECK-LABEL: init_capture_init_list
+void init_capture_init_list() {
+  // CHECK: call {{.*}}ctor
+  auto x = [a = {ctor()}] {};
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
diff --git a/test/CodeGenCXX/cxx1y-init-captures.cpp b/test/CodeGenCXX/cxx1y-init-captures.cpp
@@ -38,6 +38,19 @@ void g() {
 
 // CHECK: add nsw i32
 
+// CHECK-LABEL: define void @_Z18init_capture_dtorsv
+void init_capture_dtors() {
+  // Ensure that init-captures are not treated as separate full-expressions.
+  struct HasDtor { ~HasDtor() {} };
+  void some_function_call();
+  void other_function_call();
+  // CHECK: call {{.*}}some_function_call
+  // CHECK: call {{.*}}HasDtorD
+  ([x = (HasDtor(), 0)]{}, some_function_call());
+  // CHECK: call {{.*}}other_function_call
+  other_function_call();
+}
+
 int h(int a) {
   // CHECK-LABEL: define i32 @_Z1hi(
   // CHECK: %[[A_ADDR:.*]] = alloca i32,

diff --git a/test/SemaCXX/cxx1y-init-captures.cpp b/test/SemaCXX/cxx1y-init-captures.cpp
@@ -144,13 +144,13 @@ int test(T t = T{}) {
     };
   }
   { // will need to capture x in outer lambda
-    const int x = 10; //expected-note 2{{declared}}
-    auto L = [z = x](char a) { //expected-note 2{{begins}}
-      auto M = [&y = x](T b) { //expected-error 2{{cannot be implicitly captured}}
+    const int x = 10; //expected-note {{declared}}
+    auto L = [z = x](char a) { //expected-note {{begins}}
+      auto M = [&y = x](T b) { //expected-error {{cannot be implicitly captured}}
         return y;
       };
       return M;
-    };     
+    };
   }
   {
     // no captures