Attachment #190428 for bug #324178

Lines 7-13 Link Here

(-)compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java (-38 / +64 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	// we compute and store the null status during analyseCode (https://bugs.eclipse.org/324178):
		37	private int nullStatus = FlowInfo.UNKNOWN;
32		38
33	public ConditionalExpression(	39	public ConditionalExpression(
34	Expression condition,	40	Expression condition,
Lines 81-90 Link Here
81	FlowInfo mergedInfo;	87	FlowInfo mergedInfo;
82	if (isConditionOptimizedTrue){	88	if (isConditionOptimizedTrue){
83	mergedInfo = trueFlowInfo.addPotentialInitializationsFrom(falseFlowInfo);	89	mergedInfo = trueFlowInfo.addPotentialInitializationsFrom(falseFlowInfo);
		90	this.nullStatus = this.valueIfTrue.nullStatus(trueFlowInfo);
84	} else if (isConditionOptimizedFalse) {	91	} else if (isConditionOptimizedFalse) {
85	mergedInfo = falseFlowInfo.addPotentialInitializationsFrom(trueFlowInfo);	92	mergedInfo = falseFlowInfo.addPotentialInitializationsFrom(trueFlowInfo);
		93	this.nullStatus = this.valueIfFalse.nullStatus(falseFlowInfo);
86	} else {	94	} else {
87	// if ((t && (v = t)) ? t : t && (v = f)) r = v; -- ok	95	// this block must meet two conflicting requirements (see https://bugs.eclipse.org/324178):
		96	// (1) For null analysis of "Object o2 = (o1 != null) ? o1 : new Object();" we need to distinguish
		97	// the paths originating from the evaluation of the condition to true/false respectively.
		98	// This is used to determine the possible null status of the entire conditional expression.
		99	// (2) For definite assignment analysis (JLS 16.1.5) of boolean conditional expressions of the form
		100	// "if (c1 ? expr1 : expr2) use(v);" we need to check whether any variable v will be definitely
		101	// assigned whenever the entire conditional expression evaluates to true (to reach the then branch).
		102	// I.e., we need to collect flowInfo towards the overall outcome true/false
		103	// (regardless of the evaluation of the condition).
		104
		105	// to support (1) save the branches originating from the condition (for use by nullStatus):
		106	computeNullInfo(trueFlowInfo, 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 120-125 Link Here
120	return mergedInfo;	146	return mergedInfo;
121	}	147	}
122		148
		149	private void computeNullInfo(FlowInfo trueBranchInfo, FlowInfo falseBranchInfo) {
		150	// given that the condition cannot be optimized to a constant
		151	// we now merge the nullStatus from both branches:
		152	int ifTrueNullStatus = this.valueIfTrue.nullStatus(trueBranchInfo);
		153	int ifFalseNullStatus = this.valueIfFalse.nullStatus(falseBranchInfo);
		154
		155	if (ifTrueNullStatus == ifFalseNullStatus) {
		156	this.nullStatus = ifTrueNullStatus;
		157	return;
		158	}
		159	// is there a chance of null (or non-null)? -> potentially null etc.
		160	// https://bugs.eclipse.org/bugs/show_bug.cgi?id=133125
		161	int status = 0;
		162	int combinedStatus = ifTrueNullStatus\|ifFalseNullStatus;
		163	if ((combinedStatus & (FlowInfo.NULL\|FlowInfo.POTENTIALLY_NULL)) != 0)
		164	status \|= FlowInfo.POTENTIALLY_NULL;
		165	if ((combinedStatus & (FlowInfo.NON_NULL\|FlowInfo.POTENTIALLY_NON_NULL)) != 0)
		166	status \|= FlowInfo.POTENTIALLY_NON_NULL;
		167	if ((combinedStatus & (FlowInfo.UNKNOWN\|FlowInfo.POTENTIALLY_UNKNOWN)) != 0)
		168	status \|= FlowInfo.POTENTIALLY_UNKNOWN;
		169	if (status > 0)
		170	this.nullStatus = status;
		171	}
		172
123	/**	173	/**
124	* Code generation for the conditional operator ?:	174	* Code generation for the conditional operator ?:
125	*	175	*
Lines 306-338 Link Here
306	codeStream.updateLastRecordedEndPC(currentScope, codeStream.position);	356	codeStream.updateLastRecordedEndPC(currentScope, codeStream.position);
307	}	357	}
308		358
309	public int nullStatus(FlowInfo flowInfo) {	359	public int nullStatus(FlowInfo flowInfo) {
310	Constant cst = this.condition.optimizedBooleanConstant();	360	return this.nullStatus;
311	if (cst != Constant.NotAConstant) {
312	if (cst.booleanValue()) {
313	return this.valueIfTrue.nullStatus(flowInfo);
314	}
315	return this.valueIfFalse.nullStatus(flowInfo);
316	}	361	}
317	int ifTrueNullStatus = this.valueIfTrue.nullStatus(flowInfo),
318	ifFalseNullStatus = this.valueIfFalse.nullStatus(flowInfo);
319	if (ifTrueNullStatus == ifFalseNullStatus) {
320	return ifTrueNullStatus;
321	}
322	// is there a chance of null (or non-null)? -> potentially null etc.
323	// https://bugs.eclipse.org/bugs/show_bug.cgi?id=133125
324	int status = 0;
325	int combinedStatus = ifTrueNullStatus\|ifFalseNullStatus;
326	if ((combinedStatus & (FlowInfo.NULL\|FlowInfo.POTENTIALLY_NULL)) != 0)
327	status \|= FlowInfo.POTENTIALLY_NULL;
328	if ((combinedStatus & (FlowInfo.NON_NULL\|FlowInfo.POTENTIALLY_NON_NULL)) != 0)
329	status \|= FlowInfo.POTENTIALLY_NON_NULL;
330	if ((combinedStatus & (FlowInfo.UNKNOWN\|FlowInfo.POTENTIALLY_UNKNOWN)) != 0)
331	status \|= FlowInfo.POTENTIALLY_UNKNOWN;
332	if (status > 0)
333	return status;
334	return FlowInfo.UNKNOWN;
335	}
336		362
337	public Constant optimizedBooleanConstant() {	363	public Constant optimizedBooleanConstant() {
338		364

Lines 1-5 Link Here

(-)src/org/eclipse/jdt/core/tests/compiler/regression/InitializationTests.java (-1 / +72 lines)
1	/*******************************************************************************	1	/*******************************************************************************
2	* Copyright (c) 2010 IBM Corporation and others.	2	* Copyright (c) 2010, 2011 IBM Corporation and others.
3	* All rights reserved. This program and the accompanying materials	3	* All rights reserved. This program and the accompanying materials
4	* are made available under the terms of the Eclipse Public License v1.0	4	* are made available under the terms of the Eclipse Public License v1.0
5	* which accompanies this distribution, and is available at	5	* which accompanies this distribution, and is available at
Lines 7-12 Link Here
7	*	7	*
8	* Contributors:	8	* Contributors:
9	* IBM Corporation - initial API and implementation	9	* IBM Corporation - initial API and implementation
		10	* Stephan Herrmann - contribution for bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
10	*******************************************************************************/	11	*******************************************************************************/
11	package org.eclipse.jdt.core.tests.compiler.regression;	12	package org.eclipse.jdt.core.tests.compiler.regression;
12		13
Lines 14-19 Link Here
14		15
15	import junit.framework.Test;	16	import junit.framework.Test;
16		17
		18	import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
17	import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;	19	import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
18		20
19	public class InitializationTests extends AbstractRegressionTest {	21	public class InitializationTests extends AbstractRegressionTest {
Lines 393-398 Link Here
393	"----------\n",	395	"----------\n",
394	null, false, options);	396	null, false, options);
395	}	397	}
		398
		399	// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
		400	// definite assignment along all true-yielding paths is sufficient
		401	public void testBug324178b() {
		402	this.runConformTest(
		403	new String[] {
		404	"Bug324178.java",
		405	"public class Bug324178 {\n" +
		406	" boolean foo(boolean b) {\n" +
		407	" boolean v;\n" +
		408	" if (b ? false : (true && (v = true)))\n" +
		409	" return v;\n" + // OK to read v!
		410	" return false;\n" +
		411	" }\n" +
		412	" public static void main(String[] args) {\n" +
		413	" System.out.print(new Bug324178().foo(false));\n" +
		414	" }\n" +
		415	"}\n"
		416	},
		417	"true");
		418	}
		419
		420	// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
		421	// definite assignment along all true-yielding paths is sufficient
		422	public void testBug324178c() {
		423	this.runConformTest(
		424	new String[] {
		425	"Bug324178.java",
		426	"public class Bug324178 {\n" +
		427	" boolean foo() {\n" +
		428	" boolean r=false;" +
		429	" boolean v;\n" +
		430	" if ((true && (v = true)) ? true : true && (v = false)) r = v;\n" +
		431	" return r;\n" +
		432	" }\n" +
		433	" public static void main(String[] args) {\n" +
		434	" System.out.print(new Bug324178().foo());\n" +
		435	" }\n" +
		436	"}\n"
		437	},
		438	"true");
		439	}
		440	// Bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
		441	// must detect that b2 may be uninitialized, no special semantics for Boolean
		442	public void testBug324178d() {
		443	if (this.complianceLevel < ClassFileConstants.JDK1_5)
		444	return;
		445	this.runNegativeTest(
		446	new String[] {
		447	"Bug324178.java",
		448	"public class Bug324178 {\n" +
		449	" boolean foo(boolean b1) {\n" +
		450	" Boolean b2;\n" +
		451	" if (b1 ? (b2 = Boolean.TRUE) : null)\n" +
		452	" return b2;\n" +
		453	" return false;\n" +
		454	" }\n" +
		455	" public static void main(String[] args) {\n" +
		456	" System.out.print(new Bug324178().foo(true));\n" +
		457	" }\n" +
		458	"}\n"
		459	},
		460	"----------\n" +
		461	"1. ERROR in Bug324178.java (at line 5)\n" +
462	" return b2;\n" +
463	" ^^\n" +
464	"The local variable b2 may not have been initialized\n" +
465	"----------\n");
466	}
396	public static Class testClass() {	467	public static Class testClass() {
397	return InitializationTests.class;	468	return InitializationTests.class;
398	}	469	}




 *     						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
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"			
		},
		"");
}

}

Return to bug 324178

Lines 16-21 Link Here

(-)src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java (-1 / +37 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	public void testBug324178a() {
		14227	this.runConformTest(
		14228	new String[] {
		14229	"Bug324178.java",
		14230	"public class Bug324178 {\n" +
		14231	" boolean b;\n" +
		14232	" void foo(Boolean u) {\n" +
		14233	" if (u == null) {}\n" +
		14234	" Boolean o;\n" +
		14235	" o = (u == null) ? Boolean.TRUE : u;\n" +
		14236	" o.toString(); // Incorrect potential NPE\n" +
		14237	" }\n" +
		14238	"}\n"
		14239	},
		14240	"");
		14241	}
		14242
14207	}	14243	}