The refactoring in fb2ae68 made it such that short-circuits are no longer (explicitly) re-tried, which can cause a short circuit to be detected when there isn't one (#73). This PR re-introduces that functionality, which eliminates the bug in my testing.
More accurately, what can happen is that in a call to stepAll, a short circuit occurs in an early step (which is saved in lastShortCircuit), but in a later iteration (after other values have been propagated), that short circuit goes away, but the outdated exception is still saved in lastShortCircuit and gets thrown at the end of the stepAll call. These changes fix the issue, as a short-circuit exception is now only thrown in step if there are no changes left to be propagated (which was the behavior before the changes in fb2ae68).
It's possible that the stuff with the shortCircuited set is actually needed (I think the && linksToUpdate.isEmpty() is sufficient), since I think the call to propagateSignal will cause the short-circuited component to get re-added to linksToUpdate, but I'm not 100% sure whether or not that's always true.
The refactoring in fb2ae68 made it such that short-circuits are no longer (explicitly) re-tried, which can cause a short circuit to be detected when there isn't one (#73). This PR re-introduces that functionality, which eliminates the bug in my testing.
More accurately, what can happen is that in a call to
stepAll
, a short circuit occurs in an early step (which is saved inlastShortCircuit
), but in a later iteration (after other values have been propagated), that short circuit goes away, but the outdated exception is still saved inlastShortCircuit
and gets thrown at the end of thestepAll
call. These changes fix the issue, as a short-circuit exception is now only thrown instep
if there are no changes left to be propagated (which was the behavior before the changes in fb2ae68).It's possible that the stuff with the
shortCircuited
set is actually needed (I think the&& linksToUpdate.isEmpty()
is sufficient), since I think the call topropagateSignal
will cause the short-circuited component to get re-added tolinksToUpdate
, but I'm not 100% sure whether or not that's always true.