diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index 0d7ed7792bad..6b59fe978b01 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -605,7 +605,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { return false; } - return myDistinctClasses.areDistinct(c1Index, c2Index); + return myDistinctClasses.areDistinctUnordered(c1Index, c2Index); } @Override @@ -898,8 +898,12 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } } - if (!applyRelation(dfaLeft, dfaRight, isNegated)) { - return false; + if (dfaRelation.getRelation() == RelationType.LT) { + if (!applyLessThanRelation(dfaLeft, dfaRight)) return false; + } else if (dfaRelation.getRelation() == RelationType.GT) { + if (!applyLessThanRelation(dfaRight, dfaLeft)) return false; + } else { + if (!applyRelation(dfaLeft, dfaRight, isNegated)) return false; } if (!checkCompareWithBooleanLiteral(dfaLeft, dfaRight, isNegated)) { return false; @@ -986,13 +990,31 @@ public class DfaMemoryStateImpl implements DfaMemoryState { else { // Not Equals if (c1Index.equals(c2Index) || areCompatibleConstants(c1Index, c2Index)) return false; if (isNull(dfaLeft) && isPrimitive(dfaRight) || isNull(dfaRight) && isPrimitive(dfaLeft)) return true; - myDistinctClasses.add(c1Index, c2Index); + myDistinctClasses.add(c1Index, c2Index, false); } myCachedHash = null; return true; } + private boolean applyLessThanRelation(@NotNull final DfaValue dfaLeft, @NotNull final DfaValue dfaRight) { + if (isUnknownState(dfaLeft) || isUnknownState(dfaRight)) { + return true; + } + + // DfaConstValue || DfaVariableValue + Integer c1Index = getOrCreateEqClassIndex(dfaLeft); + Integer c2Index = getOrCreateEqClassIndex(dfaRight); + if (c1Index == null || c2Index == null) { + return true; + } + + if (c1Index.equals(c2Index) || areCompatibleConstants(c1Index, c2Index)) return false; + if (isNull(dfaLeft) && isPrimitive(dfaRight) || isNull(dfaRight) && isPrimitive(dfaLeft)) return true; + myCachedHash = null; + return myDistinctClasses.add(c1Index, c2Index, true); + } + /** * Returns true if value represents an "unstable" value. An unstable value is a value of an object type which could be * a newly object every time it's accessed. Such value is still useful as its nullability is stable @@ -1215,7 +1237,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (eqClass.findConstant(true) != null) return true; for (DistinctPairSet.DistinctPair pair : getDistinctClassPairs()) { - EqClass otherClass = pair.getOtherClass(eqClass); + EqClass otherClass = pair.getOtherClass(eqClassIndex); if (otherClass != null && otherClass.findConstant(true) != null) { return true; } @@ -1305,7 +1327,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { for (Iterator iterator = myDistinctClasses.iterator(); iterator.hasNext(); ) { DistinctPairSet.DistinctPair pair = iterator.next(); - if (pair.getOtherClass(varClassIndex) != -1) { + if (pair.getOtherClass(varClassIndex) != null) { iterator.remove(); } } @@ -1313,8 +1335,8 @@ public class DfaMemoryStateImpl implements DfaMemoryState { else if (varClass.containsConstantsOnly()) { for (Iterator iterator = myDistinctClasses.iterator(); iterator.hasNext(); ) { DistinctPairSet.DistinctPair pair = iterator.next(); - int other = pair.getOtherClass(varClassIndex); - if (other != -1 && myEqClasses.get(other).containsConstantsOnly()) { + EqClass other = pair.getOtherClass(varClassIndex); + if (other != null && other.containsConstantsOnly()) { iterator.remove(); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DistinctPairSet.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DistinctPairSet.java index 485208bc5b46..debf347cda2e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DistinctPairSet.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DistinctPairSet.java @@ -28,23 +28,43 @@ final class DistinctPairSet extends AbstractSet { other.myData.forEach(myData::add); } - void add(int firstIndex, int secondIndex) { - myData.add(createPair(firstIndex, secondIndex)); + boolean add(int firstIndex, int secondIndex, boolean ordered) { + if (ordered) { + TLongHashSet toAdd = new TLongHashSet(); + toAdd.add(createPair(firstIndex, secondIndex, true)); + for(DistinctPair pair : this) { + if (!pair.isOrdered()) continue; + if (pair.myFirst == secondIndex) { + if (pair.mySecond == firstIndex || myData.contains(createPair(pair.mySecond, firstIndex, true))) return false; + toAdd.add(createPair(firstIndex, pair.mySecond, true)); + } else if (pair.mySecond == firstIndex) { + if (myData.contains(createPair(secondIndex, pair.myFirst, true))) return false; + toAdd.add(createPair(pair.myFirst, secondIndex, true)); + } + } + myData.addAll(toAdd.toArray()); + } else { + if (!myData.contains(createPair(firstIndex, secondIndex, true)) && + !myData.contains(createPair(secondIndex, firstIndex, true))) { + myData.add(createPair(firstIndex, secondIndex, false)); + } + } + return true; } @Override public boolean contains(Object o) { - if(!(o instanceof DistinctPair)) return false; + if (!(o instanceof DistinctPair)) return false; DistinctPair dp = (DistinctPair)o; EqClass first = dp.getFirst(); EqClass second = dp.getSecond(); - if(first.isEmpty() || second.isEmpty()) return false; + if (first.isEmpty() || second.isEmpty()) return false; int firstVal = first.get(0); int secondVal = second.get(0); int firstIndex = myState.getEqClassIndex(myState.getFactory().getValue(firstVal)); int secondIndex = myState.getEqClassIndex(myState.getFactory().getValue(secondVal)); if (firstIndex == -1 || secondIndex == -1) return false; - long pair = createPair(firstIndex, secondIndex); + long pair = createPair(firstIndex, secondIndex, dp.isOrdered()); return myData.contains(pair) && decode(pair).equals(dp); } @@ -103,51 +123,54 @@ final class DistinctPairSet extends AbstractSet { for (int i = 0; i < c2Pairs.size(); i++) { long c = c2Pairs.get(i); myData.remove(c); - myData.add(createPair(c1Index, low(c) == c2Index ? high(c) : low(c))); + if (c >= 0) { + myData.add(createPair(c1Index, low(c) == c2Index ? high(c) : low(c), false)); + } + else if (low(c) == c2Index) { + myData.add(createPair(c1Index, high(c), true)); + } + else { + myData.add(createPair(low(c), c1Index, true)); + } } return true; } - public boolean areDistinct(int c1Index, int c2Index) { - long pair = createPair(c1Index, c2Index); - return myData.contains(pair); + public boolean areDistinctUnordered(int c1Index, int c2Index) { + return myData.contains(createPair(c1Index, c2Index, false)); } private DistinctPair decode(long encoded) { - return new DistinctPair(low(encoded), high(encoded), myState.getEqClasses()); + boolean ordered = encoded < 0; + encoded = Math.abs(encoded); + return new DistinctPair(low(encoded), high(encoded), ordered, myState.getEqClasses()); } - private static long createPair(int i1, int i2) { - if (i1 < i2) { - long l = i1; - l <<= 32; - l += i2; - return l; - } - else { - long l = i2; - l <<= 32; - l += i1; - return l; + private static long createPair(int low, int high, boolean ordered) { + if (ordered) { + return -(((long)high << 32) + low); } + return low < high ? ((long)low << 32) + high : ((long)high << 32) + low; } private static int low(long l) { - return (int)l; + return (int)(Math.abs(l)); } private static int high(long l) { - return (int)((l & 0xFFFFFFFF00000000L) >> 32); + return (int)((Math.abs(l) & 0xFFFFFFFF00000000L) >> 32); } static final class DistinctPair { private final int myFirst; private final int mySecond; + private final boolean myOrdered; private List myList; - private DistinctPair(int first, int second, List list) { + private DistinctPair(int first, int second, boolean ordered, List list) { myFirst = first; mySecond = second; + myOrdered = ordered; myList = list; } @@ -161,22 +184,16 @@ final class DistinctPairSet extends AbstractSet { return myList.get(mySecond); } - public int getOtherClass(int eqClassIndex) { - if(myFirst == eqClassIndex) { - return mySecond; - } - if(mySecond == eqClassIndex) { - return myFirst; - } - return -1; + public boolean isOrdered() { + return myOrdered; } @Nullable - EqClass getOtherClass(EqClass eqClass) { - if (getFirst() == eqClass) { + public EqClass getOtherClass(int eqClassIndex) { + if (myFirst == eqClassIndex) { return getSecond(); } - if (getSecond() == eqClass) { + if (mySecond == eqClassIndex) { return getFirst(); } return null; @@ -187,18 +204,19 @@ final class DistinctPairSet extends AbstractSet { if (obj == this) return true; if (!(obj instanceof DistinctPair)) return false; DistinctPair that = (DistinctPair)obj; + if (that.myOrdered != this.myOrdered) return false; return that.getFirst().equals(this.getFirst()) && that.getSecond().equals(this.getSecond()) || - that.getSecond().equals(this.getFirst()) && that.getFirst().equals(this.getSecond()); + (!myOrdered && that.getSecond().equals(this.getFirst()) && that.getFirst().equals(this.getSecond())); } @Override public int hashCode() { - return getFirst().hashCode() + getSecond().hashCode(); + return getFirst().hashCode() * (myOrdered ? 31 : 1) + getSecond().hashCode(); } @Override public String toString() { - return "{" + myFirst + ", " + mySecond + "}"; + return "{" + myFirst + (myOrdered ? "<" : "!=") + mySecond + "}"; } } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LessThanRelations.java b/java/java-tests/testData/inspection/dataFlow/fixture/LessThanRelations.java new file mode 100644 index 000000000000..d04701779166 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LessThanRelations.java @@ -0,0 +1,44 @@ +import java.util.*; + +class LessThanRelations { + // IDEA-184278 + void m(int value) { + for (int i = 0; i < value; i++) { + if (i < value) System.out.println(); + } + } + + void transitive(int a, int b, int c) { + if(a > b && b >= c) { + System.out.println("possible"); + if(c == a) { + System.out.println("Impossible"); + } + if(c > a) { + System.out.println("Impossible"); + } + } + } + + void aioobe(Object[] arr, int pos) { + if(pos >= arr.length) { + System.out.println(arr[pos]); + } + } + + void wrongOrder(Object[] arr, Object e) { + int index = 0; + while(arr[index] != e && index < arr.length) { + index++; + } + System.out.println(index); + } + + void list(List list, int index) { + if(index >= list.size()) { + System.out.println("Big index"); + } else if(index > 0 && !list.isEmpty()) { + System.out.println("ok"); + } + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NotGreaterIsNotEquals.java b/java/java-tests/testData/inspection/dataFlow/fixture/NotGreaterIsNotEquals.java index 6528fcd5396d..09c650e7c852 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/NotGreaterIsNotEquals.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NotGreaterIsNotEquals.java @@ -23,7 +23,7 @@ class Zoo2 { void foo(Some me, Some other) { if (me.depth < other.depth) { System.out.println("less"); - } else if (other.depth > me.depth) { + } else if (other.depth > me.depth) { System.out.println("more"); } } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index ec897e420dda..c1fcf7f76ffc 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -580,4 +580,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testRedundantAssignment() { doTest(); } public void testXorNullity() { doTest(); } public void testPrimitiveNull() { doTest(); } + public void testLessThanRelations() { doTest(); } }