7020047: Project Coin: generate null-check around try-with-resources close call

Reviewed-by: jjg

src/share/classes/com/sun/tools/javac/comp/Lower.java

@@ -1425,25 +1425,55 @@ public class Lower extends TreeTranslator {
         }
     }
 
-    /** Optionally replace a try statement with an automatic resource
-     *  management (ARM) block.
+    /**
+     * Optionally replace a try statement with the desugaring of a
+     * try-with-resources statement.  The canonical desugaring of
+     *
+     * try ResourceSpecification
+     *   Block
+     *
+     * is
+     *
+     * {
+     *   final VariableModifiers_minus_final R #resource = Expression;
+     *   Throwable #primaryException = null;
+     *
+     *   try ResourceSpecificationtail
+     *     Block
+     *   catch (Throwable #t) {
+     *     #primaryException = t;
+     *     throw #t;
+     *   } finally {
+     *     if (#resource != null) {
+     *       if (#primaryException != null) {
+     *         try {
+     *           #resource.close();
+     *         } catch(Throwable #suppressedException) {
+     *           #primaryException.addSuppressed(#suppressedException);
+     *         }
+     *       } else {
+     *         #resource.close();
+     *       }
+     *     }
+     *   }
+     *
      * @param tree  The try statement to inspect.
-     * @return      An ARM block, or the original try block if there are no
-     *              resources to manage.
+     * @return A a desugared try-with-resources tree, or the original
+     * try block if there are no resources to manage.
      */
-    JCTree makeArmTry(JCTry tree) {
+    JCTree makeTwrTry(JCTry tree) {
         make_at(tree.pos());
         twrVars = twrVars.dup();
-        JCBlock armBlock = makeArmBlock(tree.resources, tree.body, 0);
+        JCBlock twrBlock = makeTwrBlock(tree.resources, tree.body, 0);
         if (tree.catchers.isEmpty() && tree.finalizer == null)
-            result = translate(armBlock);
+            result = translate(twrBlock);
         else
-            result = translate(make.Try(armBlock, tree.catchers, tree.finalizer));
+            result = translate(make.Try(twrBlock, tree.catchers, tree.finalizer));
         twrVars = twrVars.leave();
         return result;
     }
 
-    private JCBlock makeArmBlock(List<JCTree> resources, JCBlock block, int depth) {
+    private JCBlock makeTwrBlock(List<JCTree> resources, JCBlock block, int depth) {
         if (resources.isEmpty())
             return block;
 
@@ -1497,16 +1527,16 @@ public class Lower extends TreeTranslator {
 
         int oldPos = make.pos;
         make.at(TreeInfo.endPos(block));
-        JCBlock finallyClause = makeArmFinallyClause(primaryException, expr);
+        JCBlock finallyClause = makeTwrFinallyClause(primaryException, expr);
         make.at(oldPos);
-        JCTry outerTry = make.Try(makeArmBlock(resources.tail, block, depth + 1),
+        JCTry outerTry = make.Try(makeTwrBlock(resources.tail, block, depth + 1),
                                   List.<JCCatch>of(catchClause),
                                   finallyClause);
         stats.add(outerTry);
         return make.Block(0L, stats.toList());
     }
 
-    private JCBlock makeArmFinallyClause(Symbol primaryException, JCExpression resource) {
+    private JCBlock makeTwrFinallyClause(Symbol primaryException, JCExpression resource) {
         // primaryException.addSuppressed(catchException);
         VarSymbol catchException =
             new VarSymbol(0, make.paramName(2),
@@ -1525,22 +1555,30 @@ public class Lower extends TreeTranslator {
         List<JCCatch> catchClauses = List.<JCCatch>of(make.Catch(catchExceptionDecl, catchBlock));
         JCTry tryTree = make.Try(tryBlock, catchClauses, null);
 
-        // if (resource != null) resourceClose;
-        JCExpression nullCheck = makeBinary(JCTree.NE,
-                                            make.Ident(primaryException),
-                                            makeNull());
-        JCIf closeIfStatement = make.If(nullCheck,
+        // if (primaryException != null) {try...} else resourceClose;
+        JCIf closeIfStatement = make.If(makeNonNullCheck(make.Ident(primaryException)),
                                         tryTree,
                                         makeResourceCloseInvocation(resource));
-        return make.Block(0L, List.<JCStatement>of(closeIfStatement));
+
+        // if (#resource != null) { if (primaryException ...  }
+        return make.Block(0L,
+                          List.<JCStatement>of(make.If(makeNonNullCheck(resource),
+                                                       closeIfStatement,
+                                                       null)));
     }
 
     private JCStatement makeResourceCloseInvocation(JCExpression resource) {
         // create resource.close() method invocation
-        JCExpression resourceClose = makeCall(resource, names.close, List.<JCExpression>nil());
+        JCExpression resourceClose = makeCall(resource,
+                                              names.close,
+                                              List.<JCExpression>nil());
         return make.Exec(resourceClose);
     }
 
+    private JCExpression makeNonNullCheck(JCExpression expression) {
+        return makeBinary(JCTree.NE, expression, makeNull());
+    }
+
     /** Construct a tree that represents the outer instance
      *  <C.this>. Never pick the current `this'.
      *  @param pos           The source code position to be used for the tree.
@@ -3573,7 +3611,7 @@ public class Lower extends TreeTranslator {
         if (tree.resources.isEmpty()) {
             super.visitTry(tree);
         } else {
-            result = makeArmTry(tree);
+            result = makeTwrTry(tree);
         }
     }
 

test/tools/javac/TryWithResources/TwrNullTests.java

+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 7020047
+ * @summary Test null handling of try-with-resources statement
+ */
+
+public class TwrNullTests {
+    /*
+     * Each try-with-resources statement generates two calls to the
+     * close method for each resource: one for when there is a primary
+     * exception present and the second for when a primary exception
+     * is absent.  The null handling of both cases needs to be
+     * checked.
+     */
+    public static void main(String... args) {
+        testNormalCompletion();
+        testNoSuppression();
+    }
+
+    /*
+     * Verify empty try-with-resources on a null resource completes
+     * normally; no NPE from the generated close call.
+     */
+    private static void testNormalCompletion() {
+        try(AutoCloseable resource = null) {
+            return; // Nothing to see here, move along.
+        } catch (Exception e) {
+            throw new AssertionError("Should not be reached", e);
+        }
+    }
+
+    /*
+     * Verify that a NPE on a null resource is <em>not</em> added as a
+     * suppressed exception to an exception from try block.
+     */
+    private static void testNoSuppression() {
+        try(AutoCloseable resource = null) {
+            throw new java.io.IOException();
+        } catch(java.io.IOException ioe) {
+            Throwable[] suppressed = ioe.getSuppressed();
+            if (suppressed.length != 0) {
+                throw new AssertionError("Non-empty suppressed exceptions",
+                                         ioe);
+            }
+        } catch (Exception e) {
+            throw new AssertionError("Should not be reached", e);
+        }
+    }
+}