fix(runtime): restore -D warnings cleanliness - #8923
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change restores warning-clean ChangesGC layout records and warning cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change restores regression coverage and warning-clean builds without altering production interfaces or deployment behavior. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description includes the required summary, changes, related issue, test plan, and checklist. Optional screenshots are not needed, and the omitted contribution and code-of-conduct checklist items are non-critical. Full details: Docstring CoverageExplanation Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
|
Merged. This fixes a red
How it got in: I ran every gate check last night with Validation — runtime 2764/0 ( |
… restored This branch deleted `test_per_object_layout_present` and `test_young_layout_records` to satisfy `-D warnings`. PerryTS#8923 landed the other resolution — restoring their regression-test callers — so removing them here merges cleanly but does not compile.
… push arm ahead of the tracked resolver (wolf-ecs −11.2% / −11.9%) (#8921) * perf(codegen): field-value arguments to sibling methods keep the proven-this clone The this-flow walker rejected a method as a proven-`this` clone candidate whenever an internal `this.m(...)` / `super.m(...)` call's ARGUMENTS mentioned `this` at all, even for a declared-field read such as `this._archChange(this._ent[id], i)`. That argument hands the callee a field's value, never the receiver; `expr_this_safe` already rejects a bare `this` in value position, a `this`-capturing closure and a non-field `this.x` read on its own. wolf-ecs `addComponent`, `removeComponent` and `createEntity` each make such a call and therefore ran their public bodies, re-proving `this` at every property, element and method site (≈14k instructions for ~20 source lines; the flat 54% inline self time of the add/remove profile). Vet the arguments with `expr_this_safe` alone. A bare `this` argument still rejects (pinned by the new test). Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ * perf(runtime): object-backed subclass push arm ahead of the tracked resolver #8897's `field_push_local_bind` turns `this.packed.push(x)` into a local `ArrayPush`, whose complete fallback is `js_array_push_f64_spec`. For an object-backed Array subclass (wolf-ecs `Archetype`) that entry paid the tracked resolver — a guaranteed miss on a `GC_TYPE_OBJECT` header — and then delegated to `js_array_push_f64`, which paid it again before reaching the dense subclass arm. Both entries now ask the dense arm first, off the header tag the guarded element tiers already read; every rejected case keeps the complete route. Test: `spec_and_generic_push_entries_append_to_an_object_backed_subclass_densely` pins, via a test-only probe counter on `try_read_tracked_gc_header`, that the spec and generic entries reach the dense arm with exactly the fused u31 entry's probes (none). Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ * changelog: fragment for #8921 Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ * runtime: clear main's -D warnings errors (dangling doc, test-only meta-edge probe, unused layout test helper) main f989075 fails the workspace -D warnings check on its own: a doc comment left without an item in object/shapes.rs, cell_has_meta_edge whose only caller is a test, and an unused #[cfg(test)] layout helper. Gate/remove them so this PR's warnings job can pass. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ * fix(gc): keep the layout-table test helpers, whose callers #8923 restored This branch deleted `test_per_object_layout_present` and `test_young_layout_records` to satisfy `-D warnings`. #8923 landed the other resolution — restoring their regression-test callers — so removing them here merges cleanly but does not compile. --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 participant
Summary
Restores perry-runtime warning cleanliness without suppressing dead-code lints. Test-only helpers are now configured explicitly, and the GC layout helpers regain their accidentally displaced regression-test callers.
Changes
Related issue
Fixes #8910
Test plan
Checklist
Summary by CodeRabbit
Bug Fixes
Tests
Documentation