diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java index 8a4ac74..ed80071 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java @@ -47,7 +47,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[] { "test358827" }; +// TESTS_NAMES = new String[] { "testBug256796" }; // TESTS_NUMBERS = new int[] { 561 }; // TESTS_RANGE = new int[] { 1, 2049 }; } @@ -14978,4 +14978,150 @@ "----------\n"); } } +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=256796 +public void testBug256796() { + Map compilerOptions = getCompilerOptions(); + compilerOptions.put(CompilerOptions.OPTION_ReportUnnecessaryElse, CompilerOptions.IGNORE); + compilerOptions.put(CompilerOptions.OPTION_ReportDeadCode, CompilerOptions.WARNING); + compilerOptions.put(CompilerOptions.OPTION_ReportDeadCodeInTrivialIfStatement, CompilerOptions.DISABLED); + this.runNegativeTest( + new String[] { + "Bug.java", + "public class Bug {\n" + + " private static final boolean TRUE = true;\n" + + " private static final boolean FALSE = false;\n" + + " void foo() throws Exception {\n" + + " if (TRUE) return;\n" + + " else System.out.println(\"\");\n" + + " System.out.println(\"\");\n" + // not dead code + " if (TRUE) throw new Exception();\n" + + " else System.out.println(\"\");\n" + + " System.out.println(\"\");\n" + // not dead code + " if (TRUE) return;\n" + + " System.out.println(\"\");\n" + // not dead code + " if (FALSE) System.out.println(\"\");\n" + + " else return;\n" + + " System.out.println(\"\");\n" + // not dead code + " if (FALSE) return;\n" + + " System.out.println(\"\");\n" + // not dead code + " if (false) return;\n" + // dead code + " System.out.println(\"\");\n" + + " if (true) return;\n" + + " System.out.println(\"\");\n" + // dead code + " }\n" + + "}\n" + }, + "----------\n" + + "1. WARNING in Bug.java (at line 18)\n" + + " if (false) return;\n" + + " ^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "2. WARNING in Bug.java (at line 21)\n" + + " System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n", + null, + true, + compilerOptions); +} +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=256796 +public void testBug256796a() { + Map compilerOptions = getCompilerOptions(); + compilerOptions.put(CompilerOptions.OPTION_ReportUnnecessaryElse, CompilerOptions.IGNORE); + compilerOptions.put(CompilerOptions.OPTION_ReportDeadCode, CompilerOptions.WARNING); + compilerOptions.put(CompilerOptions.OPTION_ReportDeadCodeInTrivialIfStatement, CompilerOptions.ENABLED); + this.runNegativeTest( + new String[] { + "Bug.java", + "public class Bug {\n" + + " private static final boolean TRUE = true;\n" + + " private static final boolean FALSE = false;\n" + + " void foo() throws Exception {\n" + + " if (TRUE) return;\n" + + " else System.out.println(\"\");\n" + + " System.out.println(\"\");\n" + // dead code + " }\n" + + " void foo2() {\n" + + " if (TRUE) return;\n" + + " System.out.println(\"\");\n" + // dead code + " }\n" + + " void foo3() throws Exception {\n" + + " if (TRUE) throw new Exception();\n" + + " else System.out.println(\"\");\n" + // dead code + " System.out.println(\"\");\n" + // dead code + " }\n" + + " void foo4() throws Exception {\n" + + " if (FALSE) System.out.println(\"\");\n" + + " else return;\n" + + " System.out.println(\"\");\n" + // dead code + " }\n" + + " void foo5() throws Exception {\n" + + " if (FALSE) return;\n" + // dead code + " System.out.println(\"\");\n" + + " }\n" + + " void foo6() throws Exception {\n" + + " if (false) return;\n" + // dead code + " System.out.println(\"\");\n" + + " if (true) return;\n" + + " System.out.println(\"\");\n" + // dead code + " }\n" + + "}\n" + }, + "----------\n" + + "1. WARNING in Bug.java (at line 6)\n" + + " else System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "2. WARNING in Bug.java (at line 7)\n" + + " System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "3. WARNING in Bug.java (at line 11)\n" + + " System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "4. WARNING in Bug.java (at line 15)\n" + + " else System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "5. WARNING in Bug.java (at line 16)\n" + + " System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "6. WARNING in Bug.java (at line 19)\n" + + " if (FALSE) System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "7. WARNING in Bug.java (at line 21)\n" + + " System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "8. WARNING in Bug.java (at line 24)\n" + + " if (FALSE) return;\n" + + " ^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "9. WARNING in Bug.java (at line 28)\n" + + " if (false) return;\n" + + " ^^^^^^^\n" + + "Dead code\n" + + "----------\n" + + "10. WARNING in Bug.java (at line 31)\n" + + " System.out.println(\"\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^\n" + + "Dead code\n" + + "----------\n", + null, + true, + compilerOptions); +} } \ No newline at end of file diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java index 2b9c876..830fe22 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java @@ -86,11 +86,12 @@ // No need if the whole if-else construct itself lies in unreachable code this.bits |= ASTNode.IsElseStatementUnreachable; } + boolean reportDeadCodeForKnownPattern = !isKnowDeadCodePattern(this.condition) || currentScope.compilerOptions().reportDeadCodeInTrivialIfStatement; if (this.thenStatement != null) { // Save info for code gen this.thenInitStateIndex = currentScope.methodScope().recordInitializationStates(thenFlowInfo); if (isConditionOptimizedFalse || ((this.bits & ASTNode.IsThenStatementUnreachable) != 0)) { - if (!isKnowDeadCodePattern(this.condition) || currentScope.compilerOptions().reportDeadCodeInTrivialIfStatement) { + if (reportDeadCodeForKnownPattern) { this.thenStatement.complainIfUnreachable(thenFlowInfo, flowContext, currentScope, initialComplaintLevel, false); } else { // its a known coding pattern which should be tolerated by dead code analysis @@ -116,7 +117,7 @@ // Save info for code gen this.elseInitStateIndex = currentScope.methodScope().recordInitializationStates(elseFlowInfo); if (isConditionOptimizedTrue || ((this.bits & ASTNode.IsElseStatementUnreachable) != 0)) { - if (!isKnowDeadCodePattern(this.condition) || currentScope.compilerOptions().reportDeadCodeInTrivialIfStatement) { + if (reportDeadCodeForKnownPattern) { this.elseStatement.complainIfUnreachable(elseFlowInfo, flowContext, currentScope, initialComplaintLevel, false); } else { // its a known coding pattern which should be tolerated by dead code analysis @@ -136,7 +137,8 @@ isConditionOptimizedFalse, true /*if(true){ return; } fake-reachable(); */, flowInfo, - this); + this, + reportDeadCodeForKnownPattern); this.mergedInitStateIndex = currentScope.methodScope().recordInitializationStates(mergedInfo); return mergedInfo; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java index 5c995f9..2a7966a 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java @@ -405,12 +405,28 @@ public static UnconditionalFlowInfo mergedOptimizedBranchesIfElse( FlowInfo initsWhenTrue, boolean isOptimizedTrue, FlowInfo initsWhenFalse, boolean isOptimizedFalse, - boolean allowFakeDeadBranch, FlowInfo flowInfo, IfStatement ifStatement) { + boolean allowFakeDeadBranch, FlowInfo flowInfo, IfStatement ifStatement, + boolean reportDeadCodeInKnownPattern) { UnconditionalFlowInfo mergedInfo; if (isOptimizedTrue){ if (initsWhenTrue == FlowInfo.DEAD_END && allowFakeDeadBranch) { - mergedInfo = initsWhenFalse.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD). - unconditionalInits(); + if (!reportDeadCodeInKnownPattern) { + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=256796 + // do not report code even after if-else as dead as a consequence of analysis done in known dead code pattern + // when the CompilerOptions$reportDeadCodeInTrivialIfStatement option is disabled + if (ifStatement.elseStatement == null) { + mergedInfo = flowInfo.unconditionalInits(); + } else { + mergedInfo = initsWhenFalse.unconditionalInits(); + if (initsWhenFalse != FlowInfo.DEAD_END) { + // let the definitely true status of known dead code pattern not affect the reachability + mergedInfo.setReachMode(flowInfo.reachMode()); + } + } + } else { + mergedInfo = initsWhenFalse.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD). + unconditionalInits(); + } } else { mergedInfo = @@ -421,8 +437,23 @@ } else if (isOptimizedFalse) { if (initsWhenFalse == FlowInfo.DEAD_END && allowFakeDeadBranch) { - mergedInfo = initsWhenTrue.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD). - unconditionalInits(); + if (!reportDeadCodeInKnownPattern) { + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=256796 + // do not report code even after if-else as dead as a consequence of analysis done in known dead code pattern + // when the CompilerOptions$reportDeadCodeInTrivialIfStatement option is disabled + if (ifStatement.thenStatement == null) { + mergedInfo = flowInfo.unconditionalInits(); + } else { + mergedInfo = initsWhenTrue.unconditionalInits(); + if (initsWhenTrue != FlowInfo.DEAD_END) { + // let the definitely false status of known dead code pattern not affect the reachability + mergedInfo.setReachMode(flowInfo.reachMode()); + } + } + } else { + mergedInfo = initsWhenTrue.setReachMode(FlowInfo.UNREACHABLE_OR_DEAD). + unconditionalInits(); + } } else { mergedInfo =