fix: properties now support the REFERENCE TO type

[Angus-Bethke-Bachmann]

Changed

• Added support to the "Reference To Return" lowerer to support properties.

Testing

• Added some lowerer and codegen unit tests to verify property with REFERENCE TO return type behaviour
• Added some lit tests to prove the case

[github-actions]

Build Artifacts

🪟 Windows

Artifact	Link	Size
stdlib.lib	Download	4.3 MB
stdlib.dll	Download	0.1 MB
plc.exe	Download	38.5 MB

From workflow run

🐧 Linux

Artifact	Link	Size
deb-x86_64	Download	38.5 MB
schema	Download	0.0 MB
stdlib	Download	32.4 MB
plc-x86_64	Download	43.6 MB
deb-aarch64	Download	30.9 MB
plc-aarch64	Download	43.6 MB

From workflow run

[mhasel]

This change looks like it was a headache to implement due to the participants. The reordering looks fine, but I would like a few additional tests to ensure we did not break anything, plus a mention of the reordering and why it was necesssary in the commit message in case this causes issues down the line.

Tests I'd like to see:

PROPERTY_SET REFERENCE TO coverage
chained-access tests for properties: fb.propA.propB where both propA and propB are REFERENCE TO properties - both SET and GET variants.

[ghaith]

Note: this review was generated by Claude (via Claude Code) and posted on my behalf. Treat the suggestions as machine-generated input — verify before acting. — Ghaith

Adding to the existing review threads from @mhasel and @volsa rather than re-stating their points. A few additional concerns and explicit test gaps below; happy to discuss.

Cross-cutting concerns

1. Name-prefix heuristics in codegen are fragile. src/codegen/generators/pou_generator.rs now branches on parameter_name.starts_with("__get_") / "__set_" (around L627 and L806). The semantic intent is "this parameter/variable is the synthetic by-ref return slot of a property accessor", but we are inferring that from a string prefix on the variable name, not on the enclosing POU kind or a synthesized-marker on the VariableIndexEntry. Two issues:

   • Any user-declared variable that happens to start with __get_ / __set_ will silently take the new code path (skip null-init, force build_store(ptr, ptr_value) instead of by-val handling). The lexer does accept leading underscores in identifiers.
   • The check is duplicated in two places without a shared helper, so future changes to the synthesis convention have to keep them in lockstep.

   Suggested fix: plumb an explicit flag (e.g. is_synthetic_property_return_ref) onto the VariableIndexEntry from the lowerer that synthesizes it, and gate both call sites on that flag rather than on a name prefix.

2. InitParticipant move from pre_annotate to post_annotate is a behavioral change, not just snapshot churn. As @mhasel noted, the new __FATPOINTER__ctor(reference) lines now appear in many interface-dispatch snapshots that previously had no constructor call there. Even if the body is currently a no-op, this is a live runtime change with potentially repo-wide blast radius (we are running constructors for references that previously weren't constructed). Two asks:

   • Confirm via an end-to-end test (lit or codegen-and-execute) that no observable behavior changes for non-property reference flows — e.g. interface dispatch with reference args still produces the same printed output it did before this PR.
   • Document the new invariant near BuildPipeline::default_participants so the next person understands why InitParticipant had to move past annotation (the inline // Had to move this step post annotate to ensure that the property lowerer has a chance to run is in the participant impl, but the pipeline ordering itself has no comment).

3. pou_original_return_type_is_reference_to is now ambiguous across same-named methods on different POUs. With should_de_qualify = !name.contains("."), an unqualified call site foo() will match any POU whose suffix is foo — including methods on unrelated FBs. This is worse than @volsa's already-flagged instanceB.value regression because here the correct match cannot be recovered after the fact. At minimum, add a targeted test (see test gaps below) and consider qualifying via the current implementation's container before falling back to suffix match.

Test gaps

The added lowerer / codegen / lit coverage is a good start, but a few cases I'd want before this lands:

• are_equal_types Pointer/Reference arm has no direct validation test. The new arm in src/validation/types.rs is reachable, but there is no corresponding validation test asserting (a) REFERENCE TO X := REFERENCE TO X is accepted, (b) REFERENCE TO X := REFERENCE TO Y is rejected, (c) the alias case @volsa flagged (MyDINT aliasing DINT). Without a test, the next refactor of this function will silently regress property-setter compatibility.
• Standalone PROPERTY_SET REFERENCE TO — the lit/codegen tests always pair GET+SET. A setter-only case (no matching getter, or a setter for a property whose getter returns a non-reference type) would prove the SET path stands on its own, and is exactly the case @mhasel asked about.
• PROPERTY_SET whose RHS is a function call returning REFERENCE TO — e.g. fb.prop := bar() where bar : REFERENCE TO X. The new value_is_used flow assumes property-SET feeds a direct expression; an inlined call on the RHS exercises the pre-statement hoisting in a setter context, which the current tests do not cover.
• Nested REFERENCE-TO property chains. The chained lit test (property_with_chained_reference_to_return_type.st) chains a struct field, not a second REFERENCE-TO property. A two-deep chain (fb.outerRef.innerRef.x where both are PROPERTY_GET ... REFERENCE TO ...) would stress node_is_call_statement_with_referenced_pou_that_is_reference_to_return's new ReferenceExpr recursion.
• Inheritance + REFERENCE-TO property override. No test covers a derived FB overriding a parent's PROPERTY_GET ... REFERENCE TO ..., nor SUPER^.prop access to such a property. Given the super/E033 fallout already visible in super_keyword_validation_tests.rs, this is the most likely place for a latent bug.
• Property name collision with the __get_ / __set_ heuristic. A property literally named get_foo (so the lowered method is __get_get_foo) and a parameter incidentally named __get_x should both still compile. No regression test currently anchors this.
• Empty-statement substitution side-effects. When value_is_used == false, the call replacement becomes AstStatement::EmptyStatement (reference_to_return.rs:594). Worth a test that downstream passes (debug info, statement-iteration in codegen) handle a bare EmptyStatement at statement position without panicking.

Smaller items

• get_return_variable_name now branches on name.starts_with("__"). The branch is doing two things at once (de-qualification + prefix collapsing). Consider splitting them and naming each step.
• The two new helpers get_method_name / set_method_name are one-line format!s. If you keep them, fine — but they hide the fact that the same __get_ / __set_ constant is hardcoded in pou_generator.rs too. Pull the prefix into a single pub const to keep the convention discoverable.
• The printf('%d$N', ...) lit tests rely on libc and a runtime; // CHECK-NEXT: 51 after an unrelated // CHECK: 51 works only because CHECK-NEXT allows immediately-following lines. If output ordering between runs ever changes (it shouldn't, but) these tests will be hard to debug. A // CHECK-LABEL: separator per phase would make intent clearer.