diff --git a/org.lflang/src/org/lflang/ASTUtils.java b/org.lflang/src/org/lflang/ASTUtils.java index f9de1f8083..8408a1b58e 100644 --- a/org.lflang/src/org/lflang/ASTUtils.java +++ b/org.lflang/src/org/lflang/ASTUtils.java @@ -27,14 +27,15 @@ package org.lflang; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; - -import com.google.common.collect.Iterables; -import com.google.common.collect.Iterators; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.ecore.resource.Resource; @@ -47,12 +48,11 @@ import org.eclipse.xtext.nodemodel.impl.HiddenLeafNode; import org.eclipse.xtext.nodemodel.util.NodeModelUtils; import org.eclipse.xtext.resource.XtextResource; -import org.eclipse.xtext.xbase.lib.CollectionExtensions; import org.eclipse.xtext.xbase.lib.IterableExtensions; import org.eclipse.xtext.xbase.lib.IteratorExtensions; import org.eclipse.xtext.xbase.lib.StringExtensions; -import org.lflang.generator.GeneratorBase; import org.lflang.generator.CodeMap; +import org.lflang.generator.GeneratorBase; import org.lflang.generator.InvalidSourceException; import org.lflang.lf.Action; import org.lflang.lf.ActionOrigin; @@ -84,6 +84,9 @@ import org.lflang.lf.WidthSpec; import org.lflang.lf.WidthTerm; +import com.google.common.collect.Iterables; +import com.google.common.collect.Iterators; + /** * A helper class for modifying and analyzing the AST. * @author{Marten Lohstroh } @@ -447,13 +450,7 @@ public static String getUniqueIdentifier(Reactor reactor, String name) { * @param definition Reactor class definition. */ public static List allActions(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allActions(toDefinition(base))); - } - result.addAll(definition.getActions()); - return result; + return ASTUtils.collectElements(definition, (Reactor r) -> r.getActions()); } /** @@ -462,28 +459,19 @@ public static List allActions(Reactor definition) { * @param definition Reactor class definition. */ public static List allConnections(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allConnections(toDefinition(base))); - } - result.addAll(definition.getConnections()); - return result; + return ASTUtils.collectElements(definition, (Reactor r) -> r.getConnections()); } /** * Given a reactor class, return a list of all its inputs, * which includes inputs of base classes that it extends. + * If the base classes include a cycle, where X extends Y and Y extends X, + * then return only the input defined in the base class. + * The returned list may be empty. * @param definition Reactor class definition. */ public static List allInputs(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allInputs(toDefinition(base))); - } - result.addAll(definition.getInputs()); - return result; + return ASTUtils.collectElements(definition, (Reactor r) -> r.getInputs()); } /** @@ -492,13 +480,7 @@ public static List allInputs(Reactor definition) { * @param definition Reactor class definition. */ public static List allInstantiations(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allInstantiations(toDefinition(base))); - } - result.addAll(definition.getInstantiations()); - return result; + return ASTUtils.collectElements(definition, (Reactor r) -> r.getInstantiations()); } /** @@ -507,13 +489,7 @@ public static List allInstantiations(Reactor definition) { * @param definition Reactor class definition. */ public static List allOutputs(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allOutputs(toDefinition(base))); - } - result.addAll(definition.getOutputs()); - return result; + return ASTUtils.collectElements(definition, (Reactor r) -> r.getOutputs()); } /** @@ -522,13 +498,7 @@ public static List allOutputs(Reactor definition) { * @param definition Reactor class definition. */ public static List allParameters(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allParameters(toDefinition(base))); - } - result.addAll(definition.getParameters()); - return result; + return ASTUtils.collectElements(definition, (Reactor r) -> r.getParameters()); } /** @@ -537,13 +507,7 @@ public static List allParameters(Reactor definition) { * @param definition Reactor class definition. */ public static List allReactions(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allReactions(toDefinition(base))); - } - result.addAll(definition.getReactions()); - return result; + return ASTUtils.collectElements(definition, (Reactor r) -> r.getReactions()); } /** @@ -552,13 +516,7 @@ public static List allReactions(Reactor definition) { * @param definition Reactor class definition. */ public static List allStateVars(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allStateVars(toDefinition(base))); - } - result.addAll(definition.getStateVars()); - return result; + return ASTUtils.collectElements(definition, (Reactor r) -> r.getStateVars()); } /** @@ -567,12 +525,43 @@ public static List allStateVars(Reactor definition) { * @param definition Reactor class definition. */ public static List allTimers(Reactor definition) { - List result = new ArrayList<>(); - List superClasses = convertToEmptyListIfNull(definition.getSuperClasses()); - for (ReactorDecl base : superClasses) { - result.addAll(allTimers(toDefinition(base))); + return ASTUtils.collectElements(definition, (Reactor r) -> r.getTimers()); + } + + /** + * Return all the superclasses of the specified reactor + * in deepest-first order. For example, if A extends B and C, and + * B and C both extend D, this will return the list [D, B, C, A]. + * Duplicates are removed. If the specified reactor does not extend + * any other reactor, then return an empty list. + * If a cycle is found, where X extends Y and Y extends X, or if + * a superclass is declared that is not found, then return null. + * @param reactor The specified reactor. + */ + public static LinkedHashSet superClasses(Reactor reactor) { + return superClasses(reactor, new LinkedHashSet()); + } + + /** + * Collect elements of type T from the class hierarchy defined by + * a given reactor definition. + * @param definition The reactor definition. + * @param elements A function that maps a reactor definition to a list of + * elements of type T. + * @param The type of elements to collect (e.g., Port, Timer, etc.) + * @return + */ + public static List collectElements(Reactor definition, Function> elements) { + List result = new ArrayList(); + // Add elements of elements defined in superclasses. + LinkedHashSet s = superClasses(definition); + if (s != null) { + for (Reactor superClass : s) { + result.addAll(elements.apply(superClass)); + } } - result.addAll(definition.getTimers()); + // Add elements of the current reactor. + result.addAll(elements.apply(definition)); return result; } @@ -894,7 +883,7 @@ public static String baseType(Type type) { /** * Report whether the given literal is zero or not. - * @param literalOrCode AST node to inspect. + * @param literal AST node to inspect. * @return True if the given literal denotes the constant `0`, false * otherwise. */ @@ -997,7 +986,7 @@ public static boolean isValidTime(Value value) { /** * Report whether the given time denotes a valid time or not. - * @param value AST node to inspect. + * @param t AST node to inspect. * @return True if the argument denotes a valid time, false otherwise. */ public static boolean isValidTime(Time t) { @@ -1010,7 +999,7 @@ public static boolean isValidTime(Time t) { /** * Report whether the given parameter denotes time list, meaning it is a list * of which all elements are valid times. - * @param value AST node to inspect. + * @param p AST node to inspect. * @return True if the argument denotes a valid time list, false otherwise. */ // TODO: why does this function always return true ??? @@ -1094,7 +1083,7 @@ public static boolean isValidTimeList(Parameter p) { * ``` * * @param parameter The parameter. - * @param instantiation The (optional) instantiation. + * @param instantiations The (optional) list of instantiations. * * @return The value of the parameter. * @@ -1161,7 +1150,7 @@ public static List initialValue(Parameter parameter, List * belongs to the specified instantiation, meaning that it is defined in * the reactor class being instantiated or one of its base classes. * @param eobject The object. - * @param instnatiation The instantiation. + * @param instantiation The instantiation. */ public static boolean belongsTo(EObject eobject, Instantiation instantiation) { Reactor reactor = toDefinition(instantiation.getReactorClass()); @@ -1173,7 +1162,7 @@ public static boolean belongsTo(EObject eobject, Instantiation instantiation) { * belongs to the specified reactor, meaning that it is defined in * reactor class or one of its base classes. * @param eobject The object. - * @param instnatiation The instantiation. + * @param reactor The reactor. */ public static boolean belongsTo(EObject eobject, Reactor reactor) { if (eobject.eContainer() == reactor) return true; @@ -1190,7 +1179,7 @@ public static boolean belongsTo(EObject eobject, Reactor reactor) { * if it does not have an integer value. * If the value of the parameter is a list of integers, * return the sum of value in the list. - * The instantiations parameter is as in + * The instantiations parameter is as in * {@link initialValue(Parameter, List)}. * * @param parameter The parameter. @@ -1490,7 +1479,7 @@ public static boolean isGeneric(Reactor r) { * return the imported reactor class definition. Otherwise, * just return the argument. * @param r A Reactor or an ImportedReactor. - * @return The Reactor class definition. + * @return The Reactor class definition or null if no definition is found. */ public static Reactor toDefinition(ReactorDecl r) { if (r == null) @@ -1640,11 +1629,43 @@ public static TargetDecl targetDecl(Model model) { public static TargetDecl targetDecl(Resource model) { return IteratorExtensions.head(Iterators.filter(model.getAllContents(), TargetDecl.class)); } - + + ///////////////////////////////////////////////////////// + //// Private methods + /** * Returns the list if it is not null. Otherwise return an empty list. */ private static List convertToEmptyListIfNull(List list) { return list != null ? list : new ArrayList<>(); } + + /** + * Return all the superclasses of the specified reactor + * in deepest-first order. For example, if A extends B and C, and + * B and C both extend D, this will return the list [D, B, C, A]. + * Duplicates are removed. If the specified reactor does not extend + * any other reactor, then return an empty list. + * If a cycle is found, where X extends Y and Y extends X, or if + * a superclass is declared that is not found, then return null. + * @param reactor The specified reactor. + * @param extensions A set of reactors extending the specified reactor + * (used to detect circular extensions). + */ + private static LinkedHashSet superClasses(Reactor reactor, Set extensions) { + LinkedHashSet result = new LinkedHashSet(); + for (ReactorDecl superDecl : convertToEmptyListIfNull(reactor.getSuperClasses())) { + Reactor r = toDefinition(superDecl); + if (r == reactor || r == null) return null; + // If r is in the extensions, then we have a circular inheritance structure. + if (extensions.contains(r)) return null; + extensions.add(r); + LinkedHashSet baseExtends = superClasses(r, extensions); + extensions.remove(r); + if (baseExtends == null) return null; + result.addAll(baseExtends); + result.add(r); + } + return result; + } } diff --git a/org.lflang/src/org/lflang/validation/LFValidator.java b/org.lflang/src/org/lflang/validation/LFValidator.java index 20770e265c..afbe0feefd 100644 --- a/org.lflang/src/org/lflang/validation/LFValidator.java +++ b/org.lflang/src/org/lflang/validation/LFValidator.java @@ -40,6 +40,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -53,6 +54,7 @@ import org.eclipse.xtext.validation.Check; import org.eclipse.xtext.validation.CheckType; import org.eclipse.xtext.validation.ValidationMessageAcceptor; +import org.lflang.ASTUtils; import org.lflang.FileConfig; import org.lflang.JavaAstUtils; import org.lflang.ModelInfo; @@ -824,6 +826,15 @@ public void checkReaction(Reaction reaction) { @Check(CheckType.FAST) public void checkReactor(Reactor reactor) throws IOException { + Set superClasses = ASTUtils.superClasses(reactor); + if (superClasses == null) { + error( + "Problem with superclasses: Either they form a cycle or are not defined", + Literals.REACTOR_DECL__NAME + ); + // Continue checks, but without any superclasses. + superClasses = new LinkedHashSet<>(); + } String name = FileConfig.nameWithoutExtension(reactor.eResource()); if (reactor.getName() == null) { if (!reactor.isFederated() && !reactor.isMain()) { @@ -900,18 +911,17 @@ public void checkReactor(Reactor reactor) throws IOException { variables.addAll(reactor.getTimers()); // Perform checks on super classes. - EList superClasses = reactor.getSuperClasses() != null ? reactor.getSuperClasses() : new BasicEList<>(); - for (ReactorDecl superClass : superClasses) { + for (Reactor superClass : superClasses) { HashSet conflicts = new HashSet<>(); // Detect input conflicts - checkConflict(toDefinition(superClass).getInputs(), reactor.getInputs(), variables, conflicts); + checkConflict(superClass.getInputs(), reactor.getInputs(), variables, conflicts); // Detect output conflicts - checkConflict(toDefinition(superClass).getOutputs(), reactor.getOutputs(), variables, conflicts); + checkConflict(superClass.getOutputs(), reactor.getOutputs(), variables, conflicts); // Detect output conflicts - checkConflict(toDefinition(superClass).getActions(), reactor.getActions(), variables, conflicts); + checkConflict(superClass.getActions(), reactor.getActions(), variables, conflicts); // Detect conflicts - for (Timer timer : toDefinition(superClass).getTimers()) { + for (Timer timer : superClass.getTimers()) { List filteredVariables = new ArrayList<>(variables); filteredVariables.removeIf(it -> reactor.getTimers().contains(it)); if (hasNameConflict(timer, filteredVariables)) { @@ -928,7 +938,8 @@ public void checkReactor(Reactor reactor) throws IOException { names.add(it.getName()); } error( - String.format("Cannot extend %s due to the following conflicts: %s.", superClass.getName(), String.join(",", names)), + String.format("Cannot extend %s due to the following conflicts: %s.", + superClass.getName(), String.join(",", names)), Literals.REACTOR__SUPER_CLASSES ); } @@ -1322,8 +1333,9 @@ private int countMainOrFederated(TreeIterator iter) { * instantiation cycle. * @param visited The set of nodes already visited in this graph traversal. */ - private boolean dependsOnCycle(Reactor reactor, Set cycleSet, - Set visited) { + private boolean dependsOnCycle( + Reactor reactor, Set cycleSet, Set visited + ) { Set origins = info.instantiationGraph.getUpstreamAdjacentNodes(reactor); if (visited.contains(reactor)) { return false; diff --git a/test/C/src/RepeatedInheritance.lf b/test/C/src/RepeatedInheritance.lf new file mode 100644 index 0000000000..9081eabaa9 --- /dev/null +++ b/test/C/src/RepeatedInheritance.lf @@ -0,0 +1,37 @@ +/** + * This tests for the situation where a reactor extends two other reactors + * that each extend a common reactor. + * @author{Edward A. Lee} + */ +target C { + timeout: 5 sec, + fast: true +} + +import Count from "lib/Count.lf"; +import TestCount from "lib/TestCount.lf"; + +reactor D { + input d:int; +} +reactor C extends D { + input c:int; +} +reactor B extends D { + input b:int; +} +reactor A extends B, C { + input a:int; + output out:int; + reaction(a, b, c, d) -> out {= + SET(out, a->value + b->value + c->value + d->value); + =} +} + +main reactor { + c = new Count(); + a = new A(); + t = new TestCount(start = 4, stride = 4, num_inputs = 6); + (c.out)+ -> a.a, a.b, a.c, a.d; + a.out -> t.in; +}