Skip to content

PERRY_L14_NBC_ORDER's ON path is exercised by no test — including the intersection rule #10954

Description

@proggeramlug

PERRY_L14_NBC_ORDER (landed in merge train 254, 2dda7c1139 / #10929) is default-OFF, and shape_numeric_inputs() early-returns empty unless it is set:

// crates/perry-codegen/src/collectors/number_by_construction.rs:796
if !nbc_order_enabled() || shape_proven.is_empty() { return ...; }

grep -rn PERRY_L14_NBC_ORDER crates/ finds exactly four sites — the gate function, its doc comment, and the two cache registries (build_cache.rs, object_cache.rs). No test sets the variable.

So the reorder the PR exists to perform is executed by no suite, and neither is the rule most likely to be wrong — that the shape inputs are the intersection of the proven receivers' numeric field sets, because the arm consumes one set and does not re-check which receiver a property belongs to. The PR's own commit message states a union would be "a wrong answer, not a weaker one", and nothing checks it.

This is harmless as shipped: with the knob off the fixpoint computes exactly what it computed before, so it cannot regress anything, and the gap suite confirms the off-path is a no-op. It is filed because the repo's own binding policy is explicit that an unexercised mode is a configuration nobody has verified — PERRY_GC_FORCE_EVACUATE was inert for every gc()-driven test (#6942/#6946), and the matrix's --pressure knob disabled the very path it was measuring (#7024).

What a test needs

Two proven receivers where a field is numeric on one only (must not be admitted) and a field numeric on both (must be admitted), with the knob on.

The obstacle is that nbc_order_enabled() reads std::env at the leaf, and perry-codegen's tests run in parallel, so an env-setting test races every sibling. Either isolate it (per_test_global!-style, as #10944 is doing for the asserted-counter class) or plumb the flag in as a parameter so the fixpoint can be exercised directly without touching the process environment — the latter is probably cleaner and removes the env read from the hot path as a side effect.

Refs #10777.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions