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 [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
