lingua-franca
lingua-franca copied to clipboard
lf-lang
Detect name conflicts within a reactor in
The following code is legal LF
reactor Foo(bar:int=2) {
input bar:int
state bar: int
method bar(): int {==}
reaction bar(bar)
}
but it causes compile errors in the C++ target and likely also other targets. The problem is, that we don't enforce uniqueness of names in the reactor scope. Currently, there is only a Xtext generated mechanism that detects if triggers have the same name, but it does not handle other reactor elements.
I have a fix for this ( #2085), but it is incompatible with test/Rust/src/CtorParamMixed.lf:
target Rust
reactor Print(value: i32 = 42, name: String = {= "xxx".into() =}, other: bool = false) {
state value = value
state name = name
state other = other
reaction(startup) {=
assert_eq!(42, self.value);
assert_eq!("x2hr", self.name.as_str());
assert_eq!(true, self.other);
println!("success");
=}
}
main reactor CtorParamMixed {
p = new Print(other=true, name = {= "x2hr".into() =})
}
To fix this test, the constructor parameters would have to be renamed... Do we want this?
Interesting. I am curious how the rust generator handles this. Is 'other' essentially handled like a mutable parameter?
I think the safe thing to do for now is enforce uniqueness of names, also between state variables and parameters in all targets. If we think we need "mutable parameters", then we can think about how to express them nicely in LF and properly introduce them in all targets. But I would like to hear @oowekyala's opinion also.
Interesting. I am curious how the rust generator handles this. Is 'other' essentially handled like a mutable parameter?
Parameters in the Rust target are treated as constructor parameters, they are not implicitly stored as state variables, so they are not a mutable parameter here. Parameters and state variables are orthogonal in the rust target. Tbh I think this should be the default behavior in all targets. It is more important that that be the case in Rust, because then you can pass references as parameters, which you couldn't do if they were implicitly stored in a field. However I think the distinction is useful in all targets, because that allows one to express that we need a value at construction time, but not later, so we don't need to store it. Defining parameters as 'constant' state variables (as now in most targets) is also not so useful IMHO, as some of our target languages cannot enforce this.
In my view name conflicts between state variables and parameters are not harmful and can even help convey the 'relatedness' between a parameter and a state var. Eg the idiom of writing a state foo = foo where the second foo is a parameter is useful. Alternatively we should introduce the shorthand syntax that declares a state var and a parameter at the same time, which I wanted to do in rust for a while. For instance:
reactor R(state foo: i32) {}
// is the same as
reactor R(foo: i32) {
state foo: i32 = foo
}
(Ref #1492)
This idiom would not fix the use case where you want to wrap your parameter in some other structure, eg
reactor R(something: i32) {
state something: Wrapper<i32> = {= wrap(something) =}
}
IMHO unless there is a technical reason to forbid conflicts we shouldn't forbid them. This kind of naming constraint doesn't really improve your code, but can be frustrating to the programmer, especially when they feel arbitrary. Just my 2 cents anyway
IMHO unless there is a technical reason to forbid conflicts we shouldn't forbid them. This kind of naming constraint doesn't really improve your code, but can be frustrating to the programmer, especially when they feel arbitrary. Just my 2 cents anyway
I agree. I noticed this when I tried to "fix" the test that started failing and had to rename the parameters...
I think the only way to address this in the C target would be to organize things in the self struct differently, e.g.: self->state->foo and self->parm->foo to distinguish between the state variable and the parameter. @edwardalee: WDYT?
I think it's bad enough in the C target that we have to refer to a state variable (and parameter) as self->name. To have to do self->state->name would be even worse.
Some time ago, I tried to change the C target so you could just use name. I failed. But that would make the name conflicts even worse.
What @oowekyala is saying though, is that in the Rust target parameters do not become part of the self struct. They are only available during construction (passed as arguments to the constructor). If they are to be stored on the self struct, then they need to be explicitly assigned to a state variable (which can be constant or mutable). Since parameters are not stored on the self struct, they also don't need to be checked for name conflicts.
This view makes a lot of sense to me, but bringing the other targets in line with this view on parameters would be a lot of work...
Does this mean that in Rust you cannot access parameter values in a reaction body? That would be a rather severe limitation.
Yes that's exactly it. I don't think it is limiting as the programmer can just save the parameter into a field if they want. It's actually giving more freedom to the lf programmer.