Support for retain sections by ghaith · Pull Request #1592 · PLC-lang/rusty

ghaith · 2026-02-11T14:54:10Z

Adds support for retain sections in the code.
If a global variable is marked as retain, or has in its datatype any variable that is marked as retain, the variable will be added tot he retain section.
Variables in PROGRAMS that are marked as retain of have variables in their datatype that are retained are converted into a pointer to a global retain variable. That variable is then in turn placed int he retain section during codegen.

Added parser tests for retain variables Added index entries and test for retain variables in globals, programs and fbs.

A retain variable in a program is added to the global vars and a reference to the variable is added to the program If a variable contains a retain variable nested inside it is also treated as retain

During codegen, if a global variable should be retained add it to the .retain linker section

Copilot

Pull request overview

Adds support for RETAIN variable blocks by propagating retain-ness through the type system, carrying retain metadata through indexing, and emitting retained globals into a dedicated LLVM linker section. Also introduces a new lowering pipeline participant intended to move/indirect retain variables, plus associated test/snapshot updates.

Changes:

Add is_retain tracking to variable indexing and implement transitive should_retain checks via datatype membership/aliasing/arrays.
Emit globals that should_retain into the LLVM section .retain.
Introduce a new lowering pass (plc_lowering::retain) and update parser/index/codegen tests & snapshots for the new retain/constant debug output and fields.

Reviewed changes

Copilot reviewed 44 out of 44 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/typesystem.rs`	Adds recursive `should_retain` computation on datatypes (struct/alias/array).
`src/index.rs`	Adds `is_retain` flag to `VariableIndexEntry` and `should_retain()` helper for codegen/lowering decisions.
`src/index/indexer/pou_indexer.rs`	Plumbs `block.retain` into member variables (`is_retain`).
`src/index/indexer/global_var_indexer.rs`	Plumbs `retain` into global variables (`is_retain`) via `VarGlobalIndexer`.
`src/index/indexer.rs`	Passes `block.retain` into the global var indexer.
`src/index/indexer/user_type_indexer.rs`	Ensures struct members are never directly marked retain (but can be contained in retained types).
`src/codegen/generators/llvm.rs`	Adds `GlobalValueExt::make_retain()` which sets the LLVM section to `.retain`.
`src/codegen/generators/variable_generator.rs`	Marks globals as retain in IR when `global_variable.should_retain(...)` is true.
`compiler/plc_lowering/src/retain.rs`	New lowering pass intended to move retain vars into global retain blocks and add indirection for PROGRAM retain vars.
`compiler/plc_lowering/src/lib.rs`	Exposes the new `retain` module.
`compiler/plc_driver/src/pipelines.rs`	Adds `RetainParticipant` into the driver pipeline.
`compiler/plc_driver/src/pipelines/participant.rs`	Implements `PipelineParticipantMut` for `RetainParticipant` (runs post-index, then re-indexes).
`compiler/plc_ast/src/ast.rs`	Updates `Debug` formatting of `VariableBlock` to conditionally include `constant`/`retain` fields.
`compiler/plc_source/src/source_location.rs`	Minor simplification (`or_else` → `or`).
`compiler/plc_diagnostics/src/reporter/codespan.rs`	Minor simplification (`unwrap_or_else` → `unwrap_or`).
`src/lowering/calls.rs`	Fixes assignment into boxed operator (`*stmt.operator = ...`).
`src/parser/tests/variable_parser_tests.rs`	Adds parsing tests for `RETAIN` on variable blocks.
`src/tests/adr/vla_adr.rs`	Updates ADR expectations to include `is_retain: false`.
`src/tests/adr/pou_adr.rs`	Updates ADR expectations to include `is_retain: false`.
`src/resolver/tests/resolve_expressions_tests.rs`	Updates expected variable entry to include `is_retain: false`.
`src/index/tests/interface_tests.rs`	Updates expected variable entries to include `is_retain: false`.
`src/index/tests/index_tests.rs`	Adds retain-related index tests and updates expected entries with `is_retain`.
`src/codegen/tests/code_gen_tests.rs`	Adds `.retain` section assertions for retained globals/program instances and whitespace normalization.
`src/parser/tests/snapshots/rusty__parser__tests__variable_parser_tests__date_and_time_constants_test.snap`	Snapshot update due to `VariableBlock` debug now showing `constant: true`.
`src/lowering/snapshots/*.snap`	Snapshot updates adding `is_retain: false` in debug outputs.
`src/index/tests/snapshots/*.snap`	Snapshot updates adding `is_retain: false` in debug outputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

compiler/plc_lowering/src/retain.rs

src/codegen/generators/variable_generator.rs

src/typesystem.rs

…during parsing

Copilot

Pull request overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/codegen/generators/variable_generator.rs:194

Potential section override issue: when online_change is enabled, the section is set again on line 194, which will override the retain section set on line 175. Consider combining the retain section with the online_change section or checking if retain variables are compatible with online_change.

        if global_variable.should_retain(self.global_index) {
            global_ir_variable = global_ir_variable.make_retain();
        };

        let global_name = if global_variable.get_name().ends_with("instance") {
            global_variable.get_name()
        } else {
            global_variable.get_qualified_name()
        };
        let global_name = global_name.to_lowercase();

        if self.online_change.is_enabled() {
            let section = SectionMangler::variable(
                global_name,
                section_names::mangle_type(
                    self.global_index,
                    self.global_index.get_effective_type_by_name(global_variable.get_type_name())?,
                )?,
            )
            .mangle();
            global_ir_variable.set_section(Some(&section));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-27T07:48:09Z

src/codegen/tests/code_gen_tests.rs


        FUNCTION main
+        VAR
        VAR


Duplicate VAR declaration found. Remove one of these VAR keywords as it appears to be unintentional.

Suggested change

VAR

Copilot · 2026-02-27T07:48:10Z

compiler/plc_lowering/src/retain.rs

+            unit.global_vars
+                .iter_mut()
+                .find(|block| block.retain)
+                .map(|block| block.variables.append(&mut self.context.retain_variables))


Inconsistent handling of retain_variables Vec after appending. When an existing retain block is found and variables are appended, the retain_variables Vec is not cleared, which could lead to duplicates if visit_compilation_unit is called multiple times or if the Vec is used elsewhere. Consider using drain() consistently or explicitly clearing the Vec after append.

Suggested change

.map(|block| block.variables.append(&mut self.context.retain_variables))

.map(|block| {

block

.variables

.extend(self.context.retain_variables.drain(..));

})

volsa · 2026-03-06T08:02:14Z

src/codegen/tests/code_gen_tests.rs

+
+    %main = type { i16, [81 x i8] }
+
+    @main_instance = global %main zeroinitializer, section ".retain"


Variables in PROGRAMS that are marked as retain of have variables in their datatype that are retained are converted into a pointer to a global retain variable

From the comment I would have assumed we create internal global variables, mark them as retain and point to them. According to this codegen output, we make the whole instance retain instead?

good catch this seems wrong to me, this should have made variables pointers and created 2 new gobal vars

the funny thing is, the application examples we are using for testing did this correctly, maybe the codegen method here is not lowering?

Try running with test_utils::codegen perhaps

volsa · 2026-03-06T08:09:06Z

src/index.rs

    }
+
+    pub fn should_retain(&self, index: &Index) -> bool {
+        self.should_retain_recursive(index, BTreeSet::new())


Just curious, why a BTreeSet? Could've been a simply HashSet as well right?

I plead the 5th..
Honestly not sure anymore what the thought process here was..

volsa · 2026-03-06T08:12:16Z

compiler/plc_lowering/src/retain.rs

+//! This module handles indirection for retain variables in PROGRAMS
+//! It also moves retain variables that are declared in non retain blocks into retain blocks.


Burn some tokens and let your clanker write a nice module documentation here please. Like a small introduction as to what this module is responsible for with perhaps an example.

volsa · 2026-03-06T08:16:17Z

compiler/plc_lowering/src/retain.rs

+            if let Some(variable_index) =
+                self.index.find_variable(self.context.container_name.as_deref(), &[variable.get_name()])
+            {


This should always yield Some(...) and otherwise is a bug in our compiler. I would hard-unwrap here, or at the very least log an internal error.

volsa · 2026-03-06T08:25:34Z

compiler/plc_lowering/src/retain.rs

+            {
+                // If the variable should be retained
+                if variable_index.should_retain(&self.index) {
+                    if self.context.in_program {


Does retain only make sense for global variables and programs or stateful POUs (i.e. function blocks)? Because this logic only covers the former I believe?

volsa · 2026-03-06T08:26:38Z

compiler/plc_lowering/src/retain.rs

+    fn visit_variable_block(&mut self, block: &mut plc_ast::ast::VariableBlock) {
+        let variables = std::mem::take(&mut block.variables);
+        for mut variable in variables {
+            if let Some(variable_index) =


I feel like this can benefit from de-nesting in general / early returns, but probably more of a nit than anything important really.

volsa · 2026-03-06T08:29:24Z

compiler/plc_lowering/src/retain.rs

+                    } else if matches!(block.kind, plc_ast::ast::VariableBlockType::Global) && !block.retain {
+                        self.context.retain_variables.push(variable);


This covers nested datatypes? I'm just confused because the if says "!block.retain" but it's guarded by the should_retain check, so I'm assuming it has to with nested datatypes? Maybe a brief comment would help?

volsa · 2026-03-06T08:30:25Z

compiler/plc_lowering/src/retain.rs

+
+        let unit = &project.units[0];
+        // Expect the unit to have a global retain variable and the original variable to be replaced with a auto reference to the retain variable
+        assert_debug_snapshot!(unit, @r#"


Let's not be lazy here and make use of a helper function perhaps instead of snapshot-testing the whole unit?

volsa · 2026-03-06T08:32:38Z

compiler/plc_lowering/src/retain.rs

+                                    },
+                                ],
+                                variable_block_type: Local,
+                                retain: true,


The (now) replaced variable still contains retain information. Is this intentional given its now just a pointer to a retain variable?

ghaith added 3 commits February 10, 2026 13:52

feat: add support for retain variables in the indexer

1b6ce05

Added parser tests for retain variables Added index entries and test for retain variables in globals, programs and fbs.

feat: ensure nested retain variables are globally marked as retain

7d30dd6

A retain variable in a program is added to the global vars and a reference to the variable is added to the program If a variable contains a retain variable nested inside it is also treated as retain

feat: mark retain variables as such for the linker

aa40fb6

During codegen, if a global variable should be retained add it to the .retain linker section

ghaith requested a review from Copilot February 11, 2026 14:56

Copilot started reviewing on behalf of ghaith February 11, 2026 14:57 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

compiler/plc_lowering/src/retain.rs Outdated Show resolved Hide resolved

src/codegen/generators/variable_generator.rs Show resolved Hide resolved

src/typesystem.rs Outdated Show resolved Hide resolved

src/typesystem.rs Show resolved Hide resolved

ghaith and others added 8 commits February 17, 2026 09:22

Merge remote-tracking branch 'origin/master' into retain

a47861f

use pointer references instead of AT since AT/addresses are resolved …

469f344

…during parsing

add wiring test, make sure the var init is a reference member

6492412

fixed a merge conflict error

edc4b6f

Merge branch 'master' into retain

4a39b35

Merge branch 'master' into retain

696478c

Merge remote-tracking branch 'origin/master' into retain

6187dc2

run cargo fmt

941bd8e

ghaith requested review from Copilot and volsa February 27, 2026 07:37

Copilot started reviewing on behalf of ghaith February 27, 2026 07:41 View session

accept snapshots

0f4951f

Copilot AI reviewed Feb 27, 2026

View reviewed changes

ghaith and others added 2 commits February 27, 2026 09:35

more snapshot fails, clippy and fmt

efe7277

Merge branch 'master' into retain

1cf50d6

volsa reviewed Mar 6, 2026

View reviewed changes


		%main = type { i16, [81 x i8] }

		@main_instance = global %main zeroinitializer, section ".retain"

		//! This module handles indirection for retain variables in PROGRAMS
		//! It also moves retain variables that are declared in non retain blocks into retain blocks.

		} else if matches!(block.kind, plc_ast::ast::VariableBlockType::Global) && !block.retain {
		self.context.retain_variables.push(variable);

Conversation

ghaith commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants