From b7ada9e1dd28ca08c34aa18cddfa7471d52f3ac8 Mon Sep 17 00:00:00 2001
From: Tagir Valeev <Tagir.Valeev@jetbrains.com>
Date: Wed, 24 Jan 2018 16:32:25 +0700
Subject: [PATCH] DfaMemoryStateImpl#uniteClasses do not corrupt
 distinctClasses when returning false

In NaN handling memstate may still be alive even if classes to unite are already distinct. In this case we uniteClasses processing was incomplete resulting in corrupted memory state.
---
 .../dataFlow/DfaMemoryStateImpl.java          |  5 ++--
 .../LoopDoubleComparisonNotComplex.java       | 26 +++++++++++++++++++
 .../DataFlowInspectionTest.java               |  1 +
 3 files changed, 29 insertions(+), 3 deletions(-)
 create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/LoopDoubleComparisonNotComplex.java

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 f6517d93815c..a330e7dc48c0 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
@@ -519,6 +519,8 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
   }
 
   private boolean uniteClasses(int c1Index, int c2Index) {
+    if (!myDistinctClasses.unite(c1Index, c2Index)) return false;
+
     EqClass c1 = myEqClasses.get(c1Index);
     EqClass c2 = myEqClasses.get(c2Index);
 
@@ -555,9 +557,6 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
       addToMap(c, c1Index);
     }
 
-    if (!myDistinctClasses.unite(c1Index, c2Index)) {
-      return false;
-    }
     myEqClasses.set(c2Index, null);
     checkInvariants();
 
diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LoopDoubleComparisonNotComplex.java b/java/java-tests/testData/inspection/dataFlow/fixture/LoopDoubleComparisonNotComplex.java
new file mode 100644
index 000000000000..ab279ced5b59
--- /dev/null
+++ b/java/java-tests/testData/inspection/dataFlow/fixture/LoopDoubleComparisonNotComplex.java
@@ -0,0 +1,26 @@
+import java.util.List;
+
+class LoopDoubleComparisonNotComplex {
+  private static Node prob(List<Node> nodes) {
+    Node maxNode = null;
+    double maxProbability = 0.0;
+
+    for (Node node : nodes) {
+      double probability = probability(node);
+      if (probability > maxProbability) {
+        maxProbability = probability;
+        maxNode = node;
+      } else if (probability == maxProbability) {
+      }
+    }
+
+    return <warning descr="Expression 'maxNode' might evaluate to null but is returned by the method which is not declared as @Nullable">maxNode</warning>;
+  }
+
+  interface Node {}
+
+  static double probability(Node node) {
+    return 0.0;
+  }
+
+}
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 fbc7fb0c3d00..32d33a2e29b9 100644
--- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java
+++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java
@@ -585,4 +585,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
   public void testNullableGetterInLoop() { doTest(); }
   public void testNullabilityBasics() { doTest(); }
   public void testReassignedVarInLoop() { doTest(); }
+  public void testLoopDoubleComparisonNotComplex() { doTest(); }
 }