codegen: thread the function name explicitly instead of ambient current_fn_name - #279
Merged
Merged
Conversation
The WASM code generator's function-body passes each recursed into Block/If/Loop independently to discover what they needed: pre_scan_locals numbered WASM locals, collect_compound_slots allocated frame slots, and body_has_dynamic_array_index decided the bounds-check scratch. They were kept in sync only by convention, so a future block-bearing statement kind (or a changed traversal order) could be handled by one pass and silently missed by another, corrupting the frame layout. Introduce `nested_blocks`, the single classifier that names which statement kinds carry sub-blocks and how their allocations combine (Sequential vs the mutually-exclusive Alternatives of an `if`). A thin pure-enumeration walker `walk_statements` is built on it and drives the two monotonic passes; collect_compound_slots consults the classifier directly because its if-arm frame-slot overlay is a genuine per-branch decision that a flat enumeration cannot express. All three passes now descend through one source of truth, so a new block kind is taught once and cannot desynchronize them. Purely internal: emitted WASM is byte-identical. The full codegen golden suite passes unmodified, and before/after binary diffs on nesting-heavy fixtures (arrays in a bare block, both if-arms of differing sizes, and a loop body) are identical. Adds unit tripwires pinning the walker's visitation order and the classifier's per-kind result. Fixes #167
…_name The WASM code generator held the name of the function being compiled as a mutable `Compiler::current_fn_name` field, set at the top of `visit_function_definition` and read implicitly far away. Its only reader is the sret-return invariant panic in `lower_sret_return`. Remove the field and thread the value explicitly as a `fn_name: &str` parameter from `visit_function_definition` through the statement-lowering walker (`lower_statement`, `lower_block`, `lower_if_statement`, `lower_loop_statement`) down to `lower_sret_return`. This forecloses a class of stale-read hazards that would surface once method, incremental, or parallel function compilation is added. Pure refactor: emitted WASM is byte-identical. The full codegen golden suite passes with zero expectation changes. Fixes #172
Signed-off-by: Georgii Plotnikov <accembler@gmail.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Fixes #172. Stacked on #278 (
167-refactoring-shared-statement-walker) — merge that first (both live incompiler.rs).Removes the mutable ambient
Compiler::current_fn_name: Stringand threads the function name explicitly asfn_name: &strfromvisit_function_definition_bodythrough the statement-lowering walker (lower_statement→lower_block/lower_if_statement/lower_loop_statement) to its sole runtime reader, the sret-return invariant panic inlower_sret_return.&strthreading, no context struct: there is exactly one reader, and this matches howarena/ctx/module_pathalready flow. The siblingcurrent_fn_keyis also ambient but out ofcurrent_fn_nameMutable State on Compiler #172's scope — left for a separate cleanup (noted intentionally).cargo test4359/0; default clippy clean, zero new pedantic findings vs baseline (990 = 990).Confidence Score: 5/5
Safe to merge — purely mechanical refactoring with no behavioral change and byte-identical WASM output confirmed by the golden test suite.
The change removes a write-once-per-function ambient string field and replaces it with an explicit
&strparameter. Every call site has been updated, the borrow lifetime is sound (the owningStringlives invisit_function_definitionfor the duration of all nested calls), and the sole reader (the sret panic message) is unchanged in meaning. No logic paths were added or altered.No files require special attention.
Important Files Changed
current_fn_name: StringfromCompiler, addsfn_name: &strparameter to five lowering functions; threading is mechanically correct and all call sites are updated.Reviews (2): Last reviewed commit: "Merge branch 'main' into 172-refactoring..." | Re-trigger Greptile