Fri, 18 Feb 2011 15:55:20 -0800
7020047: Project Coin: generate null-check around try-with-resources close call
Reviewed-by: jjg
1.1 --- a/src/share/classes/com/sun/tools/javac/comp/Lower.java Fri Feb 18 16:17:44 2011 +0000 1.2 +++ b/src/share/classes/com/sun/tools/javac/comp/Lower.java Fri Feb 18 15:55:20 2011 -0800 1.3 @@ -1425,25 +1425,55 @@ 1.4 } 1.5 } 1.6 1.7 - /** Optionally replace a try statement with an automatic resource 1.8 - * management (ARM) block. 1.9 + /** 1.10 + * Optionally replace a try statement with the desugaring of a 1.11 + * try-with-resources statement. The canonical desugaring of 1.12 + * 1.13 + * try ResourceSpecification 1.14 + * Block 1.15 + * 1.16 + * is 1.17 + * 1.18 + * { 1.19 + * final VariableModifiers_minus_final R #resource = Expression; 1.20 + * Throwable #primaryException = null; 1.21 + * 1.22 + * try ResourceSpecificationtail 1.23 + * Block 1.24 + * catch (Throwable #t) { 1.25 + * #primaryException = t; 1.26 + * throw #t; 1.27 + * } finally { 1.28 + * if (#resource != null) { 1.29 + * if (#primaryException != null) { 1.30 + * try { 1.31 + * #resource.close(); 1.32 + * } catch(Throwable #suppressedException) { 1.33 + * #primaryException.addSuppressed(#suppressedException); 1.34 + * } 1.35 + * } else { 1.36 + * #resource.close(); 1.37 + * } 1.38 + * } 1.39 + * } 1.40 + * 1.41 * @param tree The try statement to inspect. 1.42 - * @return An ARM block, or the original try block if there are no 1.43 - * resources to manage. 1.44 + * @return A a desugared try-with-resources tree, or the original 1.45 + * try block if there are no resources to manage. 1.46 */ 1.47 - JCTree makeArmTry(JCTry tree) { 1.48 + JCTree makeTwrTry(JCTry tree) { 1.49 make_at(tree.pos()); 1.50 twrVars = twrVars.dup(); 1.51 - JCBlock armBlock = makeArmBlock(tree.resources, tree.body, 0); 1.52 + JCBlock twrBlock = makeTwrBlock(tree.resources, tree.body, 0); 1.53 if (tree.catchers.isEmpty() && tree.finalizer == null) 1.54 - result = translate(armBlock); 1.55 + result = translate(twrBlock); 1.56 else 1.57 - result = translate(make.Try(armBlock, tree.catchers, tree.finalizer)); 1.58 + result = translate(make.Try(twrBlock, tree.catchers, tree.finalizer)); 1.59 twrVars = twrVars.leave(); 1.60 return result; 1.61 } 1.62 1.63 - private JCBlock makeArmBlock(List<JCTree> resources, JCBlock block, int depth) { 1.64 + private JCBlock makeTwrBlock(List<JCTree> resources, JCBlock block, int depth) { 1.65 if (resources.isEmpty()) 1.66 return block; 1.67 1.68 @@ -1497,16 +1527,16 @@ 1.69 1.70 int oldPos = make.pos; 1.71 make.at(TreeInfo.endPos(block)); 1.72 - JCBlock finallyClause = makeArmFinallyClause(primaryException, expr); 1.73 + JCBlock finallyClause = makeTwrFinallyClause(primaryException, expr); 1.74 make.at(oldPos); 1.75 - JCTry outerTry = make.Try(makeArmBlock(resources.tail, block, depth + 1), 1.76 + JCTry outerTry = make.Try(makeTwrBlock(resources.tail, block, depth + 1), 1.77 List.<JCCatch>of(catchClause), 1.78 finallyClause); 1.79 stats.add(outerTry); 1.80 return make.Block(0L, stats.toList()); 1.81 } 1.82 1.83 - private JCBlock makeArmFinallyClause(Symbol primaryException, JCExpression resource) { 1.84 + private JCBlock makeTwrFinallyClause(Symbol primaryException, JCExpression resource) { 1.85 // primaryException.addSuppressed(catchException); 1.86 VarSymbol catchException = 1.87 new VarSymbol(0, make.paramName(2), 1.88 @@ -1525,22 +1555,30 @@ 1.89 List<JCCatch> catchClauses = List.<JCCatch>of(make.Catch(catchExceptionDecl, catchBlock)); 1.90 JCTry tryTree = make.Try(tryBlock, catchClauses, null); 1.91 1.92 - // if (resource != null) resourceClose; 1.93 - JCExpression nullCheck = makeBinary(JCTree.NE, 1.94 - make.Ident(primaryException), 1.95 - makeNull()); 1.96 - JCIf closeIfStatement = make.If(nullCheck, 1.97 + // if (primaryException != null) {try...} else resourceClose; 1.98 + JCIf closeIfStatement = make.If(makeNonNullCheck(make.Ident(primaryException)), 1.99 tryTree, 1.100 makeResourceCloseInvocation(resource)); 1.101 - return make.Block(0L, List.<JCStatement>of(closeIfStatement)); 1.102 + 1.103 + // if (#resource != null) { if (primaryException ... } 1.104 + return make.Block(0L, 1.105 + List.<JCStatement>of(make.If(makeNonNullCheck(resource), 1.106 + closeIfStatement, 1.107 + null))); 1.108 } 1.109 1.110 private JCStatement makeResourceCloseInvocation(JCExpression resource) { 1.111 // create resource.close() method invocation 1.112 - JCExpression resourceClose = makeCall(resource, names.close, List.<JCExpression>nil()); 1.113 + JCExpression resourceClose = makeCall(resource, 1.114 + names.close, 1.115 + List.<JCExpression>nil()); 1.116 return make.Exec(resourceClose); 1.117 } 1.118 1.119 + private JCExpression makeNonNullCheck(JCExpression expression) { 1.120 + return makeBinary(JCTree.NE, expression, makeNull()); 1.121 + } 1.122 + 1.123 /** Construct a tree that represents the outer instance 1.124 * <C.this>. Never pick the current `this'. 1.125 * @param pos The source code position to be used for the tree. 1.126 @@ -3573,7 +3611,7 @@ 1.127 if (tree.resources.isEmpty()) { 1.128 super.visitTry(tree); 1.129 } else { 1.130 - result = makeArmTry(tree); 1.131 + result = makeTwrTry(tree); 1.132 } 1.133 } 1.134
2.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 2.2 +++ b/test/tools/javac/TryWithResources/TwrNullTests.java Fri Feb 18 15:55:20 2011 -0800 2.3 @@ -0,0 +1,72 @@ 2.4 +/* 2.5 + * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved. 2.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 2.7 + * 2.8 + * This code is free software; you can redistribute it and/or modify it 2.9 + * under the terms of the GNU General Public License version 2 only, as 2.10 + * published by the Free Software Foundation. 2.11 + * 2.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 2.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 2.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 2.15 + * version 2 for more details (a copy is included in the LICENSE file that 2.16 + * accompanied this code). 2.17 + * 2.18 + * You should have received a copy of the GNU General Public License version 2.19 + * 2 along with this work; if not, write to the Free Software Foundation, 2.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 2.21 + * 2.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 2.23 + * or visit www.oracle.com if you need additional information or have any 2.24 + * questions. 2.25 + */ 2.26 + 2.27 +/* 2.28 + * @test 2.29 + * @bug 7020047 2.30 + * @summary Test null handling of try-with-resources statement 2.31 + */ 2.32 + 2.33 +public class TwrNullTests { 2.34 + /* 2.35 + * Each try-with-resources statement generates two calls to the 2.36 + * close method for each resource: one for when there is a primary 2.37 + * exception present and the second for when a primary exception 2.38 + * is absent. The null handling of both cases needs to be 2.39 + * checked. 2.40 + */ 2.41 + public static void main(String... args) { 2.42 + testNormalCompletion(); 2.43 + testNoSuppression(); 2.44 + } 2.45 + 2.46 + /* 2.47 + * Verify empty try-with-resources on a null resource completes 2.48 + * normally; no NPE from the generated close call. 2.49 + */ 2.50 + private static void testNormalCompletion() { 2.51 + try(AutoCloseable resource = null) { 2.52 + return; // Nothing to see here, move along. 2.53 + } catch (Exception e) { 2.54 + throw new AssertionError("Should not be reached", e); 2.55 + } 2.56 + } 2.57 + 2.58 + /* 2.59 + * Verify that a NPE on a null resource is <em>not</em> added as a 2.60 + * suppressed exception to an exception from try block. 2.61 + */ 2.62 + private static void testNoSuppression() { 2.63 + try(AutoCloseable resource = null) { 2.64 + throw new java.io.IOException(); 2.65 + } catch(java.io.IOException ioe) { 2.66 + Throwable[] suppressed = ioe.getSuppressed(); 2.67 + if (suppressed.length != 0) { 2.68 + throw new AssertionError("Non-empty suppressed exceptions", 2.69 + ioe); 2.70 + } 2.71 + } catch (Exception e) { 2.72 + throw new AssertionError("Should not be reached", e); 2.73 + } 2.74 + } 2.75 +}