Skip to content

Support for retain sections#1592

Open
ghaith wants to merge 13 commits intomasterfrom
retain
Open

Support for retain sections#1592
ghaith wants to merge 13 commits intomasterfrom
retain

Conversation

@ghaith
Copy link
Collaborator

@ghaith ghaith commented Feb 11, 2026

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
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_elseor).
compiler/plc_diagnostics/src/reporter/codespan.rs Minor simplification (unwrap_or_elseunwrap_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.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


FUNCTION main
VAR
VAR
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Suggested change
VAR

Copilot uses AI. Check for mistakes.
unit.global_vars
.iter_mut()
.find(|block| block.retain)
.map(|block| block.variables.append(&mut self.context.retain_variables))
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(..));
})

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants