Skip to content

object_tombstone_deletes_enabled: doc comment says default OFF, code defaults ON (#6987 shape, second instance) #9115

Description

@proggeramlug

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.
fn object_tombstone_deletes_enabled() -> bool {
    ...
    static ON: 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugConfirmed defect or regression

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions