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
object_tombstone_deletes_enabled: doc comment says default OFF, code defaults ON (#6987 shape, second instance) #9115
object_tombstone_deletes_enabled in crates/perry-runtime/src/object/delete_rest.rs carries two contradictory statements about its own default, eleven lines apart:
/// Gate for O(1) tombstone deletes (`PERRY_OBJECT_TOMBSTONES=1`). Default OFF/// while the walker audit and differentials bake; the sibling Map tombstones/// (#9020) shipped default-on after the same sequence.fnobject_tombstone_deletes_enabled() -> bool{
...staticON: std::sync::OnceLock<bool> = std::sync::OnceLock::new();*ON.get_or_init(|| {// Default ON (#9029 shipped the mechanism flag-gated; the walker// audit and churn-bound tests are the default-on prerequisites).// `PERRY_OBJECT_TOMBSTONES=0` is the kill switch, ...
!matches!(
std::env::var("PERRY_OBJECT_TOMBSTONES").as_deref(),Ok("0") | Ok("off") | Ok("false"))})}
The code defaults ON — the body comment is the accurate one, and the doc comment is stale.
This is the exact shape CLAUDE.md records as precedent:
gc_incremental_enabled's doc said "EXPERIMENTAL — default OFF" eight lines above a body comment saying "DEFAULT ON" (#6987). A merge decision was made on the wrong one.
It already cost something
Auditing #9110 (the fix for #9108's suite-wide SIGABRT) I read the doc comment, concluded the mechanism was still gated off, and initially scoped the abort as latent-behind-a-flag rather than shipping-by-default. That is the wrong blast-radius read for a bug that was masking 767 runtime tests on main.
Suggested fix
Delete the doc comment's default claim rather than correcting it — one statement of the default, at the site that implements it. If the "walker audit and differentials bake" caveat is still true, it belongs next to the code that would change if the audit fails, not as a contradictory default.
Worth a lint: the repo already has gates that re-derive prose claims from the thing they quote (check_node_version_consistency.py does this for Node versions). A cheap variant that flags "default ON/OFF" in a doc comment disagreeing with a matches!-style env default in the same function would catch the next one — this is now the second recorded instance.
Fixed by #9747, which landed on main via merge train #9798. The train was merged as its own branch, so the Fixes #9115 keyword in #9747 never fired — closing manually. Verified the fix is on main by patch-id (git cherry), not just by the PR being closed.
Activity
proggeramlug commentedon Sep 5, 2026
Fixed by #9747, which landed on
mainvia merge train #9798. The train was merged as its own branch, so theFixes #9115keyword in #9747 never fired — closing manually. Verified the fix is onmainby patch-id (git cherry), not just by the PR being closed.