RFR JDK-8194978: Javac produces dead code for try-with-resource

[Jan Lahoda]

Hi,

Bug: https://bugs.openjdk.java.net/browse/JDK-8194978

The issue here is that the bytecode generated for try-with-resources 
produces some unreachable instructions, and may be unnecessarily big.

Consider this code:
---
         try (InputStream in = new FileInputStream(str)) {
             int read = in.read();
             if (read == (-1)) return ;
             System.err.println(read);
         }
---

javac desugars it to this:
---
         {
             final InputStream in = new FileInputStream(str);
             /*synthetic*/ Throwable primaryException0$ = null;
             try {
                 int read = in.read();
                 if (read == (-1)) return;
                 System.err.println(read);
             } catch (/*synthetic*/ final Throwable t$) {
                 primaryException0$ = t$;
                 throw t$;
             } finally {
                 if (primaryException0$ != null) try {
                     in.close();
                 } catch (/*synthetic*/ Throwable x2) {
                     primaryException0$.addSuppressed(x2);
                 } else in.close();
             }
         }
---

Which seems fine, but when the try-finally construct is written into 
bytecode, the "finally" section is duplicated into all (non-exceptional) 
exit points from the try body (it would also be copied to non-exception 
exit points from all catches, but there are no such exit points in this 
case). A rough idea on how the bytecode looks like is below - please 
note that some try-catches don't embed nicely with the rest of the code, 
so are denoted in comments only.

---
         {
             final InputStream in = new FileInputStream(str);
             /*synthetic*/ Throwable primaryException0$ = null;
             // try
                 int read = in.read();
                 if (read == (-1)) {
             // catch Throwable: goto CATCH
             // catch any: goto FINALLY [1]
                     //start injected finally
                     if (primaryException0$ != null) try { // [2]
                         in.close();
                     } catch (/*synthetic*/ Throwable x2) {
                         primaryException0$.addSuppressed(x2);
                     } else in.close();
                     //end injected finally

                     return;
                 }

             // try
                 System.err.println(read);
             // catch Throwable: goto CATCH
             // catch any: goto FINALLY [1]

             //start injected finally
             if (primaryException0$ != null) try { // [2]
                 in.close();
             } catch (/*synthetic*/ Throwable x2) {
                 primaryException0$.addSuppressed(x2);
             } else in.close();
             //end injected finally

             //goto END

             CATCH: // catch (/*synthetic*/ final Throwable t$)
                 // try
                 primaryException0$ = t$;
                 throw t$;
                 // catch any: goto FINALLY
             FINALLY:
                 if (primaryException0$ != null) try {// [3]
                     in.close();
                 } catch (/*synthetic*/ Throwable x2) {
                     primaryException0$.addSuppressed(x2);
                 } else in.close();
             END:
         }
---

A few comments about the code:
-[1]: this catches seem unnecessary: all Throwables have already been 
caught?
-[2]: at this place, it is clear that primaryException0$ is null, so the 
is-non-null section is not useful
-[3]: at this place, it is mostly clear that primaryException0$ is 
not-null, so the is-null section is not useful

So, it seems it might be possible to construct the code so that only the 
needed sections of the finally would be used at the appropriate places. 
I.e. the original try-with-resources would get desugared to something like:

---
         {
             final InputStream in = new FileInputStream(str);
             try {
                 int read = in.read();
                 if (read == (-1)) return;
                 System.err.println(read);
             } body-only-finally { //copied to exit points in try body
                                   //only, not used as a real finally
                 in.close();
             } catch (/*synthetic*/ final Throwable t$) {
                 try {
                     in.close();
                 } catch (/*synthetic*/ Throwable x2) {
                     t$.addSuppressed(x2);
                 }
                 throw t$;
             }
         }
---

Which would then get generated to bytecode as:
---
         {
             final InputStream in = new FileInputStream(str);
             //try
                 int read = in.read();
                 if (read == (-1)) {
             // catch Throwable: goto CATCH
                     in.close();
                     return;
                 }

             //try
                 System.err.println(read);
             // catch Throwable: goto CATCH

             in.close();

             //goto END

             CATCH: // catch (/*synthetic*/ final Throwable t$)
                 try {
                     in.close();
                 } catch (/*synthetic*/ Throwable x2) {
                     t$.addSuppressed(x2);
                 }
                 throw t$;
             END:
         }
---

The current desugaring is trying to pull out the finally code into a 
separate method when that appears to be worthwhile, but this proposed 
desugaring seems to be producing a better bytecode with less javac code.

Does this updated desugaring/transformation look safe?

Webrev: http://cr.openjdk.java.net/~jlahoda/8194978/webrev.00/

Thanks,
      Jan