### Eclipse Workspace Patch 1.0 #P org.eclipse.jdt.core Index: compiler/org/eclipse/jdt/internal/compiler/ast/ASTNode.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ASTNode.java,v retrieving revision 1.100 diff -u -r1.100 ASTNode.java --- compiler/org/eclipse/jdt/internal/compiler/ast/ASTNode.java 22 Jun 2009 14:00:52 -0000 1.100 +++ compiler/org/eclipse/jdt/internal/compiler/ast/ASTNode.java 10 Feb 2010 15:50:28 -0000 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2009 IBM Corporation and others. + * Copyright (c) 2000, 2010 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -30,8 +30,8 @@ public final static int Bit5 = 0x10; // value for return (expression) | has all method bodies (unit) | supertype ref (type ref) | resolved (field decl) public final static int Bit6 = 0x20; // depth (name ref, msg) | ignore need cast check (cast expression) | error in signature (method declaration/ initializer) | is recovered (annotation reference) public final static int Bit7 = 0x40; // depth (name ref, msg) | operator (operator) | need runtime checkcast (cast expression) | label used (labelStatement) | needFreeReturn (AbstractMethodDeclaration) - public final static int Bit8 = 0x80; // depth (name ref, msg) | operator (operator) | unsafe cast (cast expression) | is default constructor (constructor declaration) - public final static int Bit9 = 0x100; // depth (name ref, msg) | operator (operator) | is local type (type decl) + public final static int Bit8 = 0x80; // depth (name ref, msg) | operator (operator) | unsafe cast (cast expression) | is default constructor (constructor declaration) | isElseStatementUnreachable (if statement) + public final static int Bit9 = 0x100; // depth (name ref, msg) | operator (operator) | is local type (type decl) | isThenStatementUnreachable (if statement) public final static int Bit10= 0x200; // depth (name ref, msg) | operator (operator) | is anonymous type (type decl) public final static int Bit11 = 0x400; // depth (name ref, msg) | operator (operator) | is member type (type decl) public final static int Bit12 = 0x800; // depth (name ref, msg) | operator (operator) | has abstract methods (type decl) @@ -194,6 +194,8 @@ // for if statement public static final int IsElseIfStatement = Bit30; public static final int ThenExit = Bit31; + public static final int IsElseStatementUnreachable = Bit8; + public static final int IsThenStatementUnreachable = Bit9; // for type reference public static final int IsSuperType = Bit5; Index: compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java,v retrieving revision 1.77 diff -u -r1.77 EqualExpression.java --- compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java 21 Jan 2010 16:48:36 -0000 1.77 +++ compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java 10 Feb 2010 15:50:29 -0000 @@ -59,12 +59,6 @@ } break; } - // set the optimize constant to optimize code gen - if ((initsWhenTrue.tagBits & FlowInfo.UNREACHABLE) != 0) { - this.optimizedBooleanConstant = BooleanConstant.fromValue(false); - } else if ((initsWhenFalse.tagBits & FlowInfo.UNREACHABLE) != 0) { - this.optimizedBooleanConstant = BooleanConstant.fromValue(true); - } // we do not impact enclosing try context because this kind of protection // does not preclude the variable from being null in an enclosing scope } Index: compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java,v retrieving revision 1.66 diff -u -r1.66 IfStatement.java --- compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java 21 Jan 2010 16:48:36 -0000 1.66 +++ compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java 10 Feb 2010 15:50:29 -0000 @@ -30,7 +30,7 @@ int elseInitStateIndex = -1; int mergedInitStateIndex = -1; -public IfStatement(Expression condition, Statement thenStatement, int sourceStart, int sourceEnd) { +public IfStatement(Expression condition, Statement thenStatement, int sourceStart, int sourceEnd) { this.condition = condition; this.thenStatement = thenStatement; // remember useful empty statement @@ -69,12 +69,27 @@ if (isConditionOptimizedTrue) { elseFlowInfo.setReachMode(FlowInfo.UNREACHABLE); } + if (((flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0) && + ((thenFlowInfo.tagBits & FlowInfo.UNREACHABLE) != 0)) { + // Mark then block as unreachable + // No need if the whole if-else construct itself lies in unreachable code + this.bits |= ASTNode.IsThenStatementUnreachable; + } else if (((flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0) && + ((elseFlowInfo.tagBits & FlowInfo.UNREACHABLE) != 0)) { + // Mark else block as unreachable + // No need if the whole if-else construct itself lies in unreachable code + this.bits |= ASTNode.IsElseStatementUnreachable; + } if (this.thenStatement != null) { // Save info for code gen this.thenInitStateIndex = currentScope.methodScope().recordInitializationStates(thenFlowInfo); - if (isConditionOptimizedFalse) { + if (isConditionOptimizedFalse || ((this.bits & ASTNode.IsThenStatementUnreachable) == ASTNode.IsThenStatementUnreachable)) { if (!isKnowDeadCodePattern(this.condition) || currentScope.compilerOptions().reportDeadCodeInTrivialIfStatement) { this.thenStatement.complainIfUnreachable(thenFlowInfo, currentScope, initialComplaintLevel); + } else { + // its a known coding pattern which should be tolerated by dead code analysis + // according to isKnowDeadCodePattern() + this.bits &= ~ASTNode.IsThenStatementUnreachable; } } thenFlowInfo = this.thenStatement.analyseCode(currentScope, flowContext, thenFlowInfo); @@ -86,28 +101,33 @@ // process the ELSE part if (this.elseStatement != null) { - // signal else clause unnecessarily nested, tolerate else-if code pattern - if (thenFlowInfo == FlowInfo.DEAD_END - && (this.bits & IsElseIfStatement) == 0 // else of an else-if - && !(this.elseStatement instanceof IfStatement)) { - currentScope.problemReporter().unnecessaryElse(this.elseStatement); - } + // signal else clause unnecessarily nested, tolerate else-if code pattern + if (thenFlowInfo == FlowInfo.DEAD_END + && (this.bits & IsElseIfStatement) == 0 // else of an else-if + && !(this.elseStatement instanceof IfStatement)) { + currentScope.problemReporter().unnecessaryElse(this.elseStatement); + } // Save info for code gen this.elseInitStateIndex = currentScope.methodScope().recordInitializationStates(elseFlowInfo); - if (isConditionOptimizedTrue) { + if (isConditionOptimizedTrue || ((this.bits & ASTNode.IsElseStatementUnreachable) != 0)) { if (!isKnowDeadCodePattern(this.condition) || currentScope.compilerOptions().reportDeadCodeInTrivialIfStatement) { this.elseStatement.complainIfUnreachable(elseFlowInfo, currentScope, initialComplaintLevel); + } else { + // its a known coding pattern which should be tolerated by dead code analysis + // according to isKnowDeadCodePattern() + this.bits &= ~ASTNode.IsElseStatementUnreachable; } } elseFlowInfo = this.elseStatement.analyseCode(currentScope, flowContext, elseFlowInfo); } // merge THEN & ELSE initializations - FlowInfo mergedInfo = FlowInfo.mergedOptimizedBranches( + FlowInfo mergedInfo = FlowInfo.mergedOptimizedBranchesIfElse( thenFlowInfo, isConditionOptimizedTrue, elseFlowInfo, isConditionOptimizedFalse, - true /*if(true){ return; } fake-reachable(); */); + true /*if(true){ return; } fake-reachable(); */, + flowInfo); this.mergedInitStateIndex = currentScope.methodScope().recordInitializationStates(mergedInfo); return mergedInfo; } @@ -139,7 +159,8 @@ if (hasThenPart) { BranchLabel falseLabel = null; // generate boolean condition only if needed - if (cst != Constant.NotAConstant && cst.booleanValue() == true) { + if (((this.bits & ASTNode.IsElseStatementUnreachable) == ASTNode.IsElseStatementUnreachable) || + (cst != Constant.NotAConstant && cst.booleanValue() == true)) { // No need to generate if condition statement when we know that only the then action // will be executed this.condition.generateCode(currentScope, codeStream, false); @@ -180,7 +201,8 @@ } } else if (hasElsePart) { // generate boolean condition only if needed - if (cst != Constant.NotAConstant && cst.booleanValue() == false) { + if ((this.bits & ASTNode.IsThenStatementUnreachable) == ASTNode.IsThenStatementUnreachable || + (cst != Constant.NotAConstant && cst.booleanValue() == false)) { // No need to generate if condition statement when we know that only the else action // will be executed this.condition.generateCode(currentScope, codeStream, false); Index: compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java,v retrieving revision 1.39 diff -u -r1.39 FlowInfo.java --- compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java 12 Nov 2009 20:09:25 -0000 1.39 +++ compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java 10 Feb 2010 15:50:29 -0000 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2009 IBM Corporation and others. + * Copyright (c) 2000, 2010 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -295,6 +295,77 @@ } /** + * Merge if-else branches using optimized boolean conditions + */ +public static UnconditionalFlowInfo mergedOptimizedBranchesIfElse( + FlowInfo initsWhenTrue, boolean isOptimizedTrue, + FlowInfo initsWhenFalse, boolean isOptimizedFalse, + boolean allowFakeDeadBranch, FlowInfo flowInfo) { + UnconditionalFlowInfo mergedInfo; + if (isOptimizedTrue){ + if (initsWhenTrue == FlowInfo.DEAD_END && allowFakeDeadBranch) { + mergedInfo = initsWhenFalse.setReachMode(FlowInfo.UNREACHABLE). + unconditionalInits(); + } + else { + mergedInfo = + initsWhenTrue.addPotentialInitializationsFrom(initsWhenFalse. + nullInfoLessUnconditionalCopy()). + unconditionalInits(); + } + } + else if (isOptimizedFalse) { + if (initsWhenFalse == FlowInfo.DEAD_END && allowFakeDeadBranch) { + mergedInfo = initsWhenTrue.setReachMode(FlowInfo.UNREACHABLE). + unconditionalInits(); + } + else { + mergedInfo = + initsWhenFalse.addPotentialInitializationsFrom(initsWhenTrue. + nullInfoLessUnconditionalCopy()). + unconditionalInits(); + } + } + else if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0 && + (initsWhenFalse.tagBits & FlowInfo.UNREACHABLE) != 0 && + initsWhenTrue != FlowInfo.DEAD_END && + initsWhenFalse != FlowInfo.DEAD_END) { + // Done when the then branch will always be executed but the condition doesnt have a boolean + // true or false (i.e if(true), etc) for sure + // We don't do this if both if and else branches themselves are in an unreachable code + // or if any of them is a DEAD_END (eg. contains 'return' or 'throws') + mergedInfo = + initsWhenTrue.addPotentialInitializationsFrom(initsWhenFalse. + nullInfoLessUnconditionalCopy()). + unconditionalInits(); + // if a variable is only initialized in one branch and not initialized in the other, + // then we need to cast a doubt on its initialization in the merged info + mergedInfo.definiteInits &= initsWhenFalse.unconditionalCopy().definiteInits; + + } + else if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0 && + (initsWhenTrue.tagBits & FlowInfo.UNREACHABLE) != 0 && initsWhenTrue != FlowInfo.DEAD_END + && initsWhenFalse != FlowInfo.DEAD_END) { + // Done when the else branch will always be executed but the condition doesnt have a boolean + // true or false (i.e if(true), etc) for sure + // We don't do this if both if and else branches themselves are in an unreachable code + // or if any of them is a DEAD_END (eg. contains 'return' or 'throws') + mergedInfo = + initsWhenFalse.addPotentialInitializationsFrom(initsWhenTrue. + nullInfoLessUnconditionalCopy()). + unconditionalInits(); + // if a variable is only initialized in one branch and not initialized in the other, + // then we need to cast a doubt on its initialization in the merged info + mergedInfo.definiteInits &= initsWhenTrue.unconditionalCopy().definiteInits; + } + else { + mergedInfo = initsWhenTrue. + mergedWith(initsWhenFalse.unconditionalInits()); + } + return mergedInfo; +} + +/** * Return REACHABLE if this flow info is reachable, UNREACHABLE * else. * @return REACHABLE if this flow info is reachable, UNREACHABLE #P org.eclipse.jdt.core.tests.compiler Index: src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java,v retrieving revision 1.86 diff -u -r1.86 NullReferenceTest.java --- src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 5 Feb 2010 09:49:35 -0000 1.86 +++ src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 10 Feb 2010 15:50:30 -0000 @@ -26,18 +26,18 @@ } // Static initializer to specify tests subset using TESTS_* static variables - // All specified tests which does not belong to the class are skipped... - // Only the highest compliance level is run; add the VM argument - // -Dcompliance=1.4 (for example) to lower it if needed - static { -// TESTS_NAMES = new String[] { "test011" }; +// All specified tests which does not belong to the class are skipped... +// Only the highest compliance level is run; add the VM argument +// -Dcompliance=1.4 (for example) to lower it if needed +static { +// TESTS_NAMES = new String[] { "test0572_if_statement" }; // TESTS_NUMBERS = new int[] { 561 }; // TESTS_NUMBERS = new int[] { 2999 }; // TESTS_RANGE = new int[] { 2050, -1 }; // TESTS_RANGE = new int[] { 1, 2049 }; // TESTS_RANGE = new int[] { 449, 451 }; // TESTS_RANGE = new int[] { 900, 999 }; - } +} public static Test suite() { return buildAllCompliancesTestSuite(testClass()); @@ -4133,8 +4133,8 @@ " }\n" + "}\n"}, "----------\n" + - "1. ERROR in X.java (at line 11)\r\n" + - " while (o == null) {\r\n" + + "1. ERROR in X.java (at line 11)\n" + + " while (o == null) {\n" + " ^\n" + "Null comparison always yields false: The variable o cannot be null at this location\n" + "----------\n", @@ -5177,8 +5177,8 @@ " }\n" + "}"}, "----------\n" + - "1. ERROR in X.java (at line 13)\r\n" + - " o.toString();\r\n" + + "1. ERROR in X.java (at line 13)\n" + + " o.toString();\n" + " ^\n" + "Potential null pointer access: The variable o may be null at this location\n" + "----------\n", @@ -6054,7 +6054,157 @@ }, ""); } - +// null analysis -- try/catch +//https://bugs.eclipse.org/bugs/show_bug.cgi?id=302446 +public void test0569_try_catch() { + this.runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo() {\n" + + " Object o = null;\n" + + " int i;\n" + + " if (o == null)\n" + // redundant check + " i = 0;\n" + + " try {\n" + + " System.out.println(i);\n" + // might throw a runtime exception + " o = new Object();\n" + + " throw new Exception(\"Exception thrown from try block\");\n" + + " }\n" + + " catch (Throwable t) {\n" + // catches everything + " return;\n" + // gets out + " }\n" + + " }\n" + + "}\n"}, + "----------\n" + + "1. ERROR in X.java (at line 5)\n" + + " if (o == null)\n" + + " ^\n" + + "Redundant null check: The variable o can only be null at this location\n" + + "----------\n" + + "2. ERROR in X.java (at line 8)\n" + + " System.out.println(i);\n" + + " ^\n" + + "The local variable i may not have been initialized\n" + + "----------\n"); +} +// null analysis -- try/catch +//https://bugs.eclipse.org/bugs/show_bug.cgi?id=302446 +public void test0570_try_catch() { + this.runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo() {\n" + + " Object o = null;\n" + + " int i;\n" + + " if (o == null)\n" + // redundant check + " i = 0;\n" + + " try {\n" + + " System.out.println();\n" + // might throw a runtime exception + " o = new Object();\n" + + " if (o != null)\n" + // redundant check + " i = 1\n;" + + " throw new Exception(\"Exception thrown from try block\");\n" + + " }\n" + + " catch (Exception e) {\n" + + " if(i == 0)\n" + + " System.out.println(\"o was initialised\");\n" + + " }\n" + + " }\n" + + "}\n"}, + "----------\n" + + "1. ERROR in X.java (at line 5)\n" + + " if (o == null)\n" + + " ^\n" + + "Redundant null check: The variable o can only be null at this location\n" + + "----------\n" + + "2. ERROR in X.java (at line 10)\n" + + " if (o != null)\n" + + " ^\n" + + "Redundant null check: The variable o cannot be null at this location\n" + + "----------\n" + + "3. ERROR in X.java (at line 15)\n" + + " if(i == 0)\n" + + " ^\n" + + "The local variable i may not have been initialized\n" + + "----------\n"); +} +//null analysis -- try/catch +//https://bugs.eclipse.org/bugs/show_bug.cgi?id=302446 +public void test0571_try_catch_finally() { + this.runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo() {\n" + + " Object o = null;\n" + + " int i;\n" + + " if (o == null)\n" + // redundant check + " i = 0;\n" + + " try {\n" + + " o = new Object();\n" + + " i = 1\n;" + + " throw new Exception(\"Exception thrown from try block\");\n" + + " }\n" + + " catch (Exception e) {\n" + + " if(o == null)\n" + + " o = new Object();\n" + + " i = 1;\n" + + " }\n" + + " finally {\n" + + " if (i==1) {\n" + + " System.out.println(\"Method ended with o being initialised\");\n" + + " System.out.println(o.toString());\n" + // may be null + " } \n" + + " }\n" + + " }\n" + + "}\n"}, + "----------\n" + + "1. ERROR in X.java (at line 5)\n" + + " if (o == null)\n" + + " ^\n" + + "Redundant null check: The variable o can only be null at this location\n" + + "----------\n" + + "2. ERROR in X.java (at line 18)\n" + + " if (i==1) {\n" + + " ^\n" + + "The local variable i may not have been initialized\n" + + "----------\n" + + "3. ERROR in X.java (at line 20)\n" + + " System.out.println(o.toString());\n" + + " ^\n" + + "Potential null pointer access: The variable o may be null at this location\n" + + "----------\n"); +} +//null analysis -- if statement +//https://bugs.eclipse.org/bugs/show_bug.cgi?id=302446 +public void test0572_if_statement() { + this.runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo() {\n" + + " Object o = null;\n" + + " int i;\n" + + " if (o == null) // redundant check\n" + + " i = 0;\n" + + " System.out.println(i);\n" + + " }\n" + + "}\n" + + ""}, + "----------\n" + + "1. ERROR in X.java (at line 5)\n" + + " if (o == null) // redundant check\n" + + " ^\n" + + "Redundant null check: The variable o can only be null at this location\n" + + "----------\n" + + "2. ERROR in X.java (at line 7)\n" + + " System.out.println(i);\n" + + " ^\n" + + "The local variable i may not have been initialized\n" + + "----------\n"); +} // null analysis - throw // https://bugs.eclipse.org/bugs/show_bug.cgi?id=201182 public void test0595_throw() { @@ -8975,17 +9125,17 @@ " }\n" + "}"}, "----------\n" + - "1. ERROR in X.java (at line 6)\r\n" + - " if (o != null) return;\r\n" + + "1. ERROR in X.java (at line 6)\n" + + " if (o != null) return;\n" + " ^\n" + "Null comparison always yields false: The variable o can only be null at this location\n" + "----------\n" + - "2. ERROR in X.java (at line 7)\r\n" + - " o = null;\r\n" + + "2. ERROR in X.java (at line 7)\n" + + " o = null;\n" + " ^\n" + "Redundant assignment: The variable o can only be null at this location\n" + "----------\n", - JavacTestOptions.Excuse.EclipseWarningConfiguredAsError); + JavacTestOptions.Excuse.EclipseWarningConfiguredAsError); } public void test1019() {