Download
Getting Started
Members
Projects
Community
Marketplace
Events
Planet Eclipse
Newsletter
Videos
Participate
Report a Bug
Forums
Mailing Lists
Wiki
IRC
How to Contribute
Working Groups
Automotive
Internet of Things
LocationTech
Long-Term Support
PolarSys
Science
OpenMDM
More
Community
Marketplace
Events
Planet Eclipse
Newsletter
Videos
Participate
Report a Bug
Forums
Mailing Lists
Wiki
IRC
How to Contribute
Working Groups
Automotive
Internet of Things
LocationTech
Long-Term Support
PolarSys
Science
OpenMDM
Toggle navigation
Bugzilla – Attachment 190447 Details for
Bug 324178
[null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
Log In
[x]
|
Terms of Use
|
Copyright Agent
[patch]
final patch
Bug_324178_v5.patch (text/plain), 14.74 KB, created by
Stephan Herrmann
on 2011-03-04 16:28:16 EST
(
hide
)
Description:
final patch
Filename:
MIME Type:
Creator:
Stephan Herrmann
Created:
2011-03-04 16:28:16 EST
Size:
14.74 KB
patch
obsolete
>### 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.99 >diff -u -r1.99 ConditionalExpression.java >--- compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java 4 Mar 2011 12:41:22 -0000 1.99 >+++ compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java 4 Mar 2011 21:15:33 -0000 >@@ -7,7 +7,10 @@ > * > * Contributors: > * IBM Corporation - initial API and implementation >- * Stephen Herrmann <stephan@cs.tu-berlin.de> - Contributions for bugs 133125, 292478 >+ * Stephen Herrmann <stephan@cs.tu-berlin.de> - 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) use the infos of both branches originating from the condition for computing the nullStatus: >+ computeNullStatus(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_OR_DEAD); >+ trueFlowTowardsTrue.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD); > } > if (isValueIfFalseOptimizedFalse) { >- falseInfoWhenTrue.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD); >+ falseFlowTowardsTrue.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD); > } > if (isValueIfTrueOptimizedTrue) { >- trueInfoWhenFalse.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD); >+ trueFlowTowardsFalse.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD); > } > if (isValueIfFalseOptimizedTrue) { >- falseInfoWhenFalse.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD); >+ falseFlowTowardsFalse.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD); > } > 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 computeNullStatus(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 21:15:36 -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.115 >diff -u -r1.115 NullReferenceTest.java >--- src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 4 Mar 2011 12:41:15 -0000 1.115 >+++ src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 4 Mar 2011 21:15:52 -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; > >@@ -14238,4 +14239,39 @@ > "Dead code\n" + > "----------\n"); > } >+// 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 324178
:
188486
|
189907
|
190328
|
190428
| 190447