fix(codegen): point the module-global Ptr<Shape> denial at #10803, not closed #7109 - #10804
proggeramlug wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe module-global denial pointer changes from issue ChangesIssue Pointer Correction
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix · Severity of issue fixed: Low 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Every other suite in |
|
Landed via merge train 243 (#10838) as v0.5.1622 — This should make The pre-warm found something my design missed: there are two cold builds, not one. The Both liveness assertions are in and both are load-bearing: the presence check (a A gate caught the new step, and that is the gate working. Worth stating why it has to be listed rather than exempted: the pre-warm carries its own assertions and can fail, so it is a subject. If it were ever silently removed, every gate below would quietly inherit the timeout it exists to prevent — and the failure would look exactly like the red it just fixed. On provenance: this work came from an agent that died on an authentication error after committing but before pushing. I recovered the commit, rebased it twice as #10804 rode along: six areas, 197 fixtures weighted to its |
The optimiser report's denial message for "a module-level binding can never take a canonical slot" cited #7109. That is a different mechanism, and it is closed.
MODULE_GLOBAL_ISSUEincrates/perry-codegen/src/expr/slot_rep.rsis the issue number surfaced to users when a module-level binding is denied a canonical slot. #7109 is the module-init / program-entry context gate — a distinct rule, whichMODULE_INIT_CONTEXTa few lines below still cites correctly. #7109 is closed, and so is #10774, which lifted that gate.So a reader who followed the denial's own pointer landed on a closed issue about something else, and could reasonably conclude the module-global class had already been handled.
That is not hypothetical. One optimisation pass recorded module-global storage as "less important" on exactly that reading. A separate campaign spent a day repeating "#7109 is the blocker" on inherited belief before checking the issue state. The pointer now goes to #10803 —
Ptr<Shape>is denied to three storage classes (module globals, function parameters, and locals escaping into a module global) — with a comment recording why the old one was wrong, so the correction isn't silently reverted later.Scope, deliberately narrow
Only the live user-facing pointer for the module-global storage class moved. Left unchanged:
MODULE_INIT_CONTEXT's#7109(slot_rep.rs) — accurate to that rule's subject, which genuinely is entry-body exclusion. Whether it should also be re-pointed now that perf(codegen): stop disabling Ptr<Shape> in entry bodies for a bug fixed in the runtime — 110 to 89 instructions (#10769) #10774 lifted the gate is a separate question I haven't resolved and didn't want to guess at.opt_report/render.rs's"#7109"— fixture data inside a test helper, not a user-facing pointer.#7109mentions in prose comments (slot_rep.rs:127,:267,opt_report/mod.rs:773), which correctly describe past work.Why the two were separable, demonstrated rather than asserted
the_context_gate_is_reported_when_every_value_rule_passedassertsissue == Some("#7109")forMODULE_INIT_CONTEXT. It passes unchanged after this edit — which is the evidence that the module-global pointer and the context-gate pointer were genuinely independent, rather than my having moved one and broken the other.cargo test -p perry-codegen --lib slot_rep: 9 passed, 0 failed.cargo check -p perry-codegenclean.Found during cross-session review while handing the module-global lane to another campaign; the three denial classes are that campaign's #10803.
Summary by CodeRabbit