commit cac0a5971879533625fed92f6c7a9ff3c675cf55
author Eric Fiselier <eric@efcs.ca> Sat Mar 11 02:35:37 2017 +0000
committer Eric Fiselier <eric@efcs.ca> Sat Mar 11 02:35:37 2017 +0000
tree be512ab1006ed64f9d24996fc1c5578d47b05579
parent 81d1744519129755e64cc9f262249f89fda81e85

[coroutines] Fix diagnostics depending on the first coroutine statement.

Summary:
Some coroutine diagnostics need to point to the location of the first coroutine keyword in the function, like when diagnosing a `return` inside a coroutine. Previously we did this by storing each *valid* coroutine statement in a list and select the first one to use in diagnostics. However if every coroutine statement is invalid we would have no location to point to.

This patch fixes the storage of the first coroutine statement location, ensuring that it gets stored even when the resulting AST node would be invalid. 
This patch also removes the `CoroutineStmts` list in `FunctionScopeInfo` because it was unused.

Reviewers: rsmith, GorNishanov, aaron.ballman

Reviewed By: GorNishanov

Subscribers: mehdi_amini, cfe-commits

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

llvm-svn: 297547

clang/lib/Sema/SemaCoroutine.cpp

diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 7bbb2f4..4d0f8a8 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -400,7 +400,8 @@
 
 /// Check that this is a context in which a coroutine suspension can appear.
 static FunctionScopeInfo *checkCoroutineContext(Sema &S, SourceLocation Loc,
-                                                StringRef Keyword) {
+                                                StringRef Keyword,
+                                                bool IsImplicit = false) {
   if (!isValidCoroutineContext(S, Loc, Keyword))
     return nullptr;
 
@@ -409,6 +410,9 @@
   auto *ScopeInfo = S.getCurFunction();
   assert(ScopeInfo && "missing function scope for function");
 
+  if (ScopeInfo->FirstCoroutineStmtLoc.isInvalid() && !IsImplicit)
+    ScopeInfo->setFirstCoroutineStmt(Loc, Keyword);
+
   if (ScopeInfo->CoroutinePromise)
     return ScopeInfo;
 
@@ -488,7 +492,7 @@
 }
 
 ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *E,
-                                           UnresolvedLookupExpr *Lookup) {
+                                            UnresolvedLookupExpr *Lookup) {
   auto *FSI = checkCoroutineContext(*this, Loc, "co_await");
   if (!FSI)
     return ExprError();
@@ -504,7 +508,6 @@
   if (Promise->getType()->isDependentType()) {
     Expr *Res =
         new (Context) DependentCoawaitExpr(Loc, Context.DependentTy, E, Lookup);
-    FSI->CoroutineStmts.push_back(Res);
     return Res;
   }
 
@@ -528,7 +531,7 @@
 
 ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *E,
                                   bool IsImplicit) {
-  auto *Coroutine = checkCoroutineContext(*this, Loc, "co_await");
+  auto *Coroutine = checkCoroutineContext(*this, Loc, "co_await", IsImplicit);
   if (!Coroutine)
     return ExprError();
 
@@ -541,8 +544,6 @@
   if (E->getType()->isDependentType()) {
     Expr *Res = new (Context)
         CoawaitExpr(Loc, Context.DependentTy, E, IsImplicit);
-    if (!IsImplicit)
-      Coroutine->CoroutineStmts.push_back(Res);
     return Res;
   }
 
@@ -560,8 +561,7 @@
   Expr *Res =
       new (Context) CoawaitExpr(Loc, E, RSS.Results[0], RSS.Results[1],
                                 RSS.Results[2], RSS.OpaqueValue, IsImplicit);
-  if (!IsImplicit)
-    Coroutine->CoroutineStmts.push_back(Res);
+
   return Res;
 }
 
@@ -597,7 +597,6 @@
 
   if (E->getType()->isDependentType()) {
     Expr *Res = new (Context) CoyieldExpr(Loc, Context.DependentTy, E);
-    Coroutine->CoroutineStmts.push_back(Res);
     return Res;
   }
 
@@ -614,7 +613,7 @@
 
   Expr *Res = new (Context) CoyieldExpr(Loc, E, RSS.Results[0], RSS.Results[1],
                                         RSS.Results[2], RSS.OpaqueValue);
-  Coroutine->CoroutineStmts.push_back(Res);
+
   return Res;
 }
 
@@ -628,7 +627,7 @@
 
 StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E,
                                    bool IsImplicit) {
-  auto *FSI = checkCoroutineContext(*this, Loc, "co_return");
+  auto *FSI = checkCoroutineContext(*this, Loc, "co_return", IsImplicit);
   if (!FSI)
     return StmtError();
 
@@ -656,8 +655,6 @@
   Expr *PCE = ActOnFinishFullExpr(PC.get()).get();
 
   Stmt *Res = new (Context) CoreturnStmt(Loc, E, PCE, IsImplicit);
-  if (!IsImplicit)
-    FSI->CoroutineStmts.push_back(Res);
   return Res;
 }
 
@@ -774,15 +771,11 @@
   // Coroutines [stmt.return]p1:
   //   A return statement shall not appear in a coroutine.
   if (Fn->FirstReturnLoc.isValid()) {
+    assert(Fn->FirstCoroutineStmtLoc.isValid() &&
+                   "first coroutine location not set");
     Diag(Fn->FirstReturnLoc, diag::err_return_in_coroutine);
-    // FIXME: Every Coroutine statement may be invalid and therefore not added
-    // to CoroutineStmts. Find another way to provide location information.
-    if (!Fn->CoroutineStmts.empty()) {
-      auto *First = Fn->CoroutineStmts[0];
-      Diag(First->getLocStart(), diag::note_declared_coroutine_here)
-          << (isa<CoawaitExpr>(First) ? "co_await" :
-              isa<CoyieldExpr>(First) ? "co_yield" : "co_return");
-    }
+    Diag(Fn->FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
+            << Fn->getFirstCoroutineStmtKeyword();
   }
   SubStmtBuilder Builder(*this, *FD, *Fn, Body);
   if (Builder.isInvalid())