Skip to content

fix(runtime): gate the seven regex-engine/test-only items so the product-scope -D warnings check compiles again - #9970

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/product-warnings-dead-code
Closed

fix(runtime): gate the seven regex-engine/test-only items so the product-scope -D warnings check compiles again#9970
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/product-warnings-dead-code

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Small runtime fix on origin/main (10 lines: #[cfg] attributes + a changelog fragment). Found while replaying CI's warnings job locally because GitHub's runners were unavailable.

Why

CI's warnings job runs RUSTFLAGS="-D warnings" cargo check -p perry --bins, which compiles perry-runtime with the feature set the perry binary requests — without regex-engine. Seven items are used only by the regex-engine path and by tests, so that product-scope check fails on main with "could not compile perry-runtime (lib) due to 7 previous errors" (dead code): newborn_parent_needs_barrier (gc/barrier_store.rs) and the six REGEXP_PROTOTYPE_* statics in object/regex_proto_thunks.rs. cargo check --workspace --all-targets is clean because the tests keep them alive, which is why the local lint driver never saw it (see #9969). Every PR's warnings check has been red on main for this reason.

What changes

The seven items are gated #[cfg(any(test, feature = "regex-engine"))], matching their only users (the feature-gated regex.rs fast path / installer / root scanner and the GC tests). Reference evidence per item is in the commit message; git log -S traces newborn_parent_needs_barrier to 9a2f802. No inventory changes: the root-holder statics remain live and registered in regex-engine builds (gc_runtime_root_holders.py self-test OK — 90 planted / 350 entries; scan OK — 1,363 declarations, 593 scanner-reached; check_thread_locals.py OK; rustfmt and diff-check clean).

Verification

Local: the script gates above. The -D warnings product check itself runs on perrymaster's local CI ladder (the Mac is out of disk for a check build); the expected result is the warnings job green on this head while it stays red on main.

Verified locally (macOS arm64, 2026-09-08)

RUSTFLAGS="-D warnings" cargo check -p perry --bins: on main 8b7dc33 it fails with exactly the seven errors above ("could not compile perry-runtime (lib) due to 7 previous errors", exit 101); on this head it finishes clean (exit 0). The runtime lib suite on the box (Linux ladder) is the remaining check that the cfg gates removed nothing the tests need.

Local CI ladder on perrymaster (Linux, 2026-09-08, read against main's own baseline run): warnings PASS — zero "never used" errors under RUSTFLAGS="-D warnings" cargo check -p perry --bins and the workspace --all-targets check — where main fails with the seven; perry-runtime lib suite 3,247 passed / 0 failed / 4 ignored (the gates removed nothing the tests need); every other job identical to main's (gap suite per-test sets equal on all six shards). Nothing PR-owned is red.

https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo

Summary by CodeRabbit

  • Bug Fixes
    • Fixed warnings encountered when building the product without optional features.
    • Limited regular-expression runtime checks and related diagnostics to test builds or builds with the regular-expression engine enabled.
  • Build Improvements
    • No-default-feature builds now compile cleanly with warnings treated as errors.

Keep the no-default-features product build free of dead-code warnings while
retaining every helper where its regex feature or unit tests need it.

Dispositions:
- newborn_parent_needs_barrier: cfg(test or regex-engine); feature + tests use it.
- REGEXP_PROTOTYPE_PTR_SLOT: cfg(test or regex-engine); feature GC root backing.
- REGEXP_PROTOTYPE_TEST_CLOSURE_SLOT: cfg(test or regex-engine); feature root backing.
- REGEXP_PROTOTYPE_TEST_INDEX_SLOT: cfg(test or regex-engine); feature scalar backing.
- REGEXP_PROTOTYPE_PTR: cfg(test or regex-engine); feature fast path/root scanner.
- REGEXP_PROTOTYPE_TEST_CLOSURE: cfg(test or regex-engine); feature fast path/scanner.
- REGEXP_PROTOTYPE_TEST_WALKS: cfg(test or regex-engine); feature/test diagnostic.

Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 41d05de5-f14e-4e6b-bda8-d39d175f9d3b

📥 Commits

Reviewing files that changed from the base of the PR and between 8b7dc33 and d5115df.

📒 Files selected for processing (3)
  • changelog.d/9970-product-warnings-dead-code.md
  • crates/perry-runtime/src/gc/barrier_store.rs
  • crates/perry-runtime/src/object/regex_proto_thunks.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The runtime now compiles RegExp-only garbage-collection helpers, prototype roots, test slots, and walk counters only for tests or when regex-engine is enabled. A changelog entry documents the change.

Changes

RegExp conditional compilation

Layer / File(s) Summary
Gate RegExp-only runtime state
crates/perry-runtime/src/gc/barrier_store.rs, crates/perry-runtime/src/object/regex_proto_thunks.rs, changelog.d/9970-product-warnings-dead-code.md
The write-barrier helper and RegExp prototype state compile only under test or the regex-engine feature. The changelog records the change.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to d5115

RegExp-only runtime state is excluded from non-RegExp builds, fixing warnings-as-errors product checks while retaining test and regex-engine builds. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the seven regex-engine/test-only items being gated and the resulting -D warnings build fix.
Description check ✅ Passed The description explains the problem, lists the affected items, describes the cfg changes, references the related issue, and provides detailed verification results. It does not reproduce the template …
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via merge train #9978. Validated as a tree: 77 of 80 lint gates pass, and perry-runtime/codegen/hir/stdlib all green (5,980 tests, 0 failures).

The three non-passing gates are accounted for: public-baseline is pre-existing on main (verified on a pristine worktree; red since 2026-07-29), and the two API docs gates are an artifact of this session's CARGO_TARGET_DIR override — regen_api_docs.sh hardcodes $ROOT/target/release/perry. With the binary placed where the script expects, regeneration succeeds and the drift check is clean. Thanks!

proggeramlug pushed a commit that referenced this pull request Sep 8, 2026
…ating

#9917 collapses the three RegExp.prototype.test proof slots into one
CanonicalTestSite behind a single HotKey, but was written before #9970
gated that surface for the no-default-features product build. Merged so
both hold: the unified record, carrying #9970's cfg on the struct, its
impl, the thread_local and TEST_ACCESSOR_KEY_BIT.

Consequences reconciled:

- object_static_prototype_known_non_meta is #9917's, and its only caller
  is regex-engine gated, so it is dead in a product build. Same gate.
- The three _SLOT holder entries added for #9893 are stale: the statics
  no longer exist. Deleted, and a duplicate TEST_WALKS entry removed. The
  new REGEXP_PROTOTYPE_TEST_SITE needs no entry — the walk reaches it
  through scan_canonical_test_site_roots_mut, which
  scan_object_cache_roots_mut calls and reg_scanner! registers.
- segments_view's cursor-number store carried GC_STORE_AUDIT(NUMBER),
  which is not an accepted class, so the marker never satisfied the gate.
  POINTER_FREE is the class that states what the comment argues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant