[PATCH] D31487: [coroutines] Fix rebuilding of implicit and dependent coroutine statements.

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov added inline comments.


================
Comment at: lib/Sema/CoroutineBuilder.h:1
+//===----- CoroutineBuilder.h - Coroutine Semantic checking -----*- C++ -*-===//
+//
----------------
I would name the file to match the class it declares, i.e CoroutineStmtBuilder.h, since it is the only class defined in this header.



================
Comment at: lib/Sema/CoroutineBuilder.h:9
+//
+//  This file implements a semantic tree transformation that takes a given
+//  AST and rebuilds it, possibly transforming some nodes in the process.
----------------
I do not think that this description matches the content of the file.


================
Comment at: lib/Sema/CoroutineBuilder.h:37
+public:
+  CoroutineStmtBuilder(Sema &S, FunctionDecl &FD, sema::FunctionScopeInfo &Fn,
+                       Stmt *Body)
----------------
I would move the constructor and two build methods implementations into SemaCoroutine.cpp.


================
Comment at: lib/Sema/TreeTransform.h:6908
+  } else {
+    if (S->getFallthroughHandler()) {
+      StmtResult Res = getDerived().TransformStmt(S->getFallthroughHandler());
----------------
   if(Expr *OnFallthrough = ...) {
      ...getDerived().TransformStmt(OnFallthrough);


================
Comment at: lib/Sema/TreeTransform.h:6915
 
-  // Transform any additional statements we may have already built
-  if (S->getAllocate() && S->getDeallocate()) {
+    if (S->getExceptionHandler()) {
+      StmtResult Res = getDerived().TransformStmt(S->getExceptionHandler());
----------------
   if(Expr *OnException = ...) {
      ...getDerived().TransformStmt(OnException);


================
Comment at: lib/Sema/TreeTransform.h:6922
+
+    if (S->getReturnStmtOnAllocFailure()) {
+      StmtResult Res =
----------------
   if(Expr *OnAllocFailure = ...) {
      ...getDerived().TransformStmt(OnAllocFailure);


https://reviews.llvm.org/D31487