[PATCH] D124642: [WIP] Add support for return from an SEH __finally block.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/lib/CodeGen/CGException.cpp:1833
+/// Find all local variable captures in the statement.
+struct ReturnStmtFinder : ConstStmtVisitor<ReturnStmtFinder> {
+  bool ContainsRetStmt = false;
----------------
rnk wrote:
> We have the option to generalize the naming here to include other kinds of jumps that exit the scope of the finally. I think it's worth implementing `__leave` eventually, which I think users have asked for at some point. I could go either way, your choice.
Probably better to rename once we actually implement that?  I'm intentionally not trying to detect other forms of control flow because EnterSEHTryStmt only knows how to return from the function.  If we're going to branch elsewhere, we need to return a list of the destinations, not just a boolean "there's a return statement".


================
Comment at: llvm/lib/CodeGen/AsmPrinter/WinException.cpp:631-634
+      // Mark up the destination of _local_unwind so it doesn't unwind
+      // too far.
+      //
+      // FIXME: Can this overlap with the EH_LABEL for an invoke?
----------------
rnk wrote:
> I don't understand what this is doing, but I should look at the tests.
I don't remember all the details of this, but as I noted in the commit message, it affects nesting `__try`/`__finally` in `__try`.  I think without this, the destination of the `_local_unwind` somehow ended up inheriting an unrelated SEH state.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2942
+      if (!isa<CatchSwitchInst>(EHPadBB->getTerminator())) {
+        report_fatal_error("localunwind doesn't point to catchswitch");
+      }
----------------
rnk wrote:
> Since this isn't verifier checked yet, is it possible to emit a diagnostic via the LLVMContext and select this instruction to nothing, or would that leave behind disconnected unreachable MBBs that break too many invariants later?
I can't find any other code trying to report diagnostics that way, but I guess I can look into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124642