Fix cycle checking when calculating min delay from nearest physical action

core/src/main/java/org/lflang/federated/generator/FederateInstance.java

@@ -25,14 +25,14 @@
  */
 package org.lflang.federated.generator;
 
-import com.google.common.base.Objects;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -44,6 +44,7 @@
 import org.lflang.generator.PortInstance;
 import org.lflang.generator.ReactionInstance;
 import org.lflang.generator.ReactorInstance;
+import org.lflang.generator.RuntimeRange;
 import org.lflang.generator.TriggerInstance;
 import org.lflang.lf.Action;
 import org.lflang.lf.ActionOrigin;
@@ -57,6 +58,7 @@
 import org.lflang.lf.Output;
 import org.lflang.lf.Parameter;
 import org.lflang.lf.ParameterReference;
+import org.lflang.lf.Port;
 import org.lflang.lf.Reaction;
 import org.lflang.lf.Reactor;
 import org.lflang.lf.ReactorDecl;
@@ -419,20 +421,20 @@ public boolean includes(Action action) {
         // Look in triggers
         for (TriggerRef trigger : convertToEmptyListIfNull(react.getTriggers())) {
           if (trigger instanceof VarRef triggerAsVarRef) {
-            if (Objects.equal(triggerAsVarRef.getVariable(), action)) {
+            if (Objects.equals(triggerAsVarRef.getVariable(), action)) {
               return true;
             }
           }
         }
         // Look in sources
         for (VarRef source : convertToEmptyListIfNull(react.getSources())) {
-          if (Objects.equal(source.getVariable(), action)) {
+          if (Objects.equals(source.getVariable(), action)) {
             return true;
           }
         }
         // Look in effects
         for (VarRef effect : convertToEmptyListIfNull(react.getEffects())) {
-          if (Objects.equal(effect.getVariable(), action)) {
+          if (Objects.equals(effect.getVariable(), action)) {
             return true;
           }
         }
@@ -494,7 +496,7 @@ public boolean includes(Timer timer) {
         // Look in triggers
         for (TriggerRef trigger : convertToEmptyListIfNull(r.getTriggers())) {
           if (trigger instanceof VarRef triggerAsVarRef) {
-            if (Objects.equal(triggerAsVarRef.getVariable(), timer)) {
+            if (Objects.equals(triggerAsVarRef.getVariable(), timer)) {
               return true;
             }
           }
@@ -682,7 +684,7 @@ public LinkedHashMap<Output, TimeValue> findOutputsConnectedToPhysicalActions(
     for (PortInstance output : instance.outputs) {
       for (ReactionInstance reaction : output.getDependsOnReactions()) {
         TimeValue minDelay = findNearestPhysicalActionTrigger(reaction);
-        if (!Objects.equal(minDelay, TimeValue.MAX_VALUE)) {
+        if (!Objects.equals(minDelay, TimeValue.MAX_VALUE)) {
           physicalActionToOutputMinDelay.put((Output) output.getDefinition(), minDelay);
         }
       }
@@ -707,6 +709,25 @@ public String toString() {
    *     otherwise
    */
   public TimeValue findNearestPhysicalActionTrigger(ReactionInstance reaction) {
+    Set<ReactionInstance> visited = new HashSet<>();
+    return findNearestPhysicalActionTriggerRecursive(visited, reaction);
+  }
+
+  /**
+   * Find the nearest (shortest) path to a physical action　trigger from this 'reaction' in terms of
+   * minimum delay by recursively visiting upstream triggers and reactions until a physical action
+   * is reached.
+   *
+   * @param visited A set of reactions that have been visited used to avoid deep loops
+   * @param reaction The reaction to start with
+   * @param pathMinDelay The amount of mininum delays accumulated from the output port to the
+   *     reaction (previous argument)
+   * @return The minimum delay found to the nearest physical action and TimeValue.MAX_VALUE
+   *     otherwise
+   */
+  private TimeValue findNearestPhysicalActionTriggerRecursive(
+      Set<ReactionInstance> visited, ReactionInstance reaction) {
+    visited.add(reaction); // Mark the reaction as visited.
     TimeValue minDelay = TimeValue.MAX_VALUE;
     for (TriggerInstance<? extends Variable> trigger : reaction.triggers) {
       if (trigger.getDefinition() instanceof Action action) {
@@ -719,24 +740,69 @@ public TimeValue findNearestPhysicalActionTrigger(ReactionInstance reaction) {
           // Logical action
           // Follow it upstream inside the reactor
           for (ReactionInstance uReaction : actionInstance.getDependsOnReactions()) {
-            // Avoid a loop
-            if (!Objects.equal(uReaction, reaction)) {
+            // Avoid a potentially deep loop by checking the visited set.
+            if (!visited.contains(uReaction)) {
               TimeValue uMinDelay =
-                  actionInstance.getMinDelay().add(findNearestPhysicalActionTrigger(uReaction));
+                  actionInstance
+                      .getMinDelay()
+                      .add(findNearestPhysicalActionTriggerRecursive(visited, uReaction));
               if (uMinDelay.isEarlierThan(minDelay)) {
                 minDelay = uMinDelay;
               }
             }
           }
         }
-
-      } else if (trigger.getDefinition() instanceof Output) {
-        // Outputs of contained reactions
-        PortInstance outputInstance = (PortInstance) trigger;
-        for (ReactionInstance uReaction : outputInstance.getDependsOnReactions()) {
-          TimeValue uMinDelay = findNearestPhysicalActionTrigger(uReaction);
-          if (uMinDelay.isEarlierThan(minDelay)) {
-            minDelay = uMinDelay;
+      } else if (trigger.getDefinition() instanceof Port) {
+        PortInstance port = (PortInstance) trigger;
+        // Regardless of whether the port is an output or input port of
+        // a contained reactor, recurse on reactions that write to it.
+        for (ReactionInstance uReaction : port.getDependsOnReactions()) {
+          // Ports can form a loop also.
+          if (!visited.contains(uReaction)) {
+            TimeValue uMinDelay = findNearestPhysicalActionTriggerRecursive(visited, uReaction);
+            if (uMinDelay.isEarlierThan(minDelay)) {
+              minDelay = uMinDelay;
+            }
+          }
+        }
+        // The trigger is an input of a contained reactor. If it
+        // another upstream contained reactor connects to it, trace to
+        // the upstream contained reactor's output ports.
+        if (trigger.getDefinition() instanceof Input) {
+          var upstreamPorts = port.eventualSources();
+          for (RuntimeRange<PortInstance> range : upstreamPorts) {
+            PortInstance upstreamPort = range.instance;
+
+            // Get a connection delay value between upstream port and
+            // the current port. This is currently done by finding
+            // the corresponding SendRange from the upstream port
+            // and then extracting a delay value from the connection
+            // contained in the SendRange.
+            // TODO: Find a better way to do this, which likely involves
+            // refactoring SendRange, RuntimeRange.Port, and PortInstance.
+            TimeValue connectionDelay =
+                upstreamPort.getDependentPorts().stream()
+                    .filter(
+                        sRange ->
+                            sRange.destinations.stream()
+                                .map(it -> it.instance)
+                                .anyMatch(port::equals))
+                    .map(sRange -> ASTUtils.getDelay(sRange.connection.getDelay()))
+                    .filter(Objects::nonNull)
+                    .map(TimeValue::fromNanoSeconds)
+                    .findFirst()
+                    .orElse(TimeValue.ZERO);
+
+            for (ReactionInstance uReaction : upstreamPort.getDependsOnReactions()) {
+              if (!visited.contains(uReaction)) {
+                TimeValue uMinDelay =
+                    connectionDelay.add(
+                        findNearestPhysicalActionTriggerRecursive(visited, uReaction));
+                if (uMinDelay.isEarlierThan(minDelay)) {
+                  minDelay = uMinDelay;
+                }
+              }
+            }
           }
         }
       }

test/C/src/federated/TriggerLoop.lf

@@ -0,0 +1,54 @@
+/**
+ * When there is an output in the federate reactor, the compiler checks if the output is
+ * (transitively) connected to a physical action inside the same reactor, as the physical action
+ * could result in generating a large number of NULL messages. As the compiler looks for the
+ * physical action recursively, it should be able to handle non-trivial "trigger loop" formed by a
+ * set of actions. This tests check if the compiler can successfully handle programs with such a
+ * pattern without getting stack overflow.
+ */
+target C {
+  timeout: 1 msec
+}
+
+reactor Simple {
+  // This output is required to trigger finding the closest upstream physical action inside the reactor.
+  output out: int
+  logical action a
+  logical action b
+
+  reaction(a) -> b {=  =}
+
+  reaction(b) -> a, out {=  =}
+}
+
+reactor Hard {
+  output out2: int
+  @label(
+      "We want to find THIS physical action from out2. The compiler should warn about min path delay = 2 sec.")
+  physical action d(1 sec)
+  @label("This physical action has a larger min delay. The compiler should ignore it.")
+  physical action e(10 sec)
+  @label("This physical is also not the one that leads to a minimal path.")
+  physical action f(2 sec)
+  delay = new Delay(width=2)
+  delay2 = new Delay(width=1)
+  delay.out -> delay2.in after 1 sec
+
+  reaction(d, delay2.out) -> delay.in {=  =}
+
+  reaction(e, delay2.out) -> d, out2 {=  =}
+
+  reaction(f) -> delay.in {=  =}
+}
+
+reactor Delay(width: int = 1) {
+  input[width] in: int
+  output out: int
+
+  reaction(in) -> out {=  =}
+}
+
+federated reactor {
+  simple = new Simple()
+  hard = new Hard()
+}