[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

Shoaib Meenai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 1 18:32:16 PDT 2018


smeenai added a comment.

In https://reviews.llvm.org/D47564#1118424, @rjmccall wrote:

> In https://reviews.llvm.org/D47564#1118423, @smeenai wrote:
>
> > In https://reviews.llvm.org/D47564#1118381, @rjmccall wrote:
> >
> > > That's an interesting idea.  I don't see any particular reason not to do it this way if you're willing to accept that it's never going to support the full control-flow possibilities of @finally.  You will need to add JumpDiagnostics logic to prevent branches out of the block, and I don't know how this will interact with attempts to throw an exception out.
> >
> >
> > There's already some logic in CapturedStmt to prevent branches out of the block:
> >
> > - Attempting to return will produce "cannot return from Objective-C @finally statement"
> > - Attempting to goto out of the block will result in "use of undeclared label", which is a bad diagnostic (and should be improved), but it does error
>
>
> Alright, that makes sense.
>
> > It should be possible to add support for returns, at least; the idea we'd discussed with @rnk was setting a flag in the captured function to indicate a return having been executed, and then reading that flag outside the captured function and acting on it appropriately. gotos would be more complicated, but I think we could make them work if we really wanted to.
> > 
> > Throwing an exception out should just work, I think; the outlined function will just participate normally in exception handling. Did you have a specific case you were thinking of?
>
> No, it was just a general question.  Have you gotten this to a point where it's testable?


Yup, it's been working fine in my local testing. There's one more patch that I need to put up, which actually handles doing proper codegen for @try/@catch/@finally; I'm working on cleaning that up right now. The other piece of the puzzle is https://reviews.llvm.org/D47233, which emits the proper typeinfo required for this.


Repository:
  rC Clang

https://reviews.llvm.org/D47564





More information about the cfe-commits mailing list