perf(driver): scope lowering invalidation per unit#1739
Draft
ghaith wants to merge 14 commits into
Draft
Conversation
Build Artifacts🐧 Linux
From workflow run 🪟 Windows
From workflow run |
Adds a small RAII timer (`PhaseTimer`) that records wall-clock time for each stage of `BuildPipeline` and for every participant invocation (`pre_index`, `post_index`, `pre_annotate`, `post_annotate`). The inner `ParsedProject::index` and `IndexedProject::annotate` methods self-time, so when a participant triggers an implicit whole-project re-index or re-annotate from inside one of its hooks, the cost shows up nested under the participant that caused it. The instrumentation is gated on `PLC_TIMING=1`. With the env var unset (the default, including CI), the timers compile to a near-noop and emit nothing — no behavioural change to existing tests, no diagnostic drift, no stdout/stderr noise. `PhaseTimer::new` takes a `&'static str` for static labels, `PhaseTimer::new_with(|| format!(...))` defers label allocation for dynamic labels so the instrumentation doesn't perturb the disabled-state baseline. `PipelineParticipant` and `PipelineParticipantMut` gain a `name()` method, used as the timer label. The default returns the implementing type's short name (generics stripped); participants can override if needed. Documented in `book/src/development/phase_timing.md`. `scripts/oscat_multi_split.py` is bundled for benchmarking against real multi-unit libraries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the infrastructure that lets the index drop a single source unit's contributions without rebuilding the whole project. - `UnitId` newtype (with `BUILTIN`, `SYNTHETIC`, `UNTAGGED` sentinels) tags every entry imported into the global `Index`. The typed `UnitId::source_index() -> Option<usize>` accessor encapsulates the conversion back to a positional index so non-source ids can't slip through. - `Index::import_with_unit(other, unit_id)` is the new primary import path; it records each entry's `(SymbolKind, map_key, identifier)` in an `Index.unit_symbols` side-table. The existing `Index::import` becomes a thin wrapper that passes `UnitId::UNTAGGED`. - `Index::remove_unit(unit_id)` walks the side-table and drops only the entries that unit contributed, leaving other units' entries under shared map keys (e.g. an enum variant name appearing in two unrelated enums) untouched. - Shared entries (auto-generated POUs and implementations whose names collide across compilation units) are reference-counted via `UnitSymbolIndex::has_owner_other_than`: every importer records a stake, and `remove_unit` only drops the underlying entry when the last owner is removed. Without this, the first removal of a shared entry deletes it and orphans the remaining units' ownership rows across reindex cycles. - `SymbolMap` gets `retain_at_key` so removals can filter entries precisely instead of dropping a whole key. - The driver's index pipeline tags source units with `UnitId::source(i)`, built-in functions with `UnitId::BUILTIN`, and the resolver's `new_index` with `UnitId::SYNTHETIC`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`ReverseDependencyGraph` inverts each unit's `FxIndexSet<Dependency>` that the resolver already collects into `symbol_name → Set<UnitId>`. Lets partial re-annotation compute the "who used the symbol before its signature changed?" closure: when a lowerer rewrites a unit's signature, the units that must be re-annotated are exactly the union of dependents of every changed symbol, plus the mutated units themselves. The closure is correct only if the resolver records a `Dependency` for every cross-unit reference — including implicit ones through interface dispatch and vtable indirection. The contract on which resolver paths must record a `Dependency` (and which paths are still open risks — cross-unit generic monomorphization is the documented gap) lives in `book/src/development/reverse_dependency_graph.md`. `cross_unit_interface_dispatch_signature` exercises the interface-dispatch path: an interface method with aggregate (STRING[16]) return type, implemented in a second compilation unit and dispatched from a third. AggregateTypeLowerer rewrites all three units' view of the signature; if the closure misses any, the call site stays on the pre-rewrite calling convention and breaks at runtime. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each lowering participant that previously triggered a whole-project re-index or re-annotate after its hook fired now gates on whether the project actually has any work for it. The headline measurement on oscat: the `annotate (driver)` phase drops from 605 ms to 390 ms (-35%) with no behavioural changes — the saved ~250 ms is purely redundant work that fired on every build regardless of the project's shape. Participants migrated: * `ArrayLowerer::pre_annotate` — `lower_literal_arrays` now returns `bool`. Re-index is skipped when no array literal was lowered. * `RetainParticipant::post_index` — `lower_retain` returns `bool` driven by an internal `changed` flag on the visitor. The lowerer now borrows the index instead of consuming it so the caller can hand back the unchanged `IndexedProject` on a no-op. * `PolymorphismLowerer::post_index` — `table()`, `TableGenerator::generate`, and the inner vtable / itable generators now return `bool`. Re-index skipped on projects with no classes, function blocks, or interfaces. * `PolymorphismLowerer::post_annotate`, `AggregateTypeLowerer::post_annotate`, `InheritanceLowerer::post_annotate`, `PropertyLowerer::post_annotate` — gated on exact-predicate prechecks against the `Index`. Threading a `changed` flag through the dispatch / aggregate / inheritance / property visitors would be far more invasive, and the prechecks are exact for the conditions they check, so the outcome is the same. Supporting additions: `Index::has_any_properties`, `SymbolMap::is_empty`. The participant trait signature is unchanged; the bool-returning lowerer functions are an internal contract between each lowerer and its participant hook. Workspace test suite and lit suite both stay green. The remaining ~390 ms in oscat's `annotate` phase is dominated by participants that genuinely have work to do (oscat uses classes / FBs and aggregate return types); reducing those needs per-unit deltas from the lowerers, which is the Phase 4+ work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`TableGenerator::generate`, `VirtualTableGenerator::generate`, and `InterfaceTableGenerator::generate` now report the positional indices of units they actually modified instead of a single `bool`. `PolymorphismLowerer::post_index` uses that list to re-index only the touched units via a new `Index::reindex_unit(unit_id, unit, ids)` helper that runs `pre_process` + `indexer::index` and replaces the unit's contributions in the global index in place. Previously the participant re-indexed the whole project after vtable generation. The savings scale with the number of unmodified units in the project. Adds the `cross_unit_polymorphism_dispatch` lit test exercising cross-unit vtable generation and dispatch (`Base` and `Derived` in separate compilation units, dispatching through a base pointer). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`AggregateTypeLowerer::post_annotate` used to re-index and re-annotate the entire project whenever any POU returned an aggregate type. It now: * tracks per-unit mutations via a counter the visitor bumps on POU signature rewrites and pre/post statement insertions, * re-indexes only the units that were actually rewritten (via the Phase-1 `Index::reindex_unit` primitive plus a `evaluate_constants` pass to settle freshly-imported `ConstId`-backed expressions), * re-annotates the closure of mutated units plus their dependents in parallel through a new `AnnotatedProject::reannotate_units` method that mirrors the main annotate path's par_iter. The closure is computed by `compute_reannotate_closure`: for each mutated unit, every dependent recorded against any of that unit's POU names in the Phase-2 reverse-dependency graph is added. This is the correctness fix the user flagged — a POU whose signature changes invalidates the annotations of every unit that called it, not just the unit that owns the POU. Supporting changes: * `AnnotationMap` gains `into_any_box(self: Box<Self>) -> Box<dyn Any>` so the post_annotate participant can recover the concrete `AstAnnotations` after the visitor took ownership through the trait object. * `AggregateTypeLowerer::visit_unit_tracked` is a new public entry-point that returns whether the visitor mutated the passed-in unit. * New lit test `tests/lit/multi/cross_unit_aggregate_signature/` declares a `STRING[16]`-returning function in one compilation unit and calls it from another, asserting that the caller's annotations are refreshed against the new VAR_IN_OUT signature. Headline number on multi-file oscat: annotate phase median drops from 162 ms (post-Phase-3) to 152 ms (post-Phase-4 step 2). Full workspace test suite and lit suite both stay green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both lowerers already short-circuited the re-index when they had no work to do (Phase 3 bool gates). They now report `Vec<usize>` of positional indices instead, and the participant uses `Index::reindex_unit` to refresh only those units' entries. `lower_retain` resets its visitor's `changed` flag per unit so the returned list reflects which units actually had a retain variable hoisted or a retain block demoted. `lower_literal_arrays` already returned a per-unit bool; the caller now collects the truthful set instead of an or-reduced flag. Neither rewrite changes externally-visible symbol shapes (retain hoisting adds new globals, array lowering adds VAR_TEMP counter variables — both local additions), so no reverse-dep closure is needed here. The wins are small on oscat because both gates short-circuit, but the infrastructure is now uniform across all migrated participants and Phase 5 can drive any of them from a file-edit trigger. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A lowering pass that mutates compilation units has to invalidate downstream state in lockstep: re-index the units it touched, run `evaluate_constants` if new ConstId-backed expressions appeared, compute the reverse-dependency closure of signature changes, and partial-re-annotate the closure. Every per-unit-reindexing participant Phases 3–4 added rolled this recipe by hand, with subtle drift between copies — `compute_reannotate_closure` lived as a free function in `participant.rs`, `evaluate_constants` was explicit at the call site, and each participant remembered (or forgot) to capture the reverse-dep graph before mutation. `compiler/plc_driver/src/pipelines/bookkeeping.rs` centralises the recipe. A participant accumulates effects against a `LoweringBookkeeper` (`mark_unit`, `signature_changed`, `mark_const_introduced`) then hands the project off: * `apply_to_indexed` — for participants that fire in pre/post_index or pre_annotate. Re-indexes mutated units, optionally runs `evaluate_constants`. * `apply_to_annotated` — for post_annotate participants. Adds the reverse-dep closure + parallel `reannotate_units` step on top. Requires the caller to pass a `ReverseDependencyGraph` captured *before* the mutation (the graph asks "who used the symbol before its signature changed?"). `AggregateTypeLowerer::post_annotate` is migrated to use the bookkeeper as the worked example — body drops from ~70 lines of bespoke invalidation plumbing to ~15 lines describing what changed. The now-orphaned `compute_reannotate_closure` free function is removed (the bookkeeper absorbs it). No trait changes. `PipelineParticipantMut` is untouched. Workspace tests, lit suite, and the multi-file oscat baseline are bit-identical to the post-Phase-4 numbers. PolymorphismLowerer's table reindex, Retain, and Array all still use their inline per-unit code — migrating each is a small follow-up and entirely opt-in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A new participant author had to implement `PipelineParticipantMut`'s
four hooks, pick the right one for their stage, destructure
`ParsedProject` / `IndexedProject` / `AnnotatedProject` correctly,
and remember the bookkeeping invariants (re-index touched units,
`evaluate_constants` after, reverse-dep closure for re-annotation,
`UnitId::SYNTHETIC` for the resolver's `new_index`). The
`LoweringBookkeeper` from the previous commit collapses the
bookkeeping; this commit adds the trait that collapses the rest.
`compiler/plc_driver/src/pipelines/unit_lowerer.rs`:
* `LoweringStage` enum (`PreIndex` / `PostIndex` / `PreAnnotate` /
`PostAnnotate`) — where in the pipeline the lowerer fires.
* `LoweringContext { index, annotations, ids }` — read-only context
handed to each `lower_unit` call. `annotations` is `Some` only on
`PostAnnotate`.
* `UnitLowerer` trait with one required method: `lower_unit(&mut self,
&mut CompilationUnit, &LoweringContext) -> UnitChange`. Optional
`name()` and `diagnostics()` with sensible defaults.
* `UnitChange { mutated, changed_signatures, introduces_constants }` —
what the lowerer touched. `UnitChange::none()` and
`UnitChange::mutated()` are the common constructors.
* `AutoLowerer<T: UnitLowerer>` wraps a `UnitLowerer` and implements
`PipelineParticipantMut`. Constructor takes the lowerer, the stage,
and the shared `IdProvider`. At its stage, the adapter walks all
compilation units, calls `T::lower_unit` per unit, aggregates the
resulting `UnitChange`s into a `LoweringBookkeeper`, and hands the
project off to `apply_to_indexed` or `apply_to_annotated`. `name()`
forwards through so phase-timing labels stay readable.
Worked migration: `RetainParticipant`'s `post_index` path. Added a
new `lower_one_unit` method on `RetainParticipant` for use by adapters
(the existing `lower_retain` stays). A thin `RetainUnitLowerer`
wrapper in `participant.rs` implements `UnitLowerer` by delegating to
`lower_one_unit`. The participant list in `get_default_mut_participants`
now registers `AutoLowerer::new(RetainUnitLowerer::new(...), PostIndex,
ids)` instead of `RetainParticipant::new(...)` directly. The old
`impl PipelineParticipantMut for RetainParticipant` is removed.
`PipelineParticipantMut` is untouched. The other participants
(PolymorphismLowerer, AggregateTypeLowerer, InheritanceLowerer,
ArrayLowerer, PropertyLowerer, etc.) keep their existing impls.
Migrating each is opt-in and entirely additive.
Workspace tests, lit suite, multi-file oscat baseline all
bit-identical to the Phase 4.1 numbers (~150 ms annotate median).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Some lowerers need to scan every unit before they can transform any individual one — collect every POU returning `REFERENCE TO`, build a dispatch table from class hierarchies, or otherwise compute a project-wide context. The pattern is two-pass: gather, then transform. `UnitLowerer` now has an optional `prepare(&[&CompilationUnit], &LoweringContext)` method (no-op by default). The `AutoLowerer` adapter calls `prepare` once before walking units for `lower_unit`. At each stage the adapter builds a `Vec<&CompilationUnit>` view — cheaper than cloning the units, works uniformly across `ParsedProject` (plain units), `IndexedProject` (plain units), and `AnnotatedProject` (`AnnotatedUnit` wrappers). Lowerers that don't need the pre-pass get nothing — the default impl is empty. `RetainParticipant`'s adapter and any other existing `UnitLowerer` keep working unchanged. Adds a `PrepareCounter` test fixture demonstrating the contract: prepare sees every unit before any `lower_unit` call, and a panic asserts the ordering at runtime for any lowerer that mirrors the pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`ArrayLowerer`'s `pre_annotate` body — destructure `IndexedProject`, walk units, collect mutated indices, optional re-index — collapses into a 3-line `ArrayUnitLowerer::lower_unit` plus a registration swap. The bookkeeping (per-unit re-index when any unit was mutated) moves into `AutoLowerer`. `ArrayLowerer::lower_one_unit(&mut CompilationUnit, &Index) -> bool` is added in parallel with the existing `array_lowering` free function — single-unit entry point for adapters. The existing project-wide `array_lowering::lower_literal_arrays` stays available. The thin `ArrayUnitLowerer` wrapper in `participant.rs` implements `UnitLowerer` by delegating to `lower_one_unit`. Registered via `AutoLowerer::new(ArrayUnitLowerer::new(...), PreAnnotate, ids)` in `get_default_mut_participants`, replacing the previous direct registration. The old `impl PipelineParticipantMut for ArrayLowerer` is removed. `PipelineParticipantMut` is still untouched. Workspace tests, lit suite, and the multi-file oscat baseline are bit-identical to Phase 4.3. Behaviour parity verified by the literal-array lit tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hared instance) `PolymorphismLowerer::post_index` (the vtable / itable table pass) now runs through a `UnitLowerer` wrapper while the surviving `PolymorphismLowerer` participant keeps its `post_annotate` dispatch impl plus the diagnostics bookkeeping. Both halves share the *same* `PolymorphismLowerer` state via `Rc<RefCell<PolymorphismLowerer>>` — a previous attempt that constructed a fresh `PolymorphismLowerer` inside the wrapper diverged silently from the dispatch half and dropped `__vtable__ctor(self.__vtable)` from generated constructor bodies (13 initializer snapshots broken). The shared-instance shape is documented as a pattern for future two-hook participants. The new `cross_unit_vtable_ctor_in_member` lit test gates this contract: a `Wrapper` FB holding a `Derived` as a member field; dispatch through a `POINTER TO Base` aimed at the embedded member. If the member's vtable is not initialised (the failure mode of the reverted split), the dispatch lands on uninitialised memory and the CHECK line fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documents the developer-facing API surface introduced by Phase 4: * When to pick `UnitLowerer` vs `PipelineParticipantMut` vs the bookkeeper helper. * Minimal `UnitLowerer` example with registration. * `prepare()` two-pass pattern for lowerers that gather a project-wide context before transforming individual units. * `Rc<RefCell<...>>` shared-state pattern for lowerers that have meaningful work in two pipeline stages (the PolymorphismLowerer table-+-dispatch case, including a sidebar on why the obvious "two independent instances" design breaks `InitParticipant`). * `LoweringBookkeeper` direct usage for lowerers whose visitor state pattern doesn't fit the `UnitLowerer` shape. * What the framework does for you on the invalidation side (per-unit `reindex_unit`, `evaluate_constants`, closure construction, parallel `reannotate_units`) and what it expects from your `UnitChange` reporting. Links to the existing phase-timing chapter so users have a path to the debugging trace. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tationMap::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>
cfe3819 to
8bddbd0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this PR exists
Today every lowering participant that mutates the AST follows it up by
re-indexing and re-annotating the whole project, even on a clean
full build. That makes the wall-clock cost of
parse → index → annotatedominated by redundant re-passes inside the participantchain. It's also the structural blocker to ever making incremental
rebuilds work.
This branch reshapes the lowering pipeline around per-unit invalidation
deltas, drops the redundant re-passes where the participant can prove
it didn't touch anything, and lays down the infrastructure (unit
provenance, reverse-dep graph) that a future LSP / incremental driver
will consume.
It is intentionally posted here as an RFC-style PR — I'd like
feedback on the API shape before landing. See the "Open for review"
section at the bottom.
Where the win comes from
The savings stack from three sources, in rough order of contribution:
properties, no
RETAIN, no aggregate-returning POUs, etc., thecorresponding participant returns early instead of doing a
whole-project re-index / re-annotate.
subset of units. The polymorphism table pass, the retain
rewrite, and the array-literal rewrite each re-index just the
units they touched rather than the whole project.
AggregateTypeLowerer(and any future signature-rewriting lowererusing the
UnitLowereradapter) re-annotates only the closure ofunits that depended on the rewritten signatures, using a
reverse-dependency graph built from the resolver's existing
Dependencyset. Signature changes are tracked per-POU, so bodyedits don't ripple to callers of unrelated POUs in the same unit.
Projects vary in how much each lever fires — projects with no
polymorphism, no aggregate returns, and no retain variables see most
of the wall-clock saving. Polymorphism-heavy projects still pay a
full re-index + full re-annotate in
PolymorphismDispatchAdapter::post_annotate(the clearest nexttarget after this PR).
PLC_TIMING=1 plc --check ...reports per-phase timings if you wantto measure on a local project.
What's in the PR, step by step
The branch reads cleanly commit-by-commit. Each conventional-commit
subject covers one slice; the bullets below summarise what each slice
does and why.
1. Per-phase pipeline timing
A new
PhaseTimerRAII guard wraps the four top-level pipeline phasesand every participant invocation. Enabled by
PLC_TIMING=1; logs tostderr with nesting indent. No behaviour change when disabled
(
PhaseTimer::new_with(|| format!(...))defers label allocationuntil the env var is set, so the instrumentation doesn't perturb the
disabled baseline). Documented in
book/src/development/phase_timing.md.2. Per-unit symbol ownership on
IndexAdds a
UnitSymbolIndexside-table onIndexmappingUnitId → Vec<OwnedSymbol>.Index::import_with_unit(unit_idx)tags theimported entries;
Index::remove_unit(unit_idx)drops every entry aspecific unit contributed (POUs, globals, types, implementations,
enum globals, initializers).
Shared entries (auto-generated POUs and implementations whose names
collide across compilation units) are reference-counted via
UnitSymbolIndex::has_owner_other_than: every importer records astake, and
remove_unitonly drops the underlying entry when thelast owner is removed. Without this, the first removal of a shared
entry deletes it and orphans the remaining units' ownership rows.
The side-table approach was chosen over adding a
unit_idfield onevery entry struct: there are ~344 entry-construction sites in the
indexer and a per-entry field would force them all to thread an ID
through. The side-table is one new field on
Indexand oneconstruction site per import. Tradeoff: an
OwnedSymbol { kind, map_key, identifier }indirection per entry instead of a directfield.
UnitIdsentinels:BUILTIN,SYNTHETIC(for the resolver'snew_index),UNTAGGED(for legacy code paths during thetransition).
UnitId::source_index() -> Option<usize>is the typedaccessor for places that previously paired
is_source()withraw() as usize.3. Reverse-dependency graph
ReverseDependencyGraphinverts the per-unitFxIndexSet<Dependency>the resolver already collects intosymbol_name → Set<UnitId>. Lets partial re-annotation compute the"who used the symbol before its signature changed?" closure.
The closure is correct only if the resolver records a
Dependencyfor every cross-unit reference — including implicit ones through
interface dispatch and vtable indirection. The contract on which
resolver paths must record a
Dependency(and which paths are stillopen risks — cross-unit generic monomorphization is the documented
gap) lives in
book/src/development/reverse_dependency_graph.md.4. Precheck gates on each participant
The most-broadly-applicable near-term win. Each participant that
previously unconditionally re-indexed / re-annotated the whole project
now first asks a cheap index predicate (
has_any_properties, noRETAIN, etc.) or aboolfrom the lowerer'svisit. If the answeris "no work", the implicit whole-project re-pass is skipped.
Touches:
PropertyLowerer,InheritanceLowerer,ArrayLowerer,RetainParticipant,PolymorphismLowerer,LoopDesugarer.5. Per-unit re-index after polymorphism table
PolymorphismLowerer::post_indexpreviously re-indexed the wholeproject after generating vtable types. Now it tracks which units the
table generator touched and only re-indexes those.
Adds
Index::reindex_unit(unit_id, &mut unit)— callspre_process(unit)to mirrorParsedProject::index, removes theold entries, imports the new ones.
6. Partial re-annotation after AggregateTypeLowerer
The biggest single behavioural change in the chain. After the
aggregate-return rewrite mutates a unit's POU signatures, only the
units in the reverse-dep closure of those signatures are
re-annotated; the rest reuse their existing annotations.
Adds
AnnotatedProject::reannotate_units, the closure builder, andruns
evaluate_constantsafter the partial reindex (the newVAR_IN_OUTparameters can haveSTRING[K+1]-style sizes that needconst-folding).
This is the change with the largest behavioural surface; end-to-end
checks are the new cross-unit lit tests (see Verification).
7. Per-unit re-index for Retain + Array
Same pattern as step 5 applied to the two smaller lowerers. Uniform
shape across all
bool-returning participants.8. LoweringBookkeeper helper
Extracts the recurring "track mutated units, track signature changes,
compute closure, run reindex + evaluate_constants + reannotate"
sequence into a
LoweringBookkeeperwith two terminal methods:apply_to_indexed(IndexedProject)andapply_to_annotated(AnnotatedProject). Participants build abookkeeper and hand it the project; the orchestration lives in one
place.
9. UnitLowerer trait + AutoLowerer adapter
API ergonomics for new lowerer authors. A
UnitLowerertrait whoselower_unitreturns aUnitChange { mutated, signature_changed };AutoLowerer<T>adapts aUnitLowererinto aPipelineParticipantMutby walking the units, collecting changesinto a
LoweringBookkeeper, and applying it. The author writes onemethod per unit; the bookkeeping is automatic.
10.
UnitLowerer::preparefor two-pass lowerersSome lowerers need a project-wide context-gathering pass before they
can rewrite a unit (the polymorphism dispatch table is the
motivating case).
prepare(&mut self, ctx)runs once before theper-unit
lower_unitloop; default implementation is a no-op.11. ArrayLowerer migrated to UnitLowerer
Smallest migration — used as the canary for the new trait. No
behaviour change.
12. PolymorphismLowerer table migrated, with shared instance
The polymorphism participant has two hooks — a
post_indextablepass that emits vtables/itables, and a
post_annotatedispatch passthat consumes them. Both must see the same
PolymorphismLowererstate (vtable instance names hashed with source path, IdProvider
sequence, etc.) or the dispatch half emits constructor bodies missing
__vtable__ctor(self.__vtable)for symbols the table half generated.The two hooks are now wired through an
Rc<RefCell<PolymorphismLowerer>>shared between the
post_indexadapter (aUnitLowerer) and thepost_annotateadapter (aPipelineParticipantMut). The pattern isdocumented as the recommended shape for future two-hook participants.
13. Writing a lowering pass — book chapter
A new
book/src/development/writing_a_lowerer.mdwalks through fivepatterns for new lowerers: pure-AST, signature-changing,
two-pass-with-
prepare, full-PipelineParticipantMut(when thetrait isn't enough), and shared-state via
Rc<RefCell<...>>. Workedexamples for each.
14. AggregateTypeLowerer cleanups
Two related cleanups:
AnnotationMap::into_any_boxtrait method existed only toround-trip a
Box<dyn AnnotationMap>back to a concreteAstAnnotations. The field is nowOption<AstAnnotations>directly; the trait method and its two impls are gone.
mutations: usizecounter is replaced by aMutationTrackerthat distinguishes body mutations (statementinserts) from signature rewrites. The visitor's mutation
primitives (
push_pre_statement,push_post_statement,signature-rewrite in
visit_pou) calltouch()/signature_changed(pou_name)themselves. The call site no longerhas to remember to bump anything, and the per-POU recording means
the partial-re-annotate closure only invalidates callers of POUs
whose interface actually changed — not every POU in the touched
unit.
Verification
cargo test --workspace --lib— all lib tests pass.cargo xtask lit— full lit suite passes. Cross-unit regressiontests in
tests/lit/multi/cover the paths this PR depends on:cross_unit_aggregate_signature— STRING-returning functioncalled across a unit boundary; verifies the partial-re-annotate
closure picks up direct cross-unit calls.
cross_unit_polymorphism_dispatch— base→derived vtabledispatch across units; verifies the polymorphism table pass
leaves both vtables wired up.
cross_unit_interface_dispatch_signature— interface methodwith aggregate return implemented and dispatched across three
units; verifies the reverse-dep closure covers the
interface-dispatch path.
cross_unit_vtable_ctor_in_member—WrapperholdingDerivedas a member, dispatched viaPOINTER TO Baseaimedat the embedded member; guards against a missing
__vtable__ctorchain when the member's vtable lives in adifferent compilation unit than its consumer.
cargo fmt --allclean.cargo clippy --workspace -- -Dwarningsclean on the CI toolchain(Rust 1.93.x). Newer local toolchains surface a couple of
pre-existing
needless_lifetimeslints insrc/resolver/tests/resolve_expressions_tests.rsthat are unchangedfrom master.
Open for review
Specific things I'd appreciate input on before this lands:
unit_id. TheUnitSymbolIndextrades indirection for migration cost. If reviewers prefer the
per-entry field shape, the migration is mechanical but ~344 call
sites.
UnitLowerervsLoweringBookkeeperas the recommendedauthor surface. They coexist on purpose —
LoweringBookkeeperis the substrate for unusual participants,
UnitLowereris theshorthand for the common case. Discoverability could be better.
MutationTrackerbody-vs-signature split. Signature changesare recorded per-POU; body mutations only mark the unit dirty.
If reviewers prefer AST-level dirty flags or snapshot-and-diff,
easy to revisit.
→ infrastructure → per-step perf wins → API ergonomics → cleanup).
If reviewers would still prefer it split, the natural fault lines
are:
(steps 1–4) — fast, bit-identical, no API surface changes.
biggest behavioural surface, needs the most review.
incremental driver (the LSP-ready piece), on-disk cache,
applies_toprecheck onUnitLowerer,evaluate_constantsscoped to a delta, partial re-annotate inside
PolymorphismDispatchAdapter::post_annotate(currently stilldoes a full re-index + full re-annotate when polymorphism is in
use; the clearest next target).
Test plan
cargo test --workspacecargo xtask litcargo fmt --allcleancargo clippy --workspace -- -Dwarningsclean on Rust 1.93.xPLC_TIMING=1 plc --checkoutput on a localproject; confirm the trace lines indent sensibly and no
participant logs a nested whole-project
index/annotateit shouldn't.🤖 Generated with Claude Code