================
@@ -174,6 +174,66 @@ static bool StmtCanThrow(const Stmt *S) {
return false;
}
+// Check if this suspend should be calling `await_suspend_destroy`
+static bool useCoroAwaitSuspendDestroy(const CoroutineSuspendExpr &S) {
+ // This can only be an `await_suspend_destroy` suspend expression if it
+ // returns void -- `buildCoawaitCalls` in `SemaCoroutine.cpp` asserts this.
+ // Moreover, when `await_suspend` returns a handle, the outermost method call
+ // is `.address()` -- making it harder to get the actual class or method.
+ if (S.getSuspendReturnType() !=
+ CoroutineSuspendExpr::SuspendReturnType::SuspendVoid) {
+ return false;
+ }
+
+ // `CGCoroutine.cpp` & `SemaCoroutine.cpp` must agree on whether this suspend
+ // expression uses `[[clang::coro_await_suspend_destroy]]`.
+ //
+ // Any mismatch is a serious bug -- we would either double-free, or fail to
+ // destroy the promise type. For this reason, we make our decision based on
+ // the method name, and fatal outside of the happy path -- including on
+ // failure to find a method name.
+ //
+ // As a debug-only check we also try to detect the `AwaiterClass`. This is
+ // secondary, because detection of the awaiter type can be silently broken
by
+ // small `buildCoawaitCalls` AST changes.
+ StringRef SuspendMethodName; // Primary
+ CXXRecordDecl *AwaiterClass = nullptr; // Debug-only, best-effort
+ if (auto *SuspendCall =
+ dyn_cast<CallExpr>(S.getSuspendExpr()->IgnoreImplicit())) {
+ if (auto *SuspendMember = dyn_cast<MemberExpr>(SuspendCall->getCallee())) {
+ if (auto *BaseExpr = SuspendMember->getBase()) {
+ // `IgnoreImplicitAsWritten` is critical since `await_suspend...` can
be
+ // invoked on the base of the actual awaiter, and the base need not
have
+ // the attribute. In such cases, the AST will show the true awaiter
+ // being upcast to the base.
+ AwaiterClass = BaseExpr->IgnoreImplicitAsWritten()
+ ->getType()
+ ->getAsCXXRecordDecl();
+ }
+ if (auto *SuspendMethod =
+ dyn_cast<CXXMethodDecl>(SuspendMember->getMemberDecl())) {
+ SuspendMethodName = SuspendMethod->getName();
+ }
+ }
+ }
+ if (SuspendMethodName == "await_suspend_destroy") {
+ assert(!AwaiterClass ||
+ AwaiterClass->hasAttr<CoroAwaitSuspendDestroyAttr>());
+ return true;
+ } else if (SuspendMethodName == "await_suspend") {
+ assert(!AwaiterClass ||
+ !AwaiterClass->hasAttr<CoroAwaitSuspendDestroyAttr>());
+ return false;
+ } else {
+ llvm::report_fatal_error(
+ "Wrong method in [[clang::coro_await_suspend_destroy]] check: "
+ "expected 'await_suspend' or 'await_suspend_destroy', but got '" +
+ SuspendMethodName + "'");
+ }
----------------
snarkmaster wrote:
Yes, the "awaiter" and "method name" checks are redundant, but I did it this
way for a (good?) reason.
Initially, I just looked for the attribute on the awaiter checks, and I had a
nasty bug described in a comment higher in that function. Worse yet, I only
found it by accident because my test **happened** to have a base awaiter that
didn't have the attribute, and a derived one that did.
Since any class can be an awaiter, it's hard to be sure if we're looking on the
same CXXRecord that Sema examined.
For that reason, I made the method name check primary. There are only two valid
values here, and I can emit a fatal if I don't see either. This makes it clear
that Sema alone is responsible for checking & interpreting the attribute.
The `AwaiterClass` attribute check is secondary, and debug-only. If you prefer,
I'm happy to delete that one.
https://github.com/llvm/llvm-project/pull/152623
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
