Skip to content

Commit 8bddbd0

Browse files
ghaithclaude
andcommitted
refactor(lowering): MutationTracker (body vs signature) and drop AnnotationMap::into_any_box
Two related cleanups on `AggregateTypeLowerer`: - The `AnnotationMap::into_any_box` trait method existed only to round-trip a `Box<dyn AnnotationMap>` back to a concrete `AstAnnotations`. The field is now `Option<AstAnnotations>` directly; the trait method and its two impls are gone. - The hand-incremented `mutations: usize` counter is replaced by a `MutationTracker` that distinguishes body mutations (statement inserts) from signature rewrites. The visitor's mutation primitives (`push_pre_statement`, `push_post_statement`, signature-rewrite in `visit_pou`) call `touch()` / `signature_changed(pou_name)` themselves. The call site no longer has to remember to bump anything, and the per-POU recording means `AggregateTypeLowerer::post_annotate` only invalidates callers of POUs whose interface actually changed — not every POU in the touched unit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 47d4106 commit 8bddbd0

6 files changed

Lines changed: 183 additions & 56 deletions

File tree

compiler/plc_driver/src/pipelines/participant.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ use plc::{
1818
index::{Index, PouIndexEntry},
1919
lowering::{calls::AggregateTypeLowerer, polymorphism::PolymorphismLowerer},
2020
output::FormatOption,
21-
resolver::AstAnnotations,
2221
ConfigFormat, OnlineChange, Target,
2322
};
2423
use plc_diagnostics::diagnostics::Diagnostic;
@@ -365,18 +364,24 @@ impl PipelineParticipantMut for AggregateTypeLowerer {
365364

366365
let AnnotatedProject { mut units, index, annotations, diagnostics } = annotated_project;
367366
self.index = Some(index);
368-
self.annotation = Some(Box::new(annotations));
367+
self.annotation = Some(annotations);
369368

370369
// Walk each unit; bookkeeper records which were mutated and which
371370
// POU names had their signature rewritten (aggregate return →
372371
// VAR_IN_OUT). Constants enter the index because the new
373372
// VAR_IN_OUT parameter's type can be e.g. `STRING[K+1]`.
373+
//
374+
// `signature_changed_pous()` returns only POUs whose interface was
375+
// actually rewritten — body-only mutations (e.g. inserted alloca
376+
// prelude for an aggregate-returning call site) flip the dirty
377+
// flag but must not invalidate callers of every other POU in the
378+
// unit.
374379
let mut book = super::bookkeeping::LoweringBookkeeper::new();
375380
for (idx, annotated_unit) in units.iter_mut().enumerate() {
376381
if self.visit_unit_tracked(&mut annotated_unit.unit) {
377382
book.mark_unit(idx);
378-
for pou in &annotated_unit.unit.pous {
379-
book.signature_changed(&pou.name);
383+
for name in self.signature_changed_pous() {
384+
book.signature_changed(&name);
380385
}
381386
}
382387
}
@@ -388,10 +393,6 @@ impl PipelineParticipantMut for AggregateTypeLowerer {
388393
// borrowed them for the walk).
389394
let index = self.index.take().expect("index returned by visit");
390395
let annotations = self.annotation.take().expect("annotations returned by visit");
391-
let annotations = *annotations
392-
.into_any_box()
393-
.downcast::<AstAnnotations>()
394-
.expect("post_annotate participant always receives AstAnnotations");
395396

396397
let project = AnnotatedProject { units, index, annotations, diagnostics };
397398
book.apply_to_annotated(project, &reverse_deps, self.id_provider.clone())

src/lowering.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
pub mod calls;
22
pub mod helper;
3+
pub mod mutation;
34
pub mod polymorphism;
45
pub mod property;

src/lowering/calls.rs

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -62,28 +62,27 @@ use plc_source::source_location::SourceLocation;
6262

6363
use crate::{
6464
index::Index,
65-
lowering::helper::create_member_reference_with_location,
66-
resolver::{AnnotationMap, StatementAnnotation},
65+
lowering::{helper::create_member_reference_with_location, mutation::MutationTracker},
66+
resolver::{AnnotationMap, AstAnnotations, StatementAnnotation},
6767
typesystem::{DataType, DataTypeInformation},
6868
};
6969

7070
// Performs lowering for aggregate types defined in functions
7171
#[derive(Default)]
7272
pub struct AggregateTypeLowerer {
7373
pub index: Option<Index>,
74-
pub annotation: Option<Box<dyn AnnotationMap>>,
74+
pub annotation: Option<AstAnnotations>,
7575
pub id_provider: IdProvider,
7676
/// New statements to be added during visit, that should happen before the call. This should always be drained when read
7777
pre_stmts: Vec<Vec<AstNode>>,
7878
/// New statements to be added during visit, that should happen after the call. This should always be drained when read
7979
post_stmts: Vec<Vec<AstNode>>,
8080
counter: AtomicI32,
81-
/// Incremented every time the lowerer mutates the AST in a way that
82-
/// affects the global index (POU signature changes) or the annotations
83-
/// (new pre/post statements inserted). Compared before / after a unit
84-
/// visit so the participant can decide whether to re-index / re-annotate
85-
/// that unit.
86-
mutations: usize,
81+
/// Set whenever the lowerer mutates the AST in a way that affects the
82+
/// global index (POU signature changes) or the annotations (new pre/post
83+
/// statements inserted). Reset before each unit walk so the participant
84+
/// can decide per unit whether to re-index / re-annotate.
85+
mutations: MutationTracker,
8786
}
8887

8988
impl AggregateTypeLowerer {
@@ -102,10 +101,21 @@ impl AggregateTypeLowerer {
102101
/// Visits a single unit and returns whether the lowerer actually mutated
103102
/// it. The caller uses the result to scope re-indexing / re-annotation
104103
/// to only the units that need it.
104+
///
105+
/// Pair with [`signature_changed_pous`](Self::signature_changed_pous) to
106+
/// learn *which* POUs in this unit had their signature rewritten — body
107+
/// edits flip the dirty flag but do not invalidate callers of every POU
108+
/// in the unit.
105109
pub fn visit_unit_tracked(&mut self, unit: &mut CompilationUnit) -> bool {
106-
let before = self.mutations;
110+
self.mutations.reset();
107111
self.visit_compilation_unit(unit);
108-
self.mutations > before
112+
self.mutations.is_dirty()
113+
}
114+
115+
/// Names of POUs whose public signatures were rewritten during the most
116+
/// recent [`visit_unit_tracked`](Self::visit_unit_tracked) call.
117+
pub fn signature_changed_pous(&self) -> Vec<String> {
118+
self.mutations.signature_changed_pous()
109119
}
110120

111121
fn steal_and_walk_list(&mut self, list: &mut Vec<AstNode>) {
@@ -127,7 +137,7 @@ impl AggregateTypeLowerer {
127137
fn push_pre_statement(&mut self, stmt: AstNode) {
128138
if let Some(stmts) = self.pre_stmts.last_mut() {
129139
stmts.push(stmt);
130-
self.mutations += 1;
140+
self.mutations.touch();
131141
} else {
132142
unreachable!("Statement lists should exist at this point");
133143
}
@@ -140,7 +150,7 @@ impl AggregateTypeLowerer {
140150
fn push_post_statement(&mut self, stmt: AstNode) {
141151
if let Some(stmts) = self.post_stmts.last_mut() {
142152
stmts.push(stmt);
143-
self.mutations += 1;
153+
self.mutations.touch();
144154
} else {
145155
unreachable!("Statement lists should exist at this point");
146156
}
@@ -192,7 +202,7 @@ impl AstVisitorMut for AggregateTypeLowerer {
192202
// If the return type is aggregate, remove it from the signature and add a matching variable
193203
// in a VAR_IN_OUT block
194204
if index.get_effective_type_or_void_by_name(&return_type_name).is_aggregate_type() {
195-
self.mutations += 1;
205+
self.mutations.signature_changed(&pou.name);
196206
let original_return = pou.return_type.take().unwrap();
197207
let location = original_return.get_location();
198208
//Create a new return type for the pou
@@ -374,8 +384,8 @@ impl AstVisitorMut for AggregateTypeLowerer {
374384
.iter()
375385
.enumerate()
376386
.any(|(param_index, param)|
377-
is_output_assignment_and_type_cast_needed(param, annotation.as_ref(), index, &qualified_name, param_index, function_entry.is_method())
378-
|| is_output_assignment_and_has_direct_access(param, annotation.as_ref(), index, &qualified_name, param_index, function_entry.is_method()))
387+
is_output_assignment_and_type_cast_needed(param, annotation, index, &qualified_name, param_index, function_entry.is_method())
388+
|| is_output_assignment_and_has_direct_access(param, annotation, index, &qualified_name, param_index, function_entry.is_method()))
379389
// Stateful structs (such as function blocks) have their own output assignment mechanism in codegen,
380390
// and are not handled by this lowerer
381391
&& (function_entry.is_function() || function_entry.is_method())
@@ -392,7 +402,7 @@ impl AstVisitorMut for AggregateTypeLowerer {
392402
(&self.counter, self.id_provider.clone()),
393403
param,
394404
(&qualified_name, &return_name, function_entry.is_method()),
395-
annotation.as_ref(),
405+
annotation,
396406
index,
397407
param_index,
398408
(&mut pre_statements, &mut expressions, &mut post_statements),
@@ -803,6 +813,7 @@ mod tests {
803813

804814
use crate::index::indexer;
805815
use crate::lowering::calls::AggregateTypeLowerer;
816+
use crate::resolver::AstAnnotations;
806817
use crate::test_utils::tests::{
807818
annotate_and_lower_with_ids, annotate_with_ids, index as test_index, index_and_lower,
808819
index_unit_with_id, index_with_ids,
@@ -972,7 +983,7 @@ mod tests {
972983
lowerer.visit_compilation_unit(&mut unit);
973984
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
974985
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
975-
lowerer.annotation.replace(Box::new(annotations));
986+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
976987
lowerer.visit_compilation_unit(&mut unit);
977988
assert_debug_snapshot!(unit.implementations[1]);
978989
}
@@ -1013,7 +1024,7 @@ mod tests {
10131024
lowerer.visit_compilation_unit(&mut unit);
10141025
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
10151026
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1016-
lowerer.annotation.replace(Box::new(annotations));
1027+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
10171028
lowerer.visit_compilation_unit(&mut unit);
10181029
assert_debug_snapshot!(unit.implementations[2]);
10191030
}
@@ -1051,7 +1062,7 @@ mod tests {
10511062
lowerer.visit_compilation_unit(&mut unit);
10521063
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
10531064
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1054-
lowerer.annotation.replace(Box::new(annotations));
1065+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
10551066
lowerer.visit_compilation_unit(&mut unit);
10561067
assert_debug_snapshot!(unit.implementations[1]);
10571068
}
@@ -1091,7 +1102,7 @@ mod tests {
10911102
lowerer.visit_compilation_unit(&mut unit);
10921103
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
10931104
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1094-
lowerer.annotation.replace(Box::new(annotations));
1105+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
10951106
lowerer.visit_compilation_unit(&mut unit);
10961107
assert_debug_snapshot!(unit.implementations[1]);
10971108
}
@@ -1131,7 +1142,7 @@ mod tests {
11311142
lowerer.visit_compilation_unit(&mut unit);
11321143
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
11331144
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1134-
lowerer.annotation.replace(Box::new(annotations));
1145+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
11351146
lowerer.visit_compilation_unit(&mut unit);
11361147
assert_debug_snapshot!(unit.implementations[1]);
11371148
}
@@ -1171,7 +1182,7 @@ mod tests {
11711182
lowerer.visit_compilation_unit(&mut unit);
11721183
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
11731184
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1174-
lowerer.annotation.replace(Box::new(annotations));
1185+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
11751186
lowerer.visit_compilation_unit(&mut unit);
11761187
assert_debug_snapshot!(unit.implementations[1]);
11771188
}
@@ -1214,7 +1225,7 @@ mod tests {
12141225
lowerer.visit_compilation_unit(&mut unit);
12151226
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
12161227
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1217-
lowerer.annotation.replace(Box::new(annotations));
1228+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
12181229
lowerer.visit_compilation_unit(&mut unit);
12191230
assert_debug_snapshot!(unit.implementations[1]);
12201231
}
@@ -1252,7 +1263,7 @@ mod tests {
12521263
lowerer.visit_compilation_unit(&mut unit);
12531264
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
12541265
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1255-
lowerer.annotation.replace(Box::new(annotations));
1266+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
12561267
lowerer.visit_compilation_unit(&mut unit);
12571268
assert_debug_snapshot!(unit.implementations[1]);
12581269
}
@@ -1290,7 +1301,7 @@ mod tests {
12901301
lowerer.visit_compilation_unit(&mut unit);
12911302
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
12921303
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1293-
lowerer.annotation.replace(Box::new(annotations));
1304+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
12941305
lowerer.visit_compilation_unit(&mut unit);
12951306
assert_debug_snapshot!(unit.implementations[1]);
12961307
}
@@ -1330,7 +1341,7 @@ mod tests {
13301341
lowerer.visit_compilation_unit(&mut unit);
13311342
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
13321343
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1333-
lowerer.annotation.replace(Box::new(annotations));
1344+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
13341345
lowerer.visit_compilation_unit(&mut unit);
13351346
assert_debug_snapshot!(unit.implementations[1]);
13361347
}
@@ -1371,7 +1382,7 @@ mod tests {
13711382
lowerer.visit_compilation_unit(&mut unit);
13721383
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
13731384
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1374-
lowerer.annotation.replace(Box::new(annotations));
1385+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
13751386
lowerer.visit_compilation_unit(&mut unit);
13761387
assert_debug_snapshot!(unit.implementations[1]);
13771388
}
@@ -1399,7 +1410,7 @@ mod tests {
13991410
lowerer.visit_compilation_unit(&mut unit);
14001411
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
14011412
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1402-
lowerer.annotation.replace(Box::new(annotations));
1413+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
14031414
lowerer.visit_compilation_unit(&mut unit);
14041415
assert_debug_snapshot!(unit.implementations[0]);
14051416
}
@@ -1627,7 +1638,7 @@ mod tests {
16271638
lowerer.visit_compilation_unit(&mut unit);
16281639
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
16291640
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1630-
lowerer.annotation.replace(Box::new(annotations));
1641+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
16311642
lowerer.visit_compilation_unit(&mut unit);
16321643
assert_debug_snapshot!(unit.implementations[2].statements[0]);
16331644
}
@@ -1664,7 +1675,7 @@ mod tests {
16641675
lowerer.visit_compilation_unit(&mut unit);
16651676
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
16661677
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1667-
lowerer.annotation.replace(Box::new(annotations));
1678+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
16681679
lowerer.visit_compilation_unit(&mut unit);
16691680
assert_debug_snapshot!(unit.implementations[2].statements[0]);
16701681
}
@@ -1708,7 +1719,7 @@ mod tests {
17081719
lowerer.visit_compilation_unit(&mut unit);
17091720
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
17101721
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1711-
lowerer.annotation.replace(Box::new(annotations));
1722+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
17121723
lowerer.visit_compilation_unit(&mut unit);
17131724

17141725
let implementations = &unit.implementations;
@@ -1765,7 +1776,7 @@ mod tests {
17651776
lowerer.visit_compilation_unit(&mut unit);
17661777
lowerer.index.replace(index_unit_with_id(&unit, id_provider.clone()));
17671778
let annotations = annotate_with_ids(&unit, lowerer.index.as_mut().unwrap(), id_provider.clone());
1768-
lowerer.annotation.replace(Box::new(annotations));
1779+
lowerer.annotation.replace(AstAnnotations::new(annotations, id_provider.clone().next_id()));
17691780
lowerer.visit_compilation_unit(&mut unit);
17701781

17711782
let implementations = &unit.implementations;

0 commit comments

Comments
 (0)