### Eclipse Workspace Patch 1.0 #P org.eclipse.jdt.core Index: compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java,v retrieving revision 1.98 diff -u -r1.98 ConditionalExpression.java --- compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java 14 Jan 2011 17:02:24 -0000 1.98 +++ compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java 4 Mar 2011 18:55:46 -0000 @@ -7,7 +7,10 @@ * * Contributors: * IBM Corporation - initial API and implementation - * Stephen Herrmann - Contributions for bugs 133125, 292478 + * Stephen Herrmann - Contributions for + * bug 133125 - [compiler][null] need to report the null status of expressions and analyze them simultaneously + * bug 292478 - Report potentially null across variable assignment + * bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -29,6 +32,9 @@ int trueInitStateIndex = -1; int falseInitStateIndex = -1; int mergedInitStateIndex = -1; + + // we compute and store the null status during analyseCode (https://bugs.eclipse.org/324178): + private int nullStatus = FlowInfo.UNKNOWN; public ConditionalExpression( Expression condition, @@ -81,10 +87,30 @@ FlowInfo mergedInfo; if (isConditionOptimizedTrue){ mergedInfo = trueFlowInfo.addPotentialInitializationsFrom(falseFlowInfo); + this.nullStatus = this.valueIfTrue.nullStatus(trueFlowInfo); } else if (isConditionOptimizedFalse) { mergedInfo = falseFlowInfo.addPotentialInitializationsFrom(trueFlowInfo); + this.nullStatus = this.valueIfFalse.nullStatus(falseFlowInfo); } else { - // if ((t && (v = t)) ? t : t && (v = f)) r = v; -- ok + // this block must meet two conflicting requirements (see https://bugs.eclipse.org/324178): + // (1) For null analysis of "Object o2 = (o1 != null) ? o1 : new Object();" we need to distinguish + // the paths *originating* from the evaluation of the condition to true/false respectively. + // This is used to determine the possible null status of the entire conditional expression. + // (2) For definite assignment analysis (JLS 16.1.5) of boolean conditional expressions of the form + // "if (c1 ? expr1 : expr2) use(v);" we need to check whether any variable v will be definitely + // assigned whenever the entire conditional expression evaluates to true (to reach the then branch). + // I.e., we need to collect flowInfo *towards* the overall outcome true/false + // (regardless of the evaluation of the condition). + + // to support (1) save the branches originating from the condition (for use by nullStatus): + computeNullInfo(trueFlowInfo, falseFlowInfo); + + // to support (2) we split the true/false branches according to their inner structure. Consider this: + // if (b ? false : (true && (v = false))) return v; -- ok + // - expr1 ("false") has no path towards true (mark as unreachable) + // - expr2 ("(true && (v = false))") has a branch towards true on which v is assigned. + // -> merging these two branches yields: v is assigned + // - the paths towards false are irrelevant since the enclosing if has no else. cst = this.optimizedIfTrueConstant; boolean isValueIfTrueOptimizedTrue = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == true; boolean isValueIfTrueOptimizedFalse = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == false; @@ -93,26 +119,26 @@ boolean isValueIfFalseOptimizedTrue = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == true; boolean isValueIfFalseOptimizedFalse = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == false; - UnconditionalFlowInfo trueInfoWhenTrue = trueFlowInfo.initsWhenTrue().unconditionalCopy(); - UnconditionalFlowInfo falseInfoWhenTrue = falseFlowInfo.initsWhenTrue().unconditionalCopy(); - UnconditionalFlowInfo trueInfoWhenFalse = trueFlowInfo.initsWhenFalse().unconditionalInits(); - UnconditionalFlowInfo falseInfoWhenFalse = falseFlowInfo.initsWhenFalse().unconditionalInits(); + UnconditionalFlowInfo trueFlowTowardsTrue = trueFlowInfo.initsWhenTrue().unconditionalCopy(); + UnconditionalFlowInfo falseFlowTowardsTrue = falseFlowInfo.initsWhenTrue().unconditionalCopy(); + UnconditionalFlowInfo trueFlowTowardsFalse = trueFlowInfo.initsWhenFalse().unconditionalInits(); + UnconditionalFlowInfo falseFlowTowardsFalse = falseFlowInfo.initsWhenFalse().unconditionalInits(); if (isValueIfTrueOptimizedFalse) { - trueInfoWhenTrue.setReachMode(FlowInfo.UNREACHABLE); + trueFlowTowardsTrue.setReachMode(FlowInfo.UNREACHABLE); } if (isValueIfFalseOptimizedFalse) { - falseInfoWhenTrue.setReachMode(FlowInfo.UNREACHABLE); + falseFlowTowardsTrue.setReachMode(FlowInfo.UNREACHABLE); } if (isValueIfTrueOptimizedTrue) { - trueInfoWhenFalse.setReachMode(FlowInfo.UNREACHABLE); + trueFlowTowardsFalse.setReachMode(FlowInfo.UNREACHABLE); } if (isValueIfFalseOptimizedTrue) { - falseInfoWhenFalse.setReachMode(FlowInfo.UNREACHABLE); + falseFlowTowardsFalse.setReachMode(FlowInfo.UNREACHABLE); } mergedInfo = FlowInfo.conditional( - trueInfoWhenTrue.mergedWith(falseInfoWhenTrue), - trueInfoWhenFalse.mergedWith(falseInfoWhenFalse)); + trueFlowTowardsTrue.mergedWith(falseFlowTowardsTrue), + trueFlowTowardsFalse.mergedWith(falseFlowTowardsFalse)); } this.mergedInitStateIndex = currentScope.methodScope().recordInitializationStates(mergedInfo); @@ -120,6 +146,30 @@ return mergedInfo; } + private void computeNullInfo(FlowInfo trueBranchInfo, FlowInfo falseBranchInfo) { + // given that the condition cannot be optimized to a constant + // we now merge the nullStatus from both branches: + int ifTrueNullStatus = this.valueIfTrue.nullStatus(trueBranchInfo); + int ifFalseNullStatus = this.valueIfFalse.nullStatus(falseBranchInfo); + + if (ifTrueNullStatus == ifFalseNullStatus) { + this.nullStatus = ifTrueNullStatus; + return; + } + // is there a chance of null (or non-null)? -> potentially null etc. + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=133125 + int status = 0; + int combinedStatus = ifTrueNullStatus|ifFalseNullStatus; + if ((combinedStatus & (FlowInfo.NULL|FlowInfo.POTENTIALLY_NULL)) != 0) + status |= FlowInfo.POTENTIALLY_NULL; + if ((combinedStatus & (FlowInfo.NON_NULL|FlowInfo.POTENTIALLY_NON_NULL)) != 0) + status |= FlowInfo.POTENTIALLY_NON_NULL; + if ((combinedStatus & (FlowInfo.UNKNOWN|FlowInfo.POTENTIALLY_UNKNOWN)) != 0) + status |= FlowInfo.POTENTIALLY_UNKNOWN; + if (status > 0) + this.nullStatus = status; + } + /** * Code generation for the conditional operator ?: * @@ -306,33 +356,9 @@ codeStream.updateLastRecordedEndPC(currentScope, codeStream.position); } -public int nullStatus(FlowInfo flowInfo) { - Constant cst = this.condition.optimizedBooleanConstant(); - if (cst != Constant.NotAConstant) { - if (cst.booleanValue()) { - return this.valueIfTrue.nullStatus(flowInfo); - } - return this.valueIfFalse.nullStatus(flowInfo); + public int nullStatus(FlowInfo flowInfo) { + return this.nullStatus; } - int ifTrueNullStatus = this.valueIfTrue.nullStatus(flowInfo), - ifFalseNullStatus = this.valueIfFalse.nullStatus(flowInfo); - if (ifTrueNullStatus == ifFalseNullStatus) { - return ifTrueNullStatus; - } - // is there a chance of null (or non-null)? -> potentially null etc. - // https://bugs.eclipse.org/bugs/show_bug.cgi?id=133125 - int status = 0; - int combinedStatus = ifTrueNullStatus|ifFalseNullStatus; - if ((combinedStatus & (FlowInfo.NULL|FlowInfo.POTENTIALLY_NULL)) != 0) - status |= FlowInfo.POTENTIALLY_NULL; - if ((combinedStatus & (FlowInfo.NON_NULL|FlowInfo.POTENTIALLY_NON_NULL)) != 0) - status |= FlowInfo.POTENTIALLY_NON_NULL; - if ((combinedStatus & (FlowInfo.UNKNOWN|FlowInfo.POTENTIALLY_UNKNOWN)) != 0) - status |= FlowInfo.POTENTIALLY_UNKNOWN; - if (status > 0) - return status; - return FlowInfo.UNKNOWN; -} public Constant optimizedBooleanConstant() { #P org.eclipse.jdt.core.tests.compiler Index: src/org/eclipse/jdt/core/tests/compiler/regression/InitializationTests.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/InitializationTests.java,v retrieving revision 1.4 diff -u -r1.4 InitializationTests.java --- src/org/eclipse/jdt/core/tests/compiler/regression/InitializationTests.java 21 Sep 2010 14:02:56 -0000 1.4 +++ src/org/eclipse/jdt/core/tests/compiler/regression/InitializationTests.java 4 Mar 2011 18:55:49 -0000 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2010 IBM Corporation and others. + * Copyright (c) 2010, 2011 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 @@ -7,6 +7,7 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - contribution for bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -14,6 +15,7 @@ import junit.framework.Test; +import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants; import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; public class InitializationTests extends AbstractRegressionTest { @@ -393,6 +395,75 @@ "----------\n", null, false, options); } + +// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself +// definite assignment along all true-yielding paths is sufficient +public void testBug324178b() { + this.runConformTest( + new String[] { + "Bug324178.java", + "public class Bug324178 {\n" + + " boolean foo(boolean b) {\n" + + " boolean v;\n" + + " if (b ? false : (true && (v = true)))\n" + + " return v;\n" + // OK to read v! + " return false;\n" + + " }\n" + + " public static void main(String[] args) {\n" + + " System.out.print(new Bug324178().foo(false));\n" + + " }\n" + + "}\n" + }, + "true"); +} + +// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself +// definite assignment along all true-yielding paths is sufficient +public void testBug324178c() { + this.runConformTest( + new String[] { + "Bug324178.java", + "public class Bug324178 {\n" + + " boolean foo() {\n" + + " boolean r=false;" + + " boolean v;\n" + + " if ((true && (v = true)) ? true : true && (v = false)) r = v;\n" + + " return r;\n" + + " }\n" + + " public static void main(String[] args) {\n" + + " System.out.print(new Bug324178().foo());\n" + + " }\n" + + "}\n" + }, + "true"); +} +// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself +// must detect that b2 may be uninitialized, no special semantics for Boolean +public void testBug324178d() { + if (this.complianceLevel < ClassFileConstants.JDK1_5) + return; + this.runNegativeTest( + new String[] { + "Bug324178.java", + "public class Bug324178 {\n" + + " boolean foo(boolean b1) {\n" + + " Boolean b2;\n" + + " if (b1 ? (b2 = Boolean.TRUE) : null)\n" + + " return b2;\n" + + " return false;\n" + + " }\n" + + " public static void main(String[] args) {\n" + + " System.out.print(new Bug324178().foo(true));\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in Bug324178.java (at line 5)\n" + + " return b2;\n" + + " ^^\n" + + "The local variable b2 may not have been initialized\n" + + "----------\n"); +} public static Class testClass() { return InitializationTests.class; } 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.114 diff -u -r1.114 NullReferenceTest.java --- src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 3 Mar 2011 13:00:33 -0000 1.114 +++ src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 4 Mar 2011 18:56:05 -0000 @@ -16,6 +16,7 @@ * bug 332637 - Dead Code detection removing code that isn't dead * bug 338303 - Warning about Redundant assignment conflicts with definite assignment * bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop + * bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -43,7 +44,7 @@ // 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[] { "testBug336428e" }; +// TESTS_NAMES = new String[] { "testBug324178" }; // TESTS_NUMBERS = new int[] { 561 }; // TESTS_RANGE = new int[] { 1, 2049 }; } @@ -14204,4 +14205,39 @@ }, ""); } +// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself +public void testBug324178() { + this.runConformTest( + new String[] { + "Bug324178.java", + "public class Bug324178 {\n" + + " boolean b;\n" + + " void foo(Object u) {\n" + + " if (u == null) {}\n" + + " Object o = (u == null) ? new Object() : u;\n" + + " o.toString(); // Incorrect potential NPE\n" + + " }\n" + + "}\n" + }, + ""); +} + +// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself +public void testBug324178a() { + this.runConformTest( + new String[] { + "Bug324178.java", + "public class Bug324178 {\n" + + " boolean b;\n" + + " void foo(Boolean u) {\n" + + " if (u == null) {}\n" + + " Boolean o;\n" + + " o = (u == null) ? Boolean.TRUE : u;\n" + + " o.toString(); // Incorrect potential NPE\n" + + " }\n" + + "}\n" + }, + ""); +} + } \ No newline at end of file