Fix handling of postponed matches (match order comparison was slightly wrong)

reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/MatchJournal.kt

@@ -95,7 +95,7 @@ interface MatchJournal : MutableIterable<MatchJournal.Chunk> {
     data class OccurrencePos(val occ: Occurrence, private val chunk: MatchJournal.Chunk, private val offset: Int): MatchJournal.Pos()
     {
         companion object {
-            fun get(index: MatchJournal.Index, occ: Occurrence): OccurrencePos? {
+            fun fromIndex(index: MatchJournal.Index, occ: Occurrence): OccurrencePos? {
                 val idOcc = Id(occ)
                 val chunk = index.activatingChunkOf(idOcc) ?: return null
                 val i = chunk.entriesLog().indexOfFirst { entry ->

reactor/Core/src/jetbrains/mps/logic/reactor/core/internal/ProcessingStateImpl.kt

@@ -116,6 +116,11 @@ internal class ProcessingStateImpl(private var dispatchingFront: Dispatcher.Disp
 
     private val journalIndex: MatchJournal.Index = journal.index()
 
+    // It is a position in Journal from previous session,
+    //  from which incremental execution continues.
+    // Needed for pos comparison in handleFutureMatches.
+    private lateinit var lastIncrementalRootPos: MatchJournal.Pos
+
     private val postponedMatches: MutableMap<Id<Occurrence>, List<RuleMatchEx>> = HashMap()
 
     private val execQueue: Queue<ExecPos> =
@@ -281,6 +286,7 @@ internal class ProcessingStateImpl(private var dispatchingFront: Dispatcher.Disp
                 //  Then shouldn't replay, because currentPos is valid and more recent (!) than execPos.
                 if (execPos != prevPos)
                     replay(controller, execPos.pos)
+                    lastIncrementalRootPos = execPos.pos
                 prevPos = execPos
 
                 // If the occurrence is still in the store after replay (i.e. if it's valid to activate it)
@@ -325,8 +331,8 @@ internal class ProcessingStateImpl(private var dispatchingFront: Dispatcher.Disp
             val matches = dispatchingFront.matches().toList()
             val currentMatches =
                 // todo: assert that there can be no future matches on non-principal occurrences?
-//                if (isCurrent() || !active.isPrincipal()) {
-                if (true) {
+                if (isCurrent() || !active.isPrincipal()) {
+//                if (true) {
                     matches
                 } else {
                     val newCurrentMatches = handleFutureMatches(matches)
@@ -358,29 +364,29 @@ internal class ProcessingStateImpl(private var dispatchingFront: Dispatcher.Disp
 
     // Determines, filters out and enqueues (to execution queue) future matches. Returns only current matches.
     private fun handleFutureMatches(matches: List<RuleMatchEx>): List<RuleMatchEx> {
-        val currentPos = currentPos()
         val currentMatches = mutableListOf<RuleMatchEx>()
 
         for (m in matches) {
-            latestMatchedOccurrence(m)?.let { pos ->
-                // if it is a future match
-                if (journalIndex.compare(currentPos, pos) > 0) {
-                    val idOcc = Id(pos.occ)
-                    postponedMatches[idOcc] = (postponedMatches[idOcc] ?: emptyList()) + listOf(m)
-                    execQueue.offer(ExecPos(pos, pos.occ))
-                } else {
-                    currentMatches.add(m)
-                }
+            val pos = latestMatchedOccurrence(m)
+            // if it is a future match
+            if (pos != null && journalIndex.compare(lastIncrementalRootPos, pos) < 0) {
+                val idOcc = Id(pos.occ)
+                postponedMatches[idOcc] = (postponedMatches[idOcc] ?: emptyList()) + listOf(m)
+                execQueue.offer(ExecPos(pos, pos.occ))
+            } else {
+                currentMatches.add(m)
             }
         }
         return currentMatches
     }
 
     // The latest matched occurrence from match's head is (by definition) the occurrence which activated this match.
+    // Returns null for matches with occurrences only from this session
+    //  (because journalIndex indexes only previous session).
     private fun latestMatchedOccurrence(match: RuleMatchEx): MatchJournal.OccurrencePos? =
         match.signature().mapNotNull { occSig ->
             occSig?.wrapped?.let { occ ->
-                MatchJournal.OccurrencePos.get(journalIndex, occ)
+                MatchJournal.OccurrencePos.fromIndex(journalIndex, occ)
             }
         }.maxWith(journalIndex) // compare positions: find latest
 

reactor/Test/test/TestIncrementalProgram.kt

@@ -219,7 +219,6 @@ class TestIncrementalProgram {
                 ))
         ).launch("addRule", progSpec) { result ->
 
-            println(result.token().journalView)
             result.storeView().constraintSymbols() shouldBe setOf(sym0("foo"), sym0("main"))
             result.token().journalView.chunks.size shouldBe 2
             result.lastChunk().activatedLog().constraintSymbols() shouldBe listOf(sym0("foo"))
@@ -236,7 +235,6 @@ class TestIncrementalProgram {
                 )
             ).relaunch("atStart", progSpec, evalRes.token()) { result ->
 
-                println(result.token().journalView)
                 result.storeView().constraintSymbols() shouldBe setOf(sym0("foo"), sym0("main"), sym0("bar"))
                 result.token().journalView.chunks.size shouldBe 3
                 result.lastChunk().activatedLog().constraintSymbols() shouldBe listOf(sym0("bar"))
@@ -263,6 +261,8 @@ class TestIncrementalProgram {
             result.storeView().constraintSymbols() shouldBe setOf(sym0("foo"))
 
         }.also { (builder, evalRes) ->
+            val nPrincipalMatches = evalRes.token().journalView.chunks.size
+
             builder.programWithRules(
                 rule("main.bar",
                     headKept(
@@ -275,10 +275,12 @@ class TestIncrementalProgram {
             ).relaunch("test1", progSpec, evalRes.token()) { result ->
                 // no new matches: 'main' has been already discarded
                 result.storeView().constraintSymbols() shouldBe setOf(sym0("foo"))
+                result.token().journalView.chunks.size shouldBe nPrincipalMatches
             }
         }
     }
 
+    // Tests that incremental reactivation of discarded occurrences is handled correctly.
     @Test
     fun addMatchBeforeReplaced() {
         val progSpec = MockIncrProgSpec(
@@ -314,6 +316,8 @@ class TestIncrementalProgram {
             result.lastChunk().activatedLog().constraintSymbols() shouldBe listOf(sym0("dummy"))
 
         }.also { (builder, evalRes) ->
+            val nPrincipalMatches = evalRes.token().journalView.chunks.size
+
             // Insert rule before foo.baz: produce bar before discarding foo in foo.baz
             builder.insertRulesAt(1,
                 rule("foo.bar",
@@ -327,6 +331,7 @@ class TestIncrementalProgram {
 
                 result.storeView().constraintSymbols() shouldBe setOf(sym0("bar"), sym0("baz"), sym0("dummy"))
                 result.lastChunk().activatedLog().constraintSymbols() shouldBe listOf(sym0("dummy"))
+                result.token().journalView.chunks.size shouldBe (1 + nPrincipalMatches)
             }
         }
     }
@@ -365,6 +370,8 @@ class TestIncrementalProgram {
             result.storeView().constraintSymbols() shouldBe setOf(sym0("bar"), sym0("qux"))
 
         }.also { (builder, evalRes) ->
+            val nPrincipalMatches = evalRes.token().journalView.chunks.size
+
             builder.insertRulesAt(2,
                 rule("foo.baz",
                     headKept(
@@ -376,6 +383,7 @@ class TestIncrementalProgram {
             ).relaunch("test1", progSpec, evalRes.token()) { result ->
 
                 result.storeView().constraintSymbols() shouldBe setOf(sym0("bar"), sym0("baz"), sym0("qux"))
+                result.token().journalView.chunks.size shouldBe (1 + nPrincipalMatches)
             }
         }
     }
@@ -438,4 +446,69 @@ class TestIncrementalProgram {
             }
         }
     }
+
+    // Description: due to incremental launch, match on 'foobar' will be known
+    //  before it should actually happen. If it happens earlier than needed,
+    //  'bar' will be discarded too early and program results will be incorrect.
+    @Test
+    fun futureMatchInDueTime() {
+        val progSpec = MockIncrProgSpec(
+            setOf(".foo", ".bar", "foobar", "bar.1st"),
+            setOf(sym0("start"), sym0("foo"), sym0("bar"))
+        )
+        programWithRules(
+            rule("main",
+                headReplaced(
+                    constraint("main")
+                ),
+                body(
+                    princConstraint("start")
+                )),
+            rule(".foo",
+                headKept(
+                    princConstraint("start")
+                ),
+                body(
+                    princConstraint("foo")
+                )),
+            rule("foobar",
+                headReplaced(
+                    princConstraint("foo"),
+                    princConstraint("bar")
+                ),
+                body(
+                    constraint("2nd")
+                )),
+            rule("bar.1st",
+                headKept(
+                    princConstraint("bar")
+                ),
+                body(
+                    constraint("1st")
+                )
+            )
+        ).launch("withoutBar", progSpec) { result ->
+
+            // "foobar" hasn't matched without 'bar, so 'foo' is still here
+            result.storeView().constraintSymbols() shouldBe setOf(sym0("start"), sym0("foo"))
+
+        }.also { (builder, evalRes) ->
+            builder.insertRulesAt(1,
+                rule(".bar",
+                    headKept(
+                        princConstraint("start")
+                    ),
+                    body(
+                        princConstraint("bar")
+                    ))
+            ).relaunch("withBar", progSpec, evalRes.token()) { result ->
+
+                // if "foobar" happens too early, "1st" occ won't be produced
+                result.storeView().constraintSymbols() shouldBe setOf(sym0("start"), sym0("1st"), sym0("2nd"))
+                // ensure right rule match order: the last chunk must contain "2nd"
+                result.lastChunk().activatedLog().constraintSymbols() shouldBe listOf(sym0("2nd"))
+            }
+        }
+    }
+
 }

reactor/Test/test/TestStoreAwareJournal.kt

@@ -67,7 +67,7 @@ class TestStoreAwareJournal {
         {
             with(JournalDispatcherHelper(Dispatcher(RuleIndex(rulesLists)))) {
 
-                hist.justs() shouldBe justsOf()
+                hist.justs() shouldBe justsOf(0)
 
                 logExpand(justifiedOccurrence("foo", setOf(1)))
                 val fooMatches = d.matches()