From: Eric Fiselier <eric@efcs.ca>
Date: Sat, 3 Jun 2017 00:22:18 +0000 (+0000)
Subject: [coroutines] Fix rebuilding of dependent coroutine parameters
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=98cf823022d1d71065c71e9338226ebf8bfa36ba;p=clang

[coroutines] Fix rebuilding of dependent coroutine parameters

Summary:
We were not handling correctly rebuilding of parameter and were not creating copies for them.
Now we will always rebuild parameter moves in TreeTransform's TransformCoroutineBodyStmt.

Reviewers: rsmith, GorNishanov

Reviewed By: rsmith

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D33797

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@304620 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Sema/CoroutineStmtBuilder.h b/lib/Sema/CoroutineStmtBuilder.h
index 954a0f100e..33a368d92f 100644
--- a/lib/Sema/CoroutineStmtBuilder.h
+++ b/lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@ public:
   /// name lookup.
   bool buildDependentStatements();
 
+  /// \brief Build just parameter moves. To use for rebuilding in TreeTransform.
+  bool buildParameterMoves();
+
   bool isInvalid() const { return !this->IsValid; }
 
 private:
diff --git a/lib/Sema/SemaCoroutine.cpp b/lib/Sema/SemaCoroutine.cpp
index 8a548c0ab8..06ae66076e 100644
--- a/lib/Sema/SemaCoroutine.cpp
+++ b/lib/Sema/SemaCoroutine.cpp
@@ -832,6 +832,12 @@ bool CoroutineStmtBuilder::buildDependentStatements() {
   return this->IsValid;
 }
 
+bool CoroutineStmtBuilder::buildParameterMoves() {
+  assert(this->IsValid && "coroutine already invalid");
+  assert(this->ParamMoves.empty() && "param moves already built");
+  return this->IsValid = makeParamMoves();
+}
+
 bool CoroutineStmtBuilder::makePromiseStmt() {
   // Form a declaration statement for the promise declaration, so that AST
   // visitors can more easily find it.
@@ -1244,14 +1250,13 @@ static Expr *castForMoving(Sema &S, Expr *E, QualType T = QualType()) {
       .get();
 }
 
+
 /// \brief Build a variable declaration for move parameter.
 static VarDecl *buildVarDecl(Sema &S, SourceLocation Loc, QualType Type,
-                             StringRef Name) {
-  DeclContext *DC = S.CurContext;
-  IdentifierInfo *II = &S.PP.getIdentifierTable().get(Name);
+                             IdentifierInfo *II) {
   TypeSourceInfo *TInfo = S.Context.getTrivialTypeSourceInfo(Type, Loc);
   VarDecl *Decl =
-      VarDecl::Create(S.Context, DC, Loc, Loc, II, Type, TInfo, SC_None);
+      VarDecl::Create(S.Context, S.CurContext, Loc, Loc, II, Type, TInfo, SC_None);
   Decl->setImplicit();
   return Decl;
 }
@@ -1264,9 +1269,6 @@ bool CoroutineStmtBuilder::makeParamMoves() {
 
     // No need to copy scalars, llvm will take care of them.
     if (Ty->getAsCXXRecordDecl()) {
-      if (!paramDecl->getIdentifier())
-        continue;
-
       ExprResult ParamRef =
           S.BuildDeclRefExpr(paramDecl, paramDecl->getType(),
                              ExprValueKind::VK_LValue, Loc); // FIXME: scope?
@@ -1275,8 +1277,7 @@ bool CoroutineStmtBuilder::makeParamMoves() {
 
       Expr *RCast = castForMoving(S, ParamRef.get());
 
-      auto D = buildVarDecl(S, Loc, Ty, paramDecl->getIdentifier()->getName());
-
+      auto D = buildVarDecl(S, Loc, Ty, paramDecl->getIdentifier());
       S.AddInitializerToDecl(D, RCast, /*DirectInit=*/true);
 
       // Convert decl to a statement.
diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index a65584e3c9..7aa8f64d50 100644
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -6959,6 +6959,8 @@ TreeTransform<Derived>::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
       Builder.ReturnStmt = Res.get();
     }
   }
+  if (!Builder.buildParameterMoves())
+    return StmtError();
 
   return getDerived().RebuildCoroutineBodyStmt(Builder);
 }
diff --git a/test/CodeGenCoroutines/coro-params.cpp b/test/CodeGenCoroutines/coro-params.cpp
index c88e503a65..540f84585c 100644
--- a/test/CodeGenCoroutines/coro-params.cpp
+++ b/test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,37 @@ void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
   // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
   // CHECK-NEXT: call i8* @llvm.coro.free(
 }
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1A1BEvT_T0_S3_(%struct.A* %x, %struct.B*, %struct.B* %y)
+template <typename T, typename U>
+void dependent_params(T x, U, U y) {
+  // CHECK: %[[x_copy:.+]] = alloca %struct.A
+  // CHECK-NEXT: %[[unnamed_copy:.+]] = alloca %struct.B
+  // CHECK-NEXT: %[[y_copy:.+]] = alloca %struct.B
+
+  // CHECK: call i8* @llvm.coro.begin
+  // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %[[x_copy]], %struct.A* dereferenceable(512) %x)
+  // CHECK-NEXT: call void @_ZN1BC1EOS_(%struct.B* %[[unnamed_copy]], %struct.B* dereferenceable(512) %0)
+  // CHECK-NEXT: call void @_ZN1BC1EOS_(%struct.B* %[[y_copy]], %struct.B* dereferenceable(512) %y)
+  // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJv1A1BS2_EE12promise_typeC1Ev(
+
+  co_return;
+}
+
+struct A {
+  int WontFitIntoRegisterForSure[128];
+  A();
+  A(A&&) noexcept;
+  ~A();
+};
+
+struct B {
+  int WontFitIntoRegisterForSure[128];
+  B();
+  B(B&&) noexcept;
+  ~B();
+};
+
+void call_dependent_params() {
+  dependent_params(A{}, B{}, B{});
+}
diff --git a/test/SemaCXX/coroutines.cpp b/test/SemaCXX/coroutines.cpp
index a867cf68a6..4eedc2162f 100644
--- a/test/SemaCXX/coroutines.cpp
+++ b/test/SemaCXX/coroutines.cpp
@@ -892,3 +892,16 @@ void test_bad_suspend() {
     co_await d; // OK
   }
 }
+
+template <int ID = 0>
+struct NoCopy {
+  NoCopy(NoCopy const&) = delete; // expected-note 2 {{deleted here}}
+};
+template <class T, class U>
+void test_dependent_param(T t, U) {
+  // expected-error@-1 {{call to deleted constructor of 'NoCopy<0>'}}
+  // expected-error@-2 {{call to deleted constructor of 'NoCopy<1>'}}
+  ((void)t);
+  co_return 42;
+}
+template void test_dependent_param(NoCopy<0>, NoCopy<1>); // expected-note {{requested here}}