search

lf-lang / lingua-franca

Intuitive concurrent programming in any language
https://www.lf-lang.org
Other
240 stars 63 forks source link

Reaction names should be used as a default #2355

Open soerendomroes opened 4 months ago

soerendomroes commented 4 months ago

I would like to use reaction labels as a default and not just their numbering.

Additionally, I would like the reaction label to be just the number if it does not have a label and not reaction_i for reaction i. This way, the diagram would not change for models that do not have labeled reactions while still using a desirable default.

cmnrd commented 4 months ago

I am not sure if I follow. By reaction label, you mean the yellow box that appears when using the @label attribute on a reaction, right? Why should this become the default instead of showing the number within the reaction shape?

soerendomroes commented 4 months ago

I mean the label that is inside a reaction e.g. currently this is something like "1" or "2". One can declare a label for it, by doing something like "reaction 'insert name' (...)" which I think assigns more meaningful label to a reaction than "1". If the option show reaction labels is enabled, it currently shows the label if there is any but displays "reaction_1" for reaction 1 if it does not have a label. I think "1" should be a better default label and that reactions should be labeled as a good developer practice.

cmnrd commented 4 months ago

Ok, so it seems like you are referring to reaction names (not labels) and the "Reaction names" option. Right? This option is intended to show the name of the reaction as it is used in generated code (i.e. it represents the name of the generated function that implements the reaction body). Showing the name is particular important when bodyless reactions are used (i.e. we only code-generate a skeleton and the function implementation is provided externally).

It is important to distinguish label and name here. The label can be an arbitrary string. The reaction name needs to be a valid identifier, and in most languages numbers are not valid identifiers.

edwardalee commented 4 months ago

I'm not sure reactions should be named as good developer practice. The order of reactions is semantically important, so I think the diagram label should include the number even is the reaction is named. I agree that "reaction_1" is a pretty ugly label, and "1: reaction_1" would be even uglier...

soerendomroes commented 4 months ago

reaction_1 is the default name. I just want that the name is displayed as "1" instead of "reaction_1" if no name is specified.

Here the default "names" do nothing except waste space. load_balancer_reaction-names

This should be the default names for reactions without a name. load_balancer_reaction-numbers

edwardalee commented 4 months ago

Agreed. This should be a trivially easy fix...

soerendomroes commented 4 months ago

Additionally, my point is that labeling a reaction "send" is always better than just having a name that says "2".

edwardalee commented 4 months ago

Why? What does "send" mean? Not all reactions send anything.

soerendomroes commented 4 months ago

"send" would be the name I would want to give a reaction by writing reaction send (...) -> .... I would prefer this to leaving the reaction unnamed. It is just an example.

cmnrd commented 4 months ago

If we change this behavior, then the names displayed in the diagram do no longer correspond to their identifier in generated code. Displaying reaction_1 (the default name) is an important feature in my view, and losing the clear mapping between diagram and identifiers in code is a significant cost.

soerendomroes commented 4 months ago

@cmnrd I do not understand your statement. What are you referring to?

Why should "reaction_1" be better than "1"? Or if the reaction has a name, why should "1" or "reaction_1" be better than displaying "send" for a reaction I named "send" in the code? Even if the reaction code uses "reaction1" and cannot use "1" as a name, I am pretty sure people can infer the "reaction". I do not see how we would lose anything by encouraging developers to name their reactions and by displaying "1" instead of "reaction_1" if we enable reaction names.

Additionally, we do never lose the connection between diagram and code since clicking on a reaction will highlight it in the code.

cmnrd commented 4 months ago

This option is intended to show the name of the reaction as it is used in generated code (i.e. it represents the name of the generated function that implements the reaction body). Showing the name is particular important when bodyless reactions are used (i.e. we only code-generate a skeleton and the function implementation is provided externally).

To explain this further: We introduced reaction names to provide bodyless reactions. For bodyless reactions, the reaction body is provided externally (not in the LF file). The user needs to implement a function that is appropriately named. For reaction send (...) the user needs to implement a function called send in the target language. For an unnamed reaction (reaction (...)) the user needs to implement a function called reaction_N in the target language where N refers to the reaction's priority. The purpose of the "Reaction names" option when it was introduced was to support users of bodyless reactions. It establishes a mapping between the diagram and the external definitions of reaction bodies in target code files. This mapping breaks if we only display "1". Furthermore, the reaction name is semantically an identifier, and integer numbers are not a valid identifier.

Since initially you called the reaction names labels, it seems like you have a different use-case in mind. Could you elaborate on your use-case and why the @label attribute is not sufficient?

soerendomroes commented 4 months ago

I called them labels since they are KLabels (I think). Name or label is just an overloaded term. I am unfamiliar with the difference of @label and just naming reactions. As I understand from you is that the label is just a string that a reaction is named and name is the function in the code. That I am missing is why labels and names are not the same, since they seem redundant.

Generally, I think that reaction send ... is much more explicit a better syntax than `@label("send") reaction ....

I do not want that "1" can be a name. I just what that the "label" of the KNode in the diagram that represents the reaction if names should be displayed in a readable and concise manner.

But if you do not like it, feel free to not implement this and close the ticket.

cmnrd commented 4 months ago

But if you do not like it, feel free to not implement this and close the ticket.

Just to be clear: I did not comment on the quality of the proposal or how I like it. I actually think your user story makes sense. I am pointing out that there is a user story that we already have committed to that conflicts with the concrete implementation that you propose. That is not a matter of my personal taste.

As I stated before, I think your user story makes sense, and I don't think it is necessarily at odds with the existing one. I am sure there is an alternative implementation that allows us to achieve your goal without breaking an existing feature. I want to invite you to work on this constructively, and I am happy to assist in finding that solution.