You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
global_sink_isolation.py matches identifiers in comments and test-local consts, reddening lint for uninvolved files #10953
scripts/global_sink_isolation.py resolves an asserted identifier by name across the whole crate, with no scope awareness and without stripping comments. Both halves reddened lint during merge train 255, on two unrelated PRs, for files that touch no process-global at all.
Instance 1 — a word in a comment
#10941's new comment in crates/perry-runtime/src/gc/tests/support.rs ended:
// slots 0 and 1 of an object that has none — those two words are THE NEXT// CELL. It presents as a wrong read now and a SIGSEGV somewhere unrelated
CELL matched the identifier extractor at line 319 —
— and declaration_kind resolved it to the real static CELL: OnceLock<InternedPointerTypes> in crates/perry-runtime/src/pointer_event.rs. The audit then reported:
GLOBAL SINK: CELL is a BARE process-global `static` (crates/perry-runtime/src/pointer_event.rs),
written by gc/tests/support.rs (the guards themselves).
support.rs never mentions pointer_event. A 2×2 pinned it — main's script on main's tree 0 hazards, main's script on the train tree 1, the train's script on the train tree 1 — so the tree caused it, not that PR's own changes to this script. Changing two words of prose moved it from 1 hazard / 121 statics to 0 / 120.
Instance 2 — a test-local const shadowing a static
#10938 added, inside a #[test] body in object/dictionary_tests.rs:
constDETERMINISTIC:u64 = 1 << 63;
which resolved to crates/perry-runtime/src/stub_diag.rs's static DETERMINISTIC: OnceLock<bool>, and the second rule reported NEW ASSERTED GLOBAL … baseline 62 -> 63. The test asserts on its own local constant.
Why this matters
Both were worked around in the train by editing prose and renaming a local constant, which is the wrong direction: it makes source text answer to a scanner's parsing limits. Any future comment or local binding whose name happens to collide with a static somewhere in perry-runtime can redden a required gate, and the failure text confidently names a file that is not involved — which costs far more to diagnose than to fix.
What a fix needs
Blank string literals and comments before extracting identifiers.
Ignore identifiers bound in the same scope (const/let inside the body being scanned) before resolving them crate-wide.
One caution, learned the hard way
A first attempt at the comment-stripping half was backed out. It moved the classified population from 120 to 115 — i.e. five identifiers the audit had always counted came from comments on main too — and that invalidated the live ALLOWLIST entry for API (#9613), which the audit then correctly reported as an entry matching nothing. Whoever fixes this should expect the allowlist and the #10944 asserted baseline to move with it, and should land a sabotage test proving the fixed scanner still detects a genuinely bare process-global before trusting a green run.
Activity