Try/catch/finally builder

[Brian Goetz]

I'm still a little uncomfortable at the BiPredicate approach, for two 
reasons:

  - It's more "magic" than having the user explicitly say "unroll the 
finally here"
  - I'm not convinced the usability is better

You shared the following example:

```
         try {
             try {
                 System.out.println("INNER TRY START");
                 String s = args[0];
                 if (s.length() < 10) {
                     [0]
                     return -1;
                 }
                 System.out.println("INNER TRY END");
                 // Exit
                 // [1]
             } catch (RuntimeException e) {
                 System.out.println("INNER RUNTIME EXCEPTION");
                 // [2]
                 throw e;
             } finally {
                 System.out.println("INNER FINALLY");
             }
             System.out.println("OUTER TRY");
             // [3]
             // Exit
         } catch (RuntimeException e) {
             System.out.println("OUTER RUNTIME EXCEPTION");
             // [4]
             throw e;
         } finally {
             System.out.println("OUTER FINALLY");
         }
```

I must admit that at first look, I don't know where the finally blocks 
have to be inserted, but looking at the javap output, it looks like:

  - [0] -- inner and outer finally
  - [1] -- inner finally (then falls out)
  - [2] -- nothing (handled as stack unwinds by any handler)
  - [3] -- outer
  - [4] -- nothing (handled by any handler)

Plus a pair of "any" handlers, the first of which does the inner 
finally, the second does the outer, and the second is in scope for the 
first.  Whew!

There's several groups of questions here.

One is "what does the user know, and when do they know it."  A block 
like the above might not be generated with pure try-catch builder; it 
might be that the inner block comes from the class being adapted, and 
the outer block is being added by a transform.  So the user may not even 
know about the inner try-catch block!  In this case, we would have a 
problem at [0] only, where the code being adapted has already inlined 
the inner finally, but we would have to realize that prior to a return, 
we'd have to also unroll the inner finally.

Which exposes problem #1 -- it's not just branch instructions that the 
BiPredicate would have to examine, its also returns (unless you are 
treating these unconditionally as needing finally unrolling.)

Setting that aside, the real question is whether a branch target that is 
a branch target is internal to the try+catch blocks, or "outside".  
Which we would handle by examining the label.  But the usability of this 
is not so great, because the label may not have been bound yet, and so 
all the user can do is compare it for identity with the N labels known 
to be in the block.  That sounds like kind of a pain for the user to 
implement.

So here's another possible thought: what if BlockBuilder could dispense 
labels, as well as dispensing blocks, and could then keep track of 
containment?  So if you are branching to a label *outside this block 
builder* (including returning), we'd inline the finally at that point.  
(With nested block builders, we'd have to keep track of which BBs 
belonged to other BBs, but we already do this.)






On 8/24/2022 4:51 PM, Paul Sandoz wrote:
> Here’s an update:
>
> https://github.com/openjdk/jdk-sandbox/compare/classfile-api-branch...PaulSandoz:jdk-sandbox:try-catch-finally-builder?expand=1
> sealed interface CatchFinallyBuilder permits CatchFinallyBuilderImpl {
>      CatchFinallyBuilder catching(ClassDesc exceptionType,Consumer<BlockCodeBuilder> catchHandler);
>
>      default void finally_(Consumer<BlockCodeBuilder> finallyHandler) {
>          finally_(INLINE_FINALLY_ON_BREAK, finallyHandler);
>      }
>
>      void finally_(BiPredicate<BlockCodeBuilder,BranchInstruction> inlineFinallyTest,
>                    Consumer<BlockCodeBuilder> finallyHandler);
>
>      BiPredicate<BlockCodeBuilder,BranchInstruction> INLINE_FINALLY_ON_BREAK =
>              (b, i) -> b.breakLabel() == i.target();
> }
> The terminal finally_ builder method accepts an optional predicate.
>
> Paul.
>
>> On Aug 24, 2022, at 10:57 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>>
>> Or, alternatively, the user can provide an optional 
>> BiPredicate<BlockCodeBuilder, BranchInstruction>, which is called for 
>> every branch instruction of a try or catch block, and returns true 
>> if the branches target is known to exit the block and therefore the 
>> finally blocks need to be inlined before it. The default 
>> implementation is specified to behave as it does in the prototype, 
>> and can be composed.
>>
>> That seems to give the developer the control they need without 
>> explicitly emitting finally blocks, which could get more complex when 
>> nesting trying builders.
>>
>> Paul.
>>
>>> On Aug 23, 2022, at 6:12 PM, Brian Goetz <brian.goetz at oracle.com> wrote:
>>>
>>> Maybe the TCB should have an “emit finally” method that causes the 
>>> finally lambda to be called at that point?
>>>
>>> Sent from my MacBook Wheel
>>>
>>>> On Aug 23, 2022, at 6:39 PM, Paul Sandoz <paul.sandoz at oracle.com> 
>>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Here’s a prototype of a try/catch/finally builder (that should 
>>>> replace the current approach):
>>>>
>>>> https://github.com/openjdk/jdk-sandbox/compare/classfile-api-branch...PaulSandoz:jdk-sandbox:try-catch-finally-builder?expand=1
>>>>
>>>> So far it all seems to work and produces correct code for nested 
>>>> building, generating similar code as the source compiler produces.
>>>>
>>>> I need to do more thorough testing and commit unit tests.
>>>>
>>>> —
>>>>
>>>> One challenge is determining when a block exits. A branching 
>>>> instruction that exits the block should result in inlining of the 
>>>> finally code before the instruction, but it's hard to 
>>>> precisely determine if the branch target is within or outside of 
>>>> the block. At the moment I hard code to checking if the target is 
>>>> the break label of the block, otherwise it is assumed the 
>>>> branch does not exit the block. I don’t think this is generally 
>>>> decidable unless all block instructions are first buffered.
>>>>
>>>> Paul.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20220825/cec8fe30/attachment-0001.htm>