nridge added inline comments.

================
Comment at: clang/lib/Sema/SemaChecking.cpp:15186
+      if (ChildExpr == CSE->getOperand())
+        // Do not recurse over a CoroutineSuspendExpr's operand.
+        // The operand is also a subexpression of getCommonExpr(), and
----------------
bruno wrote:
> ChuanqiXu wrote:
> > nridge wrote:
> > > Out of curiosity, since `getCommonExpr()` is itself a subexpression of 
> > > `getReadyExpr()`, `getSuspendExpr()`, and `getResumeExpr()` (which are 
> > > also children of the `CoroutineSuspendExpr`), shouldn't `getCommonExpr()` 
> > > be skipped for the same reason?
> > >  since getCommonExpr() is itself a subexpression of getReadyExpr(), 
> > > getSuspendExpr(), and getResumeExpr()
> > 
> > This looks not true. For this example:
> > 
> > ```
> > co_await foo();
> > ```
> > 
> > `foo()` is the Operand. And the Common is `auto __tmp__ = operator 
> > co_await(foo())`. ReadyExpr should be `__tmp__.await_ready();`. SuspendExpr 
> > should be `__tmp__.await_suspend();` and ResumeExpr should be 
> > `__tmp__.await_resume();`.
> > 
> > So the method here looks good to me.
> 
> > `foo()` is the Operand. And the Common is `auto __tmp__ = operator 
> > co_await(foo())`. ReadyExpr should be `__tmp__.await_ready();`. SuspendExpr 
> > should be `__tmp__.await_suspend();` and ResumeExpr should be 
> > `__tmp__.await_resume();`.
> 
> Exactly!
> 
> 
Thank you for the explanation! I overlooked the fact that 
ReadyExpr/SuspendExpr/ResumeExpr use an 
[OpaqueValueExpr](https://searchfox.org/llvm/rev/4b051b4248bb6f9971dd1cf87fe311ebe9be917e/clang/lib/Sema/SemaCoroutine.cpp#378-379)
 wrapping CommonExpr rather than using CommonExpr directly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142077/new/

https://reviews.llvm.org/D142077

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to