[PATCH] D90990: [Coroutine][Sema] Cleanup temporaries as early as possible

Bruno Cardoso Lopes via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 10 00:48:20 PST 2020


bruno added a comment.

Very nice explanation, thanks for improving this!

Can you also add a AST dump test? The idea is to test for the presence of `ExprWithCleanups`s. Something along the lines of `clang/test/AST/coroutine-source-location-crash.cpp` ignoring the serialization part should be good.



================
Comment at: clang/lib/Sema/SemaCoroutine.cpp:475
+  if (!AwaitSuspend)
+    return Calls;
   if (!AwaitSuspend->getType()->isDependentType()) {
----------------
In case `AwaitSuspend` is null, is there any need to set `Calls.IsInvalid` as well?


================
Comment at: clang/lib/Sema/SemaCoroutine.cpp:490
+      // ExprWithCleanups is wrapped within maybeTailCall() prior to the resume
+      // call.
       Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
----------------
Is there already a test covering this tailcall case? It'd be nice to have one 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90990



More information about the cfe-commits mailing list