feat: Properties #1396
feat: Properties #1396
Conversation
Introduces the PROPERTY, GET, SET and their END_* counterpart tokens
This commit lowers properties into concrete POUs and Implementations. For example
a property such as
```
FUNCTION_BLOCK fb
PROPERTY foo : DINT
GET
END_GET
SET
END_SET
END_PROPERTY
END_FUNCTION_BLOCK
```
will be lowered into a
```
FUNCTION_BLOCK fb
VAR_PROPERTY
foo : DINT
END_VAR
METHOD __get_foo : DINT
// ...
__get_foo := foo;
END_METHOD
METHOD __set_foo : DINT
VAR_INPUT
__in : DINT;
END_VAR
foo := __in;
// ...
END_METHOD
END_FUNCTION_BLOCK
```
This commit transforms references of properties such as `myFb.myProp := 5` and `myVar := myFb.myProp` into setter and getter methods respectively. Broadly speaking when dealing with assignments the lowerer will check if the left side is a property and transform the whole node into a set call by `<lhs_node>(<rhs node);`. Otherwise, the visitor will visit every statement and check if the reference is a property and if so replace it with a getter call.
This commit introduces a POC where the GlobalContext has a handle function for reporting individual diagnostics. Furthermore the GlobalContext now makes use of the annotate-snippet-rs crate, which also is used in the Rust compiler itself (per #942 we wanted to incorparte in our compiler eventually anyways)
volsa
force-pushed
the
volsa/property2
branch
from
7897c28 to
dae9b16
Compare
Still missing some big spots, however this commit already improves the source code a bit
| @@ -15,6 +15,8 @@ use plc_source::source_location::SourceLocation; | |||
|
|
|||
| pub mod calls; | |||
| mod initializers; | |||
| pub mod property; | |||
| pub mod validator; | |||
There was a problem hiding this comment.
This is the wrong location for the validator, ideally this should live in its own crate. Personally I'd keep it there for now and refactor it once we also refactor the validation as a whole now that we have the Participant trait.
volsa
force-pushed
the
volsa/property2
branch
from
7720cdf to
f9c4a4d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1396 +/- ##
==========================================
+ Coverage 93.67% 93.69% +0.01%
==========================================
Files 166 170 +4
Lines 50097 51536 +1439
==========================================
+ Hits 46928 48285 +1357
- Misses 3169 3251 +82 ☔ View full report in Codecov by Sentry. |
|
The CI seems to fail because of OSCAT, I've created a PR which should hopefully fix that issue here PLC-lang/oscat#3 |
| // XXX: Temporary solution, is there a better way? Technically we could introduce a diagnostic when | ||
| // lowering the references to calls, then checking with the index if the defined POU exists but | ||
| // then we'd get two similar error, one describing what the exact issue is (i.e. no get/set) and | ||
| // the other describing that it cant find a reference to "__{get,set}_<property name>" | ||
| match ref_name { |
There was a problem hiding this comment.
I hate this. I'm wondering if it's time to introduce a internal_name: Option<String> to the ReferenceExpr node. Alternatively some data-structure (HashMap?) keeping track of internally mangled names with the key being the node ID. Not sure if this is overkill but if we do rely on lowering more often now (especially where we change names), it might be something to take into consideration?
| // TODO: Find better way, this is hella expensive | ||
| pub fn re_annotate(self, mut id_provider: IdProvider) -> AnnotatedProject { | ||
| // TODO: this almost duplicates IndexedProject::annotate(). Can we do this differently? | ||
| // Create and call the annotator again after lowering property references to calls | ||
| let mut annotated_units = Vec::new(); | ||
| let mut all_annotations = AnnotationMapImpl::default(); | ||
| let result = self | ||
| .units | ||
| .into_par_iter() | ||
| .map(|unit| { | ||
| let (annotation, dependencies, literals) = | ||
| TypeAnnotator::visit_unit(&self.index, &unit.unit, id_provider.clone()); | ||
| (unit, annotation, dependencies, literals) | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| for (unit, annotation, dependencies, literals) in result { | ||
| annotated_units.push(AnnotatedUnit::new(unit.unit, dependencies, literals)); | ||
| all_annotations.import(annotation); | ||
| } | ||
|
|
||
| let mut index = self.index; | ||
| index.import(std::mem::take(&mut all_annotations.new_index)); | ||
|
|
||
| let annotations = AstAnnotations::new(all_annotations, id_provider.next_id()); | ||
|
|
||
| AnnotatedProject { units: annotated_units, index, annotations } | ||
| } |
There was a problem hiding this comment.
Not sure if we want to keep it this way for now, since we have a similar issue with indexing and squash both these issues in one go? We should track them in the issue tracker though
| @@ -1,32 +1,50 @@ | |||
| use annotate_snippets::Renderer; | |||
There was a problem hiding this comment.
This is a POC, at some point we should refactor how the validation and diagnostician works since we are now able to validate at different phases in the pipeline
This PR introduces the
PROPERTYlanguage feature. Properties in itself are getter and setter methods defined in a (stateful) POU and have the form ofThey are invoked by other POUs by simply accessing the property name. For example
Specifically, references to properties always resolve to get calls with the exception in assignments when the property itself is accessed on the lhs.
The compiler will initially handle these properties as dedicated
Propertynodes in theCompilationUnitto have a 1:1 representation of the user code and only later on lower these properties to methods and property references to calls.