ChuanqiXu accepted this revision.
ChuanqiXu added a comment.
This revision is now accepted and ready to land.

Since the patch itself is  good and not large. Let me handle the trivial 
refactoring later.

Comment at: clang/lib/CodeGen/CGCoroutine.cpp:724-730
+      Stmt *BodyStmt = S.getBody();
+      CompoundStmt *Body = dyn_cast<CompoundStmt>(BodyStmt);
+      if (Body == nullptr) {
+        Body =
+            CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+                                 SourceLocation(), SourceLocation());
+      }
MatzeB wrote:
> ChuanqiXu wrote:
> > MatzeB wrote:
> > > ChuanqiXu wrote:
> > > > Can we try to move the logic to `CoroutineStmtBuilder`? That makes me 
> > > > feel better. And it will be helpful to add a comment to tell that we're 
> > > > handling the case the function body is function-try-block.
> > > I'll add a detailed comment. But would you be fine leaving the statements 
> > > here as-is? The logic only makes sense in the context of using the `Body` 
> > > to create a `CXXTryStmt` below (it's really an effect of `CXXTryStmt` 
> > > only accepting CompountStmt operands).
> > It looks like you didn't address the comments. Would you like to address 
> > it? I don't mind to address it later myself.
> Did you mean to create a new function named `CoroutineStmtBuilder` like I did 
> now?
> Did you mean to create a new function named CoroutineStmtBuilder like I did 
> now?

No, I mean we should construct this in Sema.

> Putting an assert here feels unnecessary and may be in the way if in the 
> future we ever allow other types of single-statement function bodies.

Personally I prefer the more precise style.

  rG LLVM Github Monorepo


cfe-commits mailing list

Reply via email to