From 78ecf28ff531a9d403bce8c375fdf54bd7d4e937 Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 21 Aug 2013 15:50:16 +0200 Subject: [PATCH] IDEA-112358 'Constant Conditions' does not understand non-short-circuit 'or' --- .../dataFlow/ControlFlowAnalyzer.java | 41 ++++++++++++++++--- .../dataFlow/boxingBoolean/expected.xml | 5 +++ .../fixture/LongCircuitOperations.java | 19 +++++++++ .../DataFlowInspectionTest.java | 1 + 4 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/LongCircuitOperations.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index bf0fc63be2a2..ca214aa5010c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -998,14 +998,20 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } PsiType type = expression.getType(); if (op == JavaTokenType.ANDAND) { - generateAndExpression(operands, type); + generateAndExpression(operands, type, true); } else if (op == JavaTokenType.OROR) { - generateOrExpression(operands, type); + generateOrExpression(operands, type, true); } else if (op == JavaTokenType.XOR && PsiType.BOOLEAN.equals(type)) { generateXorExpression(expression, operands, type); } + else if (op == JavaTokenType.AND && PsiType.BOOLEAN.equals(type)) { + generateAndExpression(operands, type, false); + } + else if (op == JavaTokenType.OR && PsiType.BOOLEAN.equals(type)) { + generateOrExpression(operands, type, false); + } else { generateOther(expression, op, operands, type); } @@ -1104,11 +1110,18 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } } - private void generateOrExpression(PsiExpression[] operands, final PsiType exprType) { + private void generateOrExpression(PsiExpression[] operands, final PsiType exprType, boolean shortCircuit) { for (int i = 0; i < operands.length; i++) { PsiExpression operand = operands[i]; operand.accept(this); generateBoxingUnboxingInstructionFor(operand, exprType); + if (!shortCircuit) { + if (i > 0) { + combineStackBooleans(false, operand); + } + continue; + } + PsiExpression nextOperand = i == operands.length - 1 ? null : operands[i + 1]; if (nextOperand != null) { addInstruction(new ConditionalGotoInstruction(getStartOffset(nextOperand), true, operand)); @@ -1125,7 +1138,11 @@ class ControlFlowAnalyzer extends JavaElementVisitor { lExpression.accept(this); generateBoxingUnboxingInstructionFor(lExpression, exprType); - ConditionalGotoInstruction toPopAndPushSuccess = new ConditionalGotoInstruction(-1, and, lExpression); + combineStackBooleans(and, lExpression); + } + + private void combineStackBooleans(boolean and, PsiExpression anchor) { + ConditionalGotoInstruction toPopAndPushSuccess = new ConditionalGotoInstruction(-1, and, anchor); addInstruction(toPopAndPushSuccess); GotoInstruction overPushSuccess = new GotoInstruction(-1); addInstruction(overPushSuccess); @@ -1140,17 +1157,29 @@ class ControlFlowAnalyzer extends JavaElementVisitor { overPushSuccess.setOffset(pushSuccess.getIndex() + 1); } - private void generateAndExpression(PsiExpression[] operands, final PsiType exprType) { + private void generateAndExpression(PsiExpression[] operands, final PsiType exprType, boolean shortCircuit) { List branchToFail = new ArrayList(); - for (PsiExpression operand : operands) { + for (int i = 0; i < operands.length; i++) { + PsiExpression operand = operands[i]; operand.accept(this); generateBoxingUnboxingInstructionFor(operand, exprType); + if (!shortCircuit) { + if (i > 0) { + combineStackBooleans(false, operand); + } + continue; + } + ConditionalGotoInstruction onFail = new ConditionalGotoInstruction(-1, true, operand); branchToFail.add(onFail); addInstruction(onFail); } + if (!shortCircuit) { + return; + } + addInstruction(new PushInstruction(myFactory.getConstFactory().getTrue(), null)); GotoInstruction toSuccess = new GotoInstruction(-1); addInstruction(toSuccess); diff --git a/java/java-tests/testData/inspection/dataFlow/boxingBoolean/expected.xml b/java/java-tests/testData/inspection/dataFlow/boxingBoolean/expected.xml index d04ea2815d43..eb4d42c5bf86 100644 --- a/java/java-tests/testData/inspection/dataFlow/boxingBoolean/expected.xml +++ b/java/java-tests/testData/inspection/dataFlow/boxingBoolean/expected.xml @@ -36,4 +36,9 @@ Condition <code>o</code> at the left side of assignment expression is always <code>false</code>. Can be simplified to normal assignment. + + Test.java + 52 + Condition <code>o</code> is always <code>true</code> + diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LongCircuitOperations.java b/java/java-tests/testData/inspection/dataFlow/fixture/LongCircuitOperations.java new file mode 100644 index 000000000000..e05c670be6af --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LongCircuitOperations.java @@ -0,0 +1,19 @@ +class X { + + int foo(String d1, String d2) { + if(d1 == null | d2 == null) + return 0; + return d1.compareTo(d2); + + } + void foo2(String d1, String d2) { + if(d1 == null & d1 != null) + System.out.println("impossible"); + + } + void foo3(String d1, String d2) { + if(d1 == null | d1.compareTo(d2) > 0) + System.out.println("impossible"); + } +} + diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index 6f9c7b089386..9ee62d0bb51b 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -286,4 +286,5 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase { public void testBoxingImpliesNotNull() { doTest(); } public void testLargeIntegersAreNotEqualWhenBoxed() { doTest(); } public void testNoGenericCCE() { doTest(); } + public void testLongCircuitOperations() { doTest(); } }