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

C syntax for referencing parameters and state variables #82

Closed
edwardalee opened this issue Jan 8, 2020 · 52 comments
Closed

C syntax for referencing parameters and state variables #82

edwardalee opened this issue Jan 8, 2020 · 52 comments

Comments

@edwardalee
Copy link
Collaborator

edwardalee commented Jan 8, 2020

Currently, we address parameters and state variables using the syntax self->name.
I’m finding this rather awkward. I would like to change it to just name.
Doing this requires using macros, not C variables, because a new local C variable on the stack would be a copy of the parameter or state rather than the parameter or state itself.
I suggest we generate code something like this:

void reaction_function_name(...) {
  ... existing generated prefix code ...
   #define name self->name
  ... verbatim code for reaction references name ...
   #undef name
}

The above #undef is necessary to limit the scope of the macro. Otherwise, two reactors in the same file could not have the same state variable or parameter names.

Using macros is fraught with peril, and we are already probably doing too much. We can, with some work, replace all the macros with our own preprocessing of the C code. This could be done with a relatively simple parser that understands comments, strings, and tokens. I think we are going to want to incorporate target-language grammars more fully eventually anyway to support a better editing experience in Eclipse.

I've poked around a bit in xtext docs to look for a way to conditionally include a sub grammar within our {= ... =} code block delimiters. This would be easy to do if we had only one target language, but since we don't, it is not obvious how to do this. Suggestions?

@cmnrd
Copy link
Collaborator

cmnrd commented Jan 8, 2020

In order to avoid self->name it could be an alternative to always use pointers in C (*name). This would avoid the copying and would work without any preprocessing.

An alternative to preprocessing the target language (and all target languages eventually) could be to allow to escape from the target code in order to reference to a variable defined in Lingua Franca. This could look something like this:

reaction(name) {=
  printf("Hello %s!", @name@)
=}

Then the code generateor could generate the proper code for accessing this variable in the target language. This would also add the benefit that the LF compiler would be able to verify that the reaction is actually allowd to access this variable without relying on scoping mechanisms in the target language. Of course this would require to find a proper escape charater that is unlikely to be used in any of the possible target languages.

@edwardalee
Copy link
Collaborator Author

This is a good idea, but it would still require parsing that understands to exclude comments and strings, so I'm not sure it buys that much over just parsing the language. The minimal parser would just tokenize with an understanding of comments and strings, which is almost the same parser that would be needed in this case.

@cmnrd
Copy link
Collaborator

cmnrd commented Jan 8, 2020

I am not sure why it would require to understand comments and strings. Do you mean to avoid accidental matching of the escape charater in comments and strings?

In the case of preprocessing the entire target code block, I don't believe that understanding comments and strings would be sufficient. The preprocessor would require a deeper knowledge of the language. Consider this case in C where there is a struct with a member called name:

preamble {=
typdef struct {
    char* name
=} X;

reaction(name) {=
  X x;
  x.name = name;
  // do something with x
=}

Simple replacement based on tokens would break this code. It requires deeper knowledge of the language to get it right. This is probably dooable with reasonable effort for C, but I think it can become arbitrarily complex in other languages.

@edwardalee
Copy link
Collaborator Author

Good point. I guess macros will have the same problem?

@lhstrh
Copy link
Member

lhstrh commented Jan 8, 2020

In the TypeScript runtime, there would be no problem admitting arbitrary objects as a parameter of the react function. This would allow reaction code to refer to state variables directly, without any prefix. However, since JS is pass-by-value, this strategy won't work for primitive data types (the reaction would effectively just get a copy of the shared state, not a pointer to the state).

Because of this, we just pass in a reference to the parent (a Reactor) of the Reaction in its constructor. That way, all public fields (TS has access modifiers) of the reactor are accessible within the reaction code.

In order to gain visibility into the members of the parent, which must subclass Reactor, we do a typecast. For instance:

class Bar extends Reactor {
    private x = new InPort<number>(this);
    public foo:string = "42";
    
    constructor(parent: Reactor) {
        super(parent);
        this.addReaction(new class<T> extends Reaction<T> {
            //@ts-ignore
            react(): void {
              var self = this.parent as Bar;   // Inserted by the code generator
              self.foo = "baz";
            }
          }(this, [this.x], []));  // Constructor args: parent, triggers, arguments to react
    }
    
}

I.e., we refer to foo via self.foo. This brings the syntax as close as possible to what we currently have in the C target.

I personally don't feel it's so terrible to refer to state variables via the self struct; it also makes it abundantly clear that other reactions may affect it. One problem I foresee, however, is that the term self will be problematic once we start developing a Python target. If we want something that is not likely to collide with other languages, perhaps we should use reactor rather than self?

@cmnrd
Copy link
Collaborator

cmnrd commented Jan 9, 2020

Good point. I guess macros will have the same problem?

Yes

I am also good with using self. I think the real benefit of having a way for preprocessing or embedding LF variables via escape characters in the target code would be that the LF compiler is capable of analyzing and checking these variable accesses. This would simplify code generation and produce better error messages.

BTW: self in python is not a protected keyword. It is a variable as any other and using self as a name is just a convention. In python, the first argument of a member function is the object the method is called on. How this argument is called is completly up to the programmer.

Instead of this:

class Foo:
  def bar(self):
    print(self)

one can also write this:

class Foo:
  def bar(obj):
    print(obj)

So using self in python shouldn't be much of a problem.

@lhstrh
Copy link
Member

lhstrh commented Jan 9, 2020

Here's another suggestion: since we're declaring state using the keyword state, why don't we use the same keyword to refer to it in reaction code? I.e., we'd have state.some_var (TypeScript) and state->some_var (C). I have to admit that there is some cognitive dissonance that comes with using self for state but not for ports (both are presumed to be members of the reactor object). Perhaps just using state instead of self already takes away that cognitive dissonance.

@edwardalee
Copy link
Collaborator Author

I don't think 'state' will work well because we also need a syntax for parameters.

@lhstrh
Copy link
Member

lhstrh commented Jan 10, 2020

Right. How about reactor?

@cmnrd
Copy link
Collaborator

cmnrd commented Jan 10, 2020

This is just my personal preference, but I still think self is best as (at least to me) it is clear what it means.

While we are at it: I was always wondering why self is used for state and parameters but not for ports and actions. I understand that this comes from the way things where developed in C (avoid copies of state and parameters), but in my opinion it would be a more consistent design if either everything is accessed via self or everything is accessed without such an indirection (or with an escaping mechanism).

@edwardalee
Copy link
Collaborator Author

I agree, which is why I wanted to get rid of self. I think that requiring it everywhere would really make the code much uglier. Compare these two:

reactor Count(stride:int(1)) {
    state count:int(0);
    output y:int;
    timer t(0, 100 msec);
    reaction(t) -> y {=
        set(self->y, self->count);
        self->count += self->stride;
    =}
}

vs.

reactor Count(stride:int(1)) {
    state count:int(0);
    output y:int;
    timer t(0, 100 msec);
    reaction(t) -> y {=
        set(y, count);
        count += stride;
    =}
}

The latter can only be done by making count a macro, I think.

@lhstrh
Copy link
Member

lhstrh commented Jan 10, 2020

We can eliminate the use of self in the TypeScript target with the help of generated code.
I've altered the example I shared earlier to show how it can be done:

class Bar extends Reactor {
    private x = new InPort<number>(this);
    public state = {foo: "42"}; // Notice that we store the state in a different place now
    
    constructor(parent: Reactor) {
        super(parent);
        this.addReaction(new class<T> extends Reaction<T> {
            //@ts-ignore
            react(): void {
              var self = this.parent as Bar;   // Inserted by the code generator
              var foo = self.state.foo;           // Extra line(s) inserted by the code generator
              foo = "baz";
            }
          }(this, [this.x], []));  // Constructor args: parent, triggers, arguments to react
    }    
}

This approach also ensures that every reaction references to primitive state variable (i.e, of type number, string, etc.) are updated.

It would be silly to write a hand-written reactor in this style, however: one would have the state as a public member of the reactor and reference it directly (as I showed in the previous example). These two approaches are compatible though. So, should we decide that in LF source code we don't want reactions to need to refer to self, we can bring the TypeScript target in compliance easily...

@lhstrh
Copy link
Member

lhstrh commented Jan 10, 2020

I forgot to add code-generated lines at the end of react that write the value of each introduced local var back to the state where it was pulled from...

Here's a corrected example:

class Bar extends Reactor {
    private x = new InPort<number>(this);
    public state = {foo: "42"};
    
    constructor(parent: Reactor) {
        super(parent);
        this.addReaction(new class<T> extends Reaction<T> {
            //@ts-ignore
            react(): void {
              let self = this.parent as Bar;   // Inserted by the code generator
              var foo = self.state.foo;        // Inserted by the code generator
              // --> start user-written code
              foo = "baz";
              // <-- end user-written code
              self.state.foo = foo;            // Inserted by the code generator
            }
          }(this, [this.x], []));  // Constructor args: parent, triggers, arguments to react
    }
}

@lhstrh
Copy link
Member

lhstrh commented Jan 10, 2020

But this fix doesn't take in account that the reaction code could possibly return... We could possibly use a closure to catch this without inspecting the code, but that most certainly poses difficulties to the type checker. So I take back what I said; I'm not entirely sure we can make this work.

@lhstrh
Copy link
Member

lhstrh commented Jan 10, 2020

While we are at it: I was always wondering why self is used for state and parameters but not for ports and actions. I understand that this comes from the way things where developed in C (avoid copies of state and parameters), but in my opinion it would be a more consistent design if either everything is accessed via self or everything is accessed without such an indirection (or with an escaping mechanism).

One reason to justify this is that the interface of the reaction declares explicitly the things that it can reference in its local scope; we don't do that for state and parameters, so why would we expect them to be locally accessible? The rationale is no different from OOP, actually, where a member function directly references its arguments but has to use this in order to refer to member fields of the object that it is a member of.

I'm starting to get convinced that we should probably just leave things as they are...

@edwardalee
Copy link
Collaborator Author

edwardalee commented Jan 10, 2020 via email

@lhstrh
Copy link
Member

lhstrh commented Jan 10, 2020

True. But I personally think it is bad style not to use this when referring to class variables because it can lead to confusion and is not very robust to refactoring (due to inadvertent shadowing).

Java's ability to refer to class variables without this I don't see as a reason to strive for following the same strategy. TypeScript doesn't allow it, for instance. More importantly, for the TypeScript target, I don't see a path toward introducing the feature in our framework without having to parse target code or losing the ability to statically type check reactions. Neither seem like a reasonable price to pay for a feature that is ultimately only going to save a handful of ASCII characters... Do you see a clean solution for the C target that does not require inspection of reaction code?

@edwardalee
Copy link
Collaborator Author

edwardalee commented Jan 11, 2020 via email

@lhstrh
Copy link
Member

lhstrh commented Jan 11, 2020

It does, but how did you imagine using it to prevent a return statement in the reaction code from bypassing the coda that is supposed to "write back" changed values to the reactor's state?

@edwardalee
Copy link
Collaborator Author

Put the coda in the finally clause.

@lhstrh
Copy link
Member

lhstrh commented Jan 11, 2020

You can only return in a function. If you're in a function you're in a different scope. I don't see how this helps...

What you can do, as I suggested earlier, is use a closure. I didn't think TypeScript's type inference was strong enough to preserve our ability to ensure that the types of the ports and the types of the arguments in the react function they map to actually match. I might be wrong about that, however. We could implement a pattern along the lines of the following sketch:

class Reaction {
    constructor(public port: string) {
    }
    react(port: string) {
        x++;
    }
}

var r = new Reaction("dummyport");
var x = 1;
function y() {
    r.react.apply(null, [r.port]);
}
y();
console.log(x); // prints 2

So, assuming that we would be able to get this to work in TypeScript, what would the solution be in C?

@edwardalee
Copy link
Collaborator Author

I don't understand. Why won't this work:

        this.addReaction(new class<T> extends Reaction<T> {
            //@ts-ignore
            react(): void {
              let self = this.parent as Bar;   // Inserted by the code generator
              var foo = self.state.foo;        // Inserted by the code generator
              try {       // Inserted by the code generator.
              // --> start user-written code
              foo = "baz";
              // <-- end user-written code
             } finally { // Inserted.
              self.state.foo = foo;            // Inserted by the code generator
             } // Inserted.
            }
          }(this, [this.x], []));  // Constructor args: parent, triggers, arguments to react

@lhstrh
Copy link
Member

lhstrh commented Jan 11, 2020

Ah, I didn't realize that finally also caught returns. I was under the assumption that it would only catch exceptions... In other words, I expected the following code to print nothing:

function y() {
    try {
        return;
    } finally {
        console.log("Never to be seen")
    }
}
y();

But it prints "never to be seen", so you must be right :-) This is a neat solution. The open question then remains: how do we do it in C?

@lhstrh
Copy link
Member

lhstrh commented Jan 11, 2020

We could use longjmp...

@edwardalee
Copy link
Collaborator Author

longjmp is extremely dangerous. It can leave inconsistent state and cause memory leaks.
It's also difficult to make portable. I see only two ways in C to reference state and parameter variables without 'self->':

  1. Use macros. This has the cost that the macro may be expanded in inappropriate places, as pointed out by Christian. These problems could be fixed by parsing the code and performing some (probably rudimentary) static analysis.

  2. Use escape characters, such as Christian's suggestion of @name@. This would take some getting used to.

I personally like best referring to parameters and state simply by name. This seems the most natural and readable to me. We could implement this with macros for now, with a "to do" item to add a parser. I think we are going to want to add a parser eventually anyway to get a better editing/IDE experience.

I don't think it's really necessary for the syntax to be identical across target languages. I don't think you will find many programmers writing reactors simultaneously in multiple languages. Those that do will be facing bigger cognitive dissonances than this one.

@lhstrh
Copy link
Member

lhstrh commented Jan 11, 2020

I agree with all the points you make. However, I would be more inclined to keep things the way they are now, and add the macros only after we've mixed in a grammar for C and are sure we can do it safely. Once we have that, all sorts of things can be done to make the programming experience more fluent; we could do away with the =}, for instance, and we could infer dependencies rather than have the programmer declare them. To me, this all sounds like a v.2 of Lingua Franca...

@edwardalee
Copy link
Collaborator Author

OK, let's proceed that way. Moving from "self->statename" to just "statename" can be done in a backward-compatible way, whereas going the other direction cannot, so let's leave it the way it is for now.

@MattEWeber
Copy link
Contributor

MattEWeber commented Feb 5, 2020

I implemented the try, finally approach to parameter and state naming in the TypeScript generator. An example generated react function looks like this:

function (this, t: Readable<TimeInstant>, c: Writable<number>, __i: State<number>) {
                // =============== START react prologue
                let i = __i.get();
                // =============== END react prologue
                try {
                    i++;
                    c.set(i);
                } finally {
                    // =============== START react epilogue
                    __i.set(i);
                    // =============== END react epilogue
                }
            }

It occurred to me that my scheme of prefacing state and parameters with a "__" in the react function signature would fail if you happened to have two state variables named for example "foo" and "__foo" because then you couldn't declare the prologue variable "__foo" for state "__foo" without conflicting with "__foo" in the signature for state "foo".

Any thoughts on adding a check in the validator to prohibit naming state and parameters with an initial "__"? I think this would prevent various other potential problems in the TS target like naming a state variable "__parent__".

@MattEWeber
Copy link
Contributor

I could also sidestep the underscore issue entirely if I were to write state and parameters into a state object and pass that in as an argument. So my example would become.

function (this, t: Readable<TimeInstant>, c: Writable<number>,  __stateVar : { i : State<number>} ) {
                // =============== START react prologue
                let i = __stateVar.i.get();
                // =============== END react prologue
                try {
                    i++;
                    c.set(i);
                } finally {
                    // =============== START react epilogue
                    __stateVar.i.set(i);
                    // =============== END react epilogue
                }
            }

I think I should just do this. Then we just have to prohibit "__stateVar" or whatever I wind up naming it.

@lhstrh
Copy link
Member

lhstrh commented Feb 5, 2020

It sounds reasonable to me to prohibit a "__" prefix in the names of ports, actions, timers, state, and parameters.

@lhstrh
Copy link
Member

lhstrh commented Feb 5, 2020

I'm not sure that the type inference that TypeScript does is clever enough to correctly infer the type of a single state object that holds all the stuff. Have you confirmed that things get typed correctly with this approach? And if so, are the type errors readable when something is off?

@lhstrh
Copy link
Member

lhstrh commented Feb 5, 2020

Also, shouldn't the ports in the signature be prefixed with a double underscore? I would expect we treat state and ports equally. In which case, lumping together the state doesn't solve any problem at all.

@edwardalee
Copy link
Collaborator Author

I’m in favor of prohibiting __ prefixes across the board.
In the C target, we use such prefixes for things the user should not access, like __schedule().
When we eventually parse the C code, we could catch those violations.

@MattEWeber
Copy link
Contributor

I can confirm the type inference is okay with either of the two options I gave above. It catches errors with reasonable messages. If we do get rid of the __prefixes, and based on your feedback it sounds like I should go ahead and do that, I'll keep the original "add an underscore to everything" scheme. It's simpler.

Regarding ports, I thought it might be best to avoid including them in the try finally trick because unlike state, we don't want to call __portName.set(portName) in the epilogue. That could have the unintended effect of writing to a port that wasn't actually written to in the react function. I suppose we could get away with writing portName = __portName.get() in the prologue for input ports, but that would result in the confusing situation where you have separate rules for different kinds of ports.

@lhstrh
Copy link
Member

lhstrh commented Feb 5, 2020

As for the epilogue: we can just check whether the variable is defined and only call set on those that have indeed been assigned a value. As for the prologue: we can do portName = __portName.get() for all ports and actions that are dependencies; all others would be declared but remain undefined. All this information is known in the code generator, so this should be easy to do.

@MattEWeber
Copy link
Contributor

I like explicit sets and gets on ports because it makes it clear you're preforming an operation on a port. Usually when I make an assignment to a variable I don't expect that assignment to result in a function being called, so there's counterintuitive to me about this.

The really annoying case for state was when you wanted to increment it because you had to write i.set(i.get() + 1) instead of i++. That's not a scenario that can happen for ports because you never need to get and set from the same port, so I don't see it as quite so annoying.

Is undefined a valid type for ports? Because we'd have to prohibit it if we wanted to do the "set if it has indeed been assigned a value" strategy.

@lhstrh
Copy link
Member

lhstrh commented Feb 5, 2020

I don't see the point of treating ports and state differently. We also don't use get in the C target. The only reason why we have to use set is because in C primitive data types are not nullable (hence it is implemented as a macro that also sets the variable that indicates the presence of a value).

I was OK with not having the try/catch, but if we use it, it is my strong preference that we treat ports, actions, timers, state, and parameters the same and define them all directly as local variables in reaction scope.

@lhstrh
Copy link
Member

lhstrh commented Feb 5, 2020

In that case, a local var corresponding to a InPort<string> would be of type string | undefined. Setting it to null would not type check, and setting it to undefined would simply mean that there is no event. Does that sound reasonable?

@MattEWeber
Copy link
Contributor

I see this as an issue of providing the author of a react function the most reasonable abstractions for ports, actions, timers, etc. It makes a lot of sense to me to represent state and parameters as local variables because they're really just variables that belong to the reactor.

Ports are a bit more complicated because a port is conceptually an object you can interact with. Sure, they're like variables in that they carry values, but there's a semantic distinction between writing a value and sending a message. For example

portName.send(1); // I know we use set(), see below.
portName.send(2);

means something different than

portName = 1;
portName = 2;

because in the first case it's clear you're sending two messages, and in the second case you've staged the value 1 before changing your mind and staging the value 2. Similarly, an import port is an object you can ask multiple questions to: did you receive a message (what used to be isPresent()) and what value did you receive.

That said, I understand that recent changes you've made to the reactor-ts API have changed ports so they can be encoded into variables: Using the value null to represent an absent value on an input port to get rid of isPresent(). And the fact that you actually call set() instead of send() on a output port means writing to a port in a reaction doesn't really trigger a send() until the end of the reaction. The behavior is like a variable.

I'd lump timers and triggering actions into the same category as input ports because they really aren't variables, but can be encoded as such with null representing absent. But schedulable actions definitely can't be encoded as variables because you schedule an action with both a delay and a value. You can't just schedule an action as

actionName = "value"

because its delay is unspecified. So at the very least we have to leave some actions as non-variables.

I can get 100% behind parameters and state as variables because that's where they are. I'm maybe 40% behind encoding everything else as a variable. It seems to have some pros and cons.

@lhstrh
Copy link
Member

lhstrh commented Feb 5, 2020

I don't see the conceptual distinction you're making between ports and state. In fact, the way they are currently implemented in the TypeScript runtime is exactly the same.

Actions are a different, I agree. They could be dealt with by bringing into scope a schedule function that takes a regular variable (as opposed to an Action object) as an argument, but it might be hard to check statically whether or not the argument relates to an actual action. We could try...

@cmnrd
Copy link
Collaborator

cmnrd commented Feb 6, 2020

I think that this discussion is going in the wrong direction. I would like to go back to a suggestion I made earlier (escaping traget code), as I strongly believe that the question of what ports, actions, state, etc. are and how they are accessed should be answered by LF and not by the implementation of each target. LF is the central place where this problem can be solved. If we do it in the target code, we end up with a multitude of solutions where languange A useses appraoch X to access LF primitves and language B uses approach Y, while the compiler of language A can check constraints 1,2 and 3, but the compiler of language B can only check constraints 2,4 and 5.

Why should we rely on the target compiler for validating port accesses and the scheduling of actions if we can do this in the Lingua Franca compiler? What I have in mind is something like this (modified from ActionDelay.lf):

reactor GeneratedDelay {
    input y_in:int;
    output y_out:int;
    state y_state:int(0);
    logical action act(100 msec);

    reaction(y_in) -> act {=
        @y_state@ = @y_in@;
        @schedule act in 100 msec@(NULL);
    =}

    reaction(act) -> y_out {=
        @set y_out@(@y_state@);
    =}
}

Where everything between two @ is processed by the LF compiler. For sure the syntax can be improved, but an approach like this enables the LF compiler to validate any accesses to the primitives of LF. It can then generate the corresponding target code and the target language compiler can do what its best at: compile target language code without knowing about higher level concepts like reactors.

Combine this approach with a type-system in LF and you get a really powerful tool.

@edwardalee
Copy link
Collaborator Author

This is an intriguing proposal, but I have to admit an aesthetic negative reaction to the syntax. It seems to me that the "right" long-term solution is to integrate a compiler for the target language with the Lingua Franca compiler. In principle, this would not be that hard to do with an LLVM-based compiler, but of course, it would be a tremendous amount of work. Nevertheless, if our language takes of (a BIG if), then that is what should eventually happen. Analogously, the first C++ compilers translated C++ to C. That didn't last long once the language became established.

So I would prefer to find a solution that has an eye towards this long term solution, and I believe that that solution refers to inputs, outputs, and state variables simply by name.

Also, I'm not that bothered by the syntax of target differing across target languages. These syntax differences pale in comparison to the syntax differences in the languages themselves.

@MattEWeber
Copy link
Contributor

Since there seemed to be a consensus on this, I committed a change to the validator to block on all targets leading "__" on names for inputs, outputs, actions, timers, parameters, state, reactor definitions, and reactor instantiations. I wrote JUnit tests for these as well (although part of the reactor definitions, and reactor instantiation tests are incomplete because of reasons I don't understand yet).

@lhstrh I see this as a question of choosing the most concise, intuitive, aesthetically pleasing, etc. framework to LF programmers. Even if ports and state are implemented a particular way, there's no reason we have to expose them the same way to LF programmers. I think there's a case to be made that it's more intuitive to treat some of the entities available inside a reaction as variables and others as objects even if that's a little more verbose. Again, I'm feeling maybe 60% that it's worth it.

@MattEWeber
Copy link
Contributor

MattEWeber commented Feb 10, 2020

How do we feel about blocking entities named "_" (just a single underscore)? Currently the name "__" is illegal, I think "_" should be too.

@cmnrd
Copy link
Collaborator

cmnrd commented Feb 11, 2020

In some languages there is a convention to use the prefix _ to indicate private usage. This is for instance common in python, where any member of a class is acessible from outside. Also in C++ there is a common convention that prefixes private member variables _variable and uses a getter method variable() without the prefix on the public class interface. Therefore, I think _ should be allowed as it is commonly used within class definitions.

@MattEWeber
Copy link
Contributor

I should explain because my last post was a bit confusing. I had just caught a bug in validation that came up when checking for a leading __ on names with only one character. It occurred to me when writing the guard for strings of length two or more, that I was still allowing the one character long name _. I think a single underscore is fine as a prefix and we should keep it. So _variable is good but _ is maybe bad.

@cmnrd
Copy link
Collaborator

cmnrd commented Feb 11, 2020

Ah, I see. Yes, I am fine with banning the single character _.

@MattEWeber
Copy link
Contributor

After some time playing with TS tests I have revised thoughts on naming elements in TypeScript for reactions. When writing reactions using the input.get() or action.get() style, I found myself doing this a lot:

let localInput = input.get();
if (localInput == someValue) {
    ...
}

and that let localInput = input.get(); is exactly what we're talking about doing automatically in a try finally block. If that local variable assignment is something most programmers are going to want to do anyway at the top of every reaction, we might as well do it for them. So in my latest commit to the TS code generator I changed it to assign inports, timers, and read-only actions to local variables (in addition to state and parameters).

However, I did not have a similar frustration with outports or schedulable actions. It's not particularly painful to write

output.set(2)

and it's a lot clearer to me that this code is performing an operation on a port than

output = 2

There's also a case with actions that's particularly hard to handle with assignments to local variables. With z as an action:

reaction(startup, z) -> z {=
    if (z.get()) {
          console.log(z.get());
    }
    z.schedule(new UnitBasedTimeInterval(1, TimeUnit.sec), "hello") 
=}

Imagine how this would look if we tried turning z into a local variable assigned the value of its payload in a reaction prologue:

reaction(startup, z) -> z {=
    if (z) {
          console.log(z);
    }
     util.scheduleAction( ?????, new UnitBasedTimeInterval(1, TimeUnit.sec), "hello")
=}

Even if we created a utility function to schedule the action, how do we tell that function to schedule z? We can't put z where the question marks are because z is just a local variable that might contain the string "hello".

In summary I think we we should do local variable assignments for read-only elements, but it's better to leave the objects that need to have actions performed on them available as objects.

@edwardalee
Copy link
Collaborator Author

I had a similar problem with the C target and ended deciding to define a variable z_value to represent the value of the action, whereas z represents the action itself.

@lhstrh
Copy link
Member

lhstrh commented Mar 2, 2020

After some discussion we settled on having a try/finally solution in the TypeScript generator for all entities (ports, action, timers, state, parameters) so we don't have to call get or set in reaction code all the time. We bring into scope an object called actions that can be referenced for scheduling actions. For example, if a schedulable action is an argument of the react function, then the generated preamble will have something like this in it:

var actions: {x:Schedulable<number>} = {x: __x};
let x = __x.get();

Scheduling x will then be done like this:

actions.x.schedule(...)

@MattEWeber
Copy link
Contributor

I believe the discussion involving TypeScript is now resolved. Should we leave this open for C and C++ discussion or are we good to close the issue?

@edwardalee
Copy link
Collaborator Author

I think that enough time has passed with the self-> syntax in C that we are settled on it, so I am closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants