Attachment #190328 for bug #324178

Lines 7-13 Link Here

(-)compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java (-14 / +43 lines)
7	*	7	*
8	* Contributors:	8	* Contributors:
9	* IBM Corporation - initial API and implementation	9	* IBM Corporation - initial API and implementation
10	* Stephen Herrmann <stephan@cs.tu-berlin.de> - Contributions for bugs 133125, 292478	10	* Stephen Herrmann <stephan@cs.tu-berlin.de> - Contributions for
		11	* bug 133125 - [compiler][null] need to report the null status of expressions and analyze them simultaneously
		12	* bug 292478 - Report potentially null across variable assignment
		13	* bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
11	*******************************************************************************/	14	*******************************************************************************/
12	package org.eclipse.jdt.internal.compiler.ast;	15	package org.eclipse.jdt.internal.compiler.ast;
13		16
Lines 29-34 Link Here
29	int trueInitStateIndex = -1;	32	int trueInitStateIndex = -1;
30	int falseInitStateIndex = -1;	33	int falseInitStateIndex = -1;
31	int mergedInitStateIndex = -1;	34	int mergedInitStateIndex = -1;
		35
		36	// remember flow infos of true/false branches for nullStatus (see comment in analyseCode):
		37	private FlowInfo trueBranchInfo;
		38	private FlowInfo falseBranchInfo;
32		39
33	public ConditionalExpression(	40	public ConditionalExpression(
34	Expression condition,	41	Expression condition,
Lines 84-90 Link Here
84	} else if (isConditionOptimizedFalse) {	91	} else if (isConditionOptimizedFalse) {
85	mergedInfo = falseFlowInfo.addPotentialInitializationsFrom(trueFlowInfo);	92	mergedInfo = falseFlowInfo.addPotentialInitializationsFrom(trueFlowInfo);
86	} else {	93	} else {
87	// if ((t && (v = t)) ? t : t && (v = f)) r = v; -- ok	94	// this block must meet two conflicting requirements (see https://bugs.eclipse.org/324178):
		95	// (1) For null analysis of "Object o2 = (o1 != null) ? o1 : new Object();" we need to distinguish
		96	// the paths originating from the evaluation of the condition to true/false respectively.
		97	// This is used to determine the possible null status of the entire conditional expression.
		98	// (2) For definite assignment analysis (JLS 16.1.5) of boolean conditional expressions of the form
		99	// "if (c1 ? expr1 : expr2) use(v);" we need to check whether any variable v will be definitely
		100	// assigned whenever the entire conditional expression evaluates to true (to reach the then branch).
		101	// I.e., we need to collect flowInfo towards the overall outcome true/false
		102	// (regardless of the evaluation of the condition).
		103
		104	// to support (1) save the branches originating from the condition (for use by nullStatus):
		105	this.trueBranchInfo = trueFlowInfo;
		106	this.falseBranchInfo = falseFlowInfo;
		107
		108	// to support (2) we split the true/false branches according to their inner structure. Consider this:
		109	// if (b ? false : (true && (v = false))) return v; -- ok
		110	// - expr1 ("false") has no path towards true (mark as unreachable)
		111	// - expr2 ("(true && (v = false))") has a branch towards true on which v is assigned.
		112	// -> merging these two branches yields: v is assigned
		113	// - the paths towards false are irrelevant since the enclosing if has no else.
88	cst = this.optimizedIfTrueConstant;	114	cst = this.optimizedIfTrueConstant;
89	boolean isValueIfTrueOptimizedTrue = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == true;	115	boolean isValueIfTrueOptimizedTrue = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == true;
90	boolean isValueIfTrueOptimizedFalse = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == false;	116	boolean isValueIfTrueOptimizedFalse = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == false;
Lines 93-118 Link Here
93	boolean isValueIfFalseOptimizedTrue = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == true;	119	boolean isValueIfFalseOptimizedTrue = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == true;
94	boolean isValueIfFalseOptimizedFalse = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == false;	120	boolean isValueIfFalseOptimizedFalse = cst != null && cst != Constant.NotAConstant && cst.booleanValue() == false;
95		121
96	UnconditionalFlowInfo trueInfoWhenTrue = trueFlowInfo.initsWhenTrue().unconditionalCopy();	122	UnconditionalFlowInfo trueFlowTowardsTrue = trueFlowInfo.initsWhenTrue().unconditionalCopy();
97	UnconditionalFlowInfo falseInfoWhenTrue = falseFlowInfo.initsWhenTrue().unconditionalCopy();	123	UnconditionalFlowInfo falseFlowTowardsTrue = falseFlowInfo.initsWhenTrue().unconditionalCopy();
98	UnconditionalFlowInfo trueInfoWhenFalse = trueFlowInfo.initsWhenFalse().unconditionalInits();	124	UnconditionalFlowInfo trueFlowTowardsFalse = trueFlowInfo.initsWhenFalse().unconditionalInits();
99	UnconditionalFlowInfo falseInfoWhenFalse = falseFlowInfo.initsWhenFalse().unconditionalInits();	125	UnconditionalFlowInfo falseFlowTowardsFalse = falseFlowInfo.initsWhenFalse().unconditionalInits();
100	if (isValueIfTrueOptimizedFalse) {	126	if (isValueIfTrueOptimizedFalse) {
101	trueInfoWhenTrue.setReachMode(FlowInfo.UNREACHABLE);	127	trueFlowTowardsTrue.setReachMode(FlowInfo.UNREACHABLE);
102	}	128	}
103	if (isValueIfFalseOptimizedFalse) {	129	if (isValueIfFalseOptimizedFalse) {
104	falseInfoWhenTrue.setReachMode(FlowInfo.UNREACHABLE);	130	falseFlowTowardsTrue.setReachMode(FlowInfo.UNREACHABLE);
105	}	131	}
106	if (isValueIfTrueOptimizedTrue) {	132	if (isValueIfTrueOptimizedTrue) {
107	trueInfoWhenFalse.setReachMode(FlowInfo.UNREACHABLE);	133	trueFlowTowardsFalse.setReachMode(FlowInfo.UNREACHABLE);
108	}	134	}
109	if (isValueIfFalseOptimizedTrue) {	135	if (isValueIfFalseOptimizedTrue) {
110	falseInfoWhenFalse.setReachMode(FlowInfo.UNREACHABLE);	136	falseFlowTowardsFalse.setReachMode(FlowInfo.UNREACHABLE);
111	}	137	}
112	mergedInfo =	138	mergedInfo =
113	FlowInfo.conditional(	139	FlowInfo.conditional(
114	trueInfoWhenTrue.mergedWith(falseInfoWhenTrue),	140	trueFlowTowardsTrue.mergedWith(falseFlowTowardsTrue),
115	trueInfoWhenFalse.mergedWith(falseInfoWhenFalse));	141	trueFlowTowardsFalse.mergedWith(falseFlowTowardsFalse));
116	}	142	}
117	this.mergedInitStateIndex =	143	this.mergedInitStateIndex =
118	currentScope.methodScope().recordInitializationStates(mergedInfo);	144	currentScope.methodScope().recordInitializationStates(mergedInfo);
Lines 314-321 Link Here
314	}	340	}
315	return this.valueIfFalse.nullStatus(flowInfo);	341	return this.valueIfFalse.nullStatus(flowInfo);
316	}	342	}
317	int ifTrueNullStatus = this.valueIfTrue.nullStatus(flowInfo),	343	int ifTrueNullStatus =
318	ifFalseNullStatus = this.valueIfFalse.nullStatus(flowInfo);	344	this.valueIfTrue.nullStatus(this.trueBranchInfo != null ? this.trueBranchInfo : flowInfo.initsWhenTrue());
		345	int ifFalseNullStatus =
		346	this.valueIfFalse.nullStatus(this.falseBranchInfo != null ? this.falseBranchInfo : flowInfo.initsWhenFalse());
		347
319	if (ifTrueNullStatus == ifFalseNullStatus) {	348	if (ifTrueNullStatus == ifFalseNullStatus) {
320	return ifTrueNullStatus;	349	return ifTrueNullStatus;
321	}	350	}




 *     						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;


// 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[] { "testBug324178" };
//		TESTS_NUMBERS = new int[] { 561 };
//		TESTS_RANGE = new int[] { 1, 2049 };
}

		},
		"");
}
// 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
// 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
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");
}
// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
public void testBug324178e() {
	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"			
		},
		"");
}

}

Return to bug 324178

Lines 16-21 Link Here

(-)src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java (-1 / +104 lines)
16	* bug 332637 - Dead Code detection removing code that isn't dead	16	* bug 332637 - Dead Code detection removing code that isn't dead
17	* bug 338303 - Warning about Redundant assignment conflicts with definite assignment	17	* bug 338303 - Warning about Redundant assignment conflicts with definite assignment
18	* bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop	18	* bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop
		19	* bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
19	*******************************************************************************/	20	*******************************************************************************/
20	package org.eclipse.jdt.core.tests.compiler.regression;	21	package org.eclipse.jdt.core.tests.compiler.regression;
21		22
Lines 43-49 Link Here
43	// Only the highest compliance level is run; add the VM argument	44	// Only the highest compliance level is run; add the VM argument
44	// -Dcompliance=1.4 (for example) to lower it if needed	45	// -Dcompliance=1.4 (for example) to lower it if needed
45	static {	46	static {
46	// TESTS_NAMES = new String[] { "testBug336428e" };	47	// TESTS_NAMES = new String[] { "testBug324178" };
47	// TESTS_NUMBERS = new int[] { 561 };	48	// TESTS_NUMBERS = new int[] { 561 };
48	// TESTS_RANGE = new int[] { 1, 2049 };	49	// TESTS_RANGE = new int[] { 1, 2049 };
49	}	50	}
Lines 14204-14207 Link Here
14204	},	14205	},
14205	"");	14206	"");
14206	}	14207	}
		14208	// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
		14209	public void testBug324178() {
		14210	this.runConformTest(
		14211	new String[] {
		14212	"Bug324178.java",
		14213	"public class Bug324178 {\n" +
		14214	" boolean b;\n" +
		14215	" void foo(Object u) {\n" +
		14216	" if (u == null) {}\n" +
		14217	" Object o = (u == null) ? new Object() : u;\n" +
		14218	" o.toString(); // Incorrect potential NPE\n" +
		14219	" }\n" +
		14220	"}\n"
		14221	},
		14222	"");
		14223	}
		14224
		14225	// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
		14226	// definite assignment along all true-yielding paths is sufficient
		14227	public void testBug324178b() {
		14228	this.runConformTest(
		14229	new String[] {
		14230	"Bug324178.java",
		14231	"public class Bug324178 {\n" +
		14232	" boolean foo(boolean b) {\n" +
		14233	" boolean v;\n" +
		14234	" if (b ? false : (true && (v = true)))\n" +
		14235	" return v;\n" + // OK to read v!
		14236	" return false;\n" +
		14237	" }\n" +
		14238	" public static void main(String[] args) {\n" +
		14239	" System.out.print(new Bug324178().foo(false));\n" +
		14240	" }\n" +
		14241	"}\n"
		14242	},
		14243	"true");
		14244	}
		14245	// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
		14246	// definite assignment along all true-yielding paths is sufficient
		14247	public void testBug324178c() {
		14248	this.runConformTest(
		14249	new String[] {
		14250	"Bug324178.java",
		14251	"public class Bug324178 {\n" +
		14252	" boolean foo() {\n" +
		14253	" boolean r=false;" +
		14254	" boolean v;\n" +
		14255	" if ((true && (v = true)) ? true : true && (v = false)) r = v;\n" +
		14256	" return r;\n" +
		14257	" }\n" +
		14258	" public static void main(String[] args) {\n" +
		14259	" System.out.print(new Bug324178().foo());\n" +
		14260	" }\n" +
		14261	"}\n"
		14262	},
		14263	"true");
		14264	}
		14265	// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
		14266	// must detect that b2 may be uninitialized
		14267	public void testBug324178d() {
		14268	if (this.complianceLevel < ClassFileConstants.JDK1_5)
		14269	return;
		14270	this.runNegativeTest(
		14271	new String[] {
14272	"Bug324178.java",
14273	"public class Bug324178 {\n" +
14274	" boolean foo(boolean b1) {\n" +
14275	" Boolean b2;\n" +
14276	" if (b1 ? (b2 = Boolean.TRUE) : null)\n" +
14277	" return b2;\n" +
14278	" return false;\n" +
14279	" }\n" +
14280	" public static void main(String[] args) {\n" +
14281	" System.out.print(new Bug324178().foo(true));\n" +
14282	" }\n" +
14283	"}\n"
14284	},
14285	"----------\n" +
14286	"1. ERROR in Bug324178.java (at line 5)\n" +
14287	" return b2;\n" +
14288	" ^^\n" +
14289	"The local variable b2 may not have been initialized\n" +
14290	"----------\n");
14291	}
14292	// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
14293	public void testBug324178e() {
14294	this.runConformTest(
14295	new String[] {
14296	"Bug324178.java",
14297	"public class Bug324178 {\n" +
14298	" boolean b;\n" +
14299	" void foo(Boolean u) {\n" +
14300	" if (u == null) {}\n" +
14301	" Boolean o;\n" +
14302	" o = (u == null) ? Boolean.TRUE : u;\n" +
14303	" o.toString(); // Incorrect potential NPE\n" +
14304	" }\n" +
14305	"}\n"
14306	},
14307	"");
14308	}
14309
14207	}	14310	}