Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More natural syntax for reaction declarations #1853

Merged
merged 19 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cli/lfc/src/test/resources/org/lflang/cli/issue490.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ lfc: error: Name of main reactor must match the file name (or be omitted).
| ^ Name of main reactor must match the file name (or be omitted).
|
5 | state liss(2, 3);
lfc: error: The Python target does not support reaction declarations. Please specify a reaction body.
--> %%%PATH.lf%%%:6:3
|
5 | state liss(2, 3);
6 | reaction (startup) {
| ^^^^^^^^^^^^^^^^^^^^ The Python target does not support reaction declarations. Please specify a reaction body.
|
7 | print(self.liss)
lfc: error: no viable alternative at input '{'
--> %%%PATH.lf%%%:6:22
|
Expand Down
7 changes: 4 additions & 3 deletions core/src/main/java/org/lflang/LinguaFranca.xtext
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,11 @@ Action:
Reaction:
(attributes+=Attribute)*
(('reaction') | mutation ?= 'mutation')
(name=ID)?
('(' (triggers+=TriggerRef (',' triggers+=TriggerRef)*)? ')')
(sources+=VarRef (',' sources+=VarRef)*)?
( => sources+=VarRef (',' sources+=VarRef)*)?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to tell ANTLR to always prefer going down this path rather than use the Connection production.

('->' effects+=VarRefOrModeTransition (',' effects+=VarRefOrModeTransition)*)?
((('named' name=ID)? code=Code) | 'named' name=ID)(stp=STP)?(deadline=Deadline)?
(code=Code)? (stp=STP)? (deadline=Deadline)? (delimited?=';')?
;

TriggerRef:
Expand Down Expand Up @@ -511,7 +512,7 @@ Token:
'mutable' | 'input' | 'output' | 'timer' | 'action' | 'reaction' |
'startup' | 'shutdown' | 'after' | 'deadline' | 'mutation' | 'preamble' |
'new' | 'federated' | 'at' | 'as' | 'from' | 'widthof' | 'const' | 'method' |
'interleaved' | 'mode' | 'initial' | 'reset' | 'history' | 'watchdog' | 'named' |
'interleaved' | 'mode' | 'initial' | 'reset' | 'history' | 'watchdog' |

// Other terminals
NEGINT | TRUE | FALSE |
Expand Down
9 changes: 9 additions & 0 deletions core/src/main/java/org/lflang/Target.java
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,15 @@ public boolean supportsParameterizedWidths() {
return true;
}

/**
* Return true of reaction declarations (i.e., reactions without inlined code) are supported by
* this target.
*/
public boolean supportsReactionDeclarations() {
if (this.equals(Target.C)) return true;
else return false;
}

/**
* Return true if this code for this target should be built using Docker if Docker is used.
*
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/lflang/ast/ToLf.java
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ public MalleableString caseReaction(Reaction object) {
} else {
msb.append("reaction");
}
if (object.getName() != null) msb.append(" ").append(object.getName());
msb.append(list(true, object.getTriggers()));
msb.append(list(", ", " ", "", true, false, object.getSources()));
if (!object.getEffects().isEmpty()) {
Expand All @@ -639,7 +640,6 @@ public MalleableString caseReaction(Reaction object) {
: doSwitch(varRef))
.collect(new Joiner(", ")));
}
if (object.getName() != null) msb.append(" named ").append(object.getName());
if (object.getCode() != null) msb.append(" ").append(doSwitch(object.getCode()));
if (object.getStp() != null) msb.append(" ").append(doSwitch(object.getStp()));
if (object.getDeadline() != null) msb.append(" ").append(doSwitch(object.getDeadline()));
Expand Down
50 changes: 44 additions & 6 deletions core/src/main/java/org/lflang/validation/LFValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
***************/

package org.lflang.validation;

import static org.lflang.ast.ASTUtils.inferPortWidth;
Expand All @@ -42,6 +43,7 @@
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.emf.common.util.EList;
Expand Down Expand Up @@ -706,12 +708,35 @@ public void checkReaction(Reaction reaction) {
if (reaction.getTriggers() == null || reaction.getTriggers().size() == 0) {
warning("Reaction has no trigger.", Literals.REACTION__TRIGGERS);
}
HashSet<Variable> triggers = new HashSet<>();

if (reaction.getCode() == null) {
if (!this.target.supportsReactionDeclarations()) {
error(
"The "
+ this.target
+ " target does not support reaction declarations. Please specify a reaction body.",
Literals.REACTION__CODE);
return;
}
if (reaction.getDeadline() == null && reaction.getStp() == null) {
var text = NodeModelUtils.findActualNodeFor(reaction).getText();
var matcher = Pattern.compile("\\)\\s*[\\n\\r]+(.*[\\n\\r])*.*->").matcher(text);
if (matcher.find()) {
error(
"A connection statement may have been unintentionally parsed as the sources and"
+ " effects of a reaction declaration. To correct this, add a semicolon at the"
+ " end of the reaction declaration. To instead silence this message, remove any"
+ " newlines between the reaction triggers and sources.",
Literals.REACTION__CODE);
}
}
}
HashSet<VarRef> triggers = new HashSet<>();
// Make sure input triggers have no container and output sources do.
for (TriggerRef trigger : reaction.getTriggers()) {
if (trigger instanceof VarRef) {
VarRef triggerVarRef = (VarRef) trigger;
triggers.add(triggerVarRef.getVariable());
triggers.add(triggerVarRef);
if (triggerVarRef instanceof Input) {
if (triggerVarRef.getContainer() != null) {
error(
Expand All @@ -735,7 +760,14 @@ public void checkReaction(Reaction reaction) {
// Make sure input sources have no container and output sources do.
// Also check that a source is not already listed as a trigger.
for (VarRef source : reaction.getSources()) {
if (triggers.contains(source.getVariable())) {
var duplicate =
triggers.stream()
.anyMatch(
t -> {
return t.getVariable().equals(source.getVariable())
&& t.getContainer().equals(source.getContainer());
});
if (duplicate) {
error(
String.format(
"Source is already listed as a trigger: %s", source.getVariable().getName()),
Expand Down Expand Up @@ -1889,8 +1921,12 @@ private boolean sameType(Type type1, Type type2) {
// Most common case first.
if (type1.getId() != null) {
if (type1.getStars() != null) {
if (type2.getStars() == null) return false;
if (type1.getStars().size() != type2.getStars().size()) return false;
if (type2.getStars() == null) {
return false;
}
if (type1.getStars().size() != type2.getStars().size()) {
return false;
}
}
return (type1.getId().equals(type2.getId()));
}
Expand All @@ -1899,7 +1935,9 @@ private boolean sameType(Type type1, Type type2) {
// (time?='time' (arraySpec=ArraySpec)?) | ((id=(DottedName) (stars+='*')* ('<'
// typeParms+=TypeParm (',' typeParms+=TypeParm)* '>')? (arraySpec=ArraySpec)?) | code=Code);
if (type1.isTime()) {
if (!type2.isTime()) return false;
if (!type2.isTime()) {
return false;
}
// Ignore the arraySpec because that is checked when connection
// is checked for balance.
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,52 @@ public void disallowReactorCalledPreamble() throws Exception {
"Reactor cannot be named 'Preamble'");
}

@Test
public void requireSemicolonIfAmbiguous() throws Exception {
String testCase =
"""
target C

reactor Foo {
output out: int
input inp: int
reaction(inp) -> out {==}
}

main reactor {
f1 = new Foo()
f2 = new Foo()
f3 = new Foo()

reaction increment(f1.out)
f2.out -> f3.inp
lhstrh marked this conversation as resolved.
Show resolved Hide resolved
}

""";
validator.assertError(
parseWithoutError(testCase),
LfPackage.eINSTANCE.getReaction(),
null,
"A connection statement may have been unintentionally parsed");
}

@Test
public void noSemicolonIfNotAmbiguous() throws Exception {
String testCase =
"""
target C

main reactor {
timer t(0)

reaction increment(t)
reaction multiply(t)
}

""";
validator.assertNoErrors(parseWithoutError(testCase));
}

/** Ensure that "__" is not allowed at the start of an input name. */
@Test
public void disallowUnderscoreInputs() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ main reactor {

s = new[2] DoubleCount()

reaction(s.out) named check
reaction check(s.out)

reaction(shutdown) {=
if (!self->received) {
Expand Down
2 changes: 1 addition & 1 deletion test/C/src/no_inlining/BankToReactionNoInlining.lf
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ main reactor {

s = new[2] Count()

reaction(s.out) named check
reaction check(s.out)
}
4 changes: 2 additions & 2 deletions test/C/src/no_inlining/Count.lf
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ main reactor Count {

state count: int

reaction(t) named increment
reaction increment(t)

reaction(t) named check_done
reaction check_done(t)

reaction(shutdown) {= printf("%s", "shutting down\n"); =}
}
4 changes: 2 additions & 2 deletions test/C/src/no_inlining/CountHierarchy.lf
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ main reactor {

state count: int

reaction(t.out) named increment
reaction increment(t.out)

reaction(t.out) named check_done
reaction check_done(t.out)

reaction(shutdown) {= printf("%s", "shutting down\n"); =}
}
4 changes: 2 additions & 2 deletions test/C/src/no_inlining/IntPrint.lf
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ target C {
reactor Print {
output out: int

reaction(startup) -> out named sender
reaction sender(startup) -> out
}

// expected parameter is for testing.
reactor Check(expected: int = 42) {
input in: int

reaction(in) named receiver
reaction receiver(in)
}

main reactor {
Expand Down
2 changes: 1 addition & 1 deletion test/C/src/no_inlining/MultiportToReactionNoInlining.lf
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ main reactor {
state s: int = 6
b = new Source(width = 4)

reaction(b.out) named check
reaction check(b.out)

reaction(shutdown) {=
if (self->s <= 6) {
Expand Down