Skip to content

fix(internal/harness): exclude permutations that change meaning - #243

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/order-oracle-permutation-faithfulness
Open

fix(internal/harness): exclude permutations that change meaning#243
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/order-oracle-permutation-faithfulness

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

The two-order oracle compiles a source as written and again with every mapping
reversed, and reports what the permutation changed. Its correctness rests on the
rewrite being meaning-preserving. reverseMappings documented one case where it
is not — a duplicate mapping key, whose meaning depends on the order being
changed — and excluded it. Three more went unexcluded, and in each the two
compiles differ because of the rewrite rather than because of a lowering, so the
oracle reported a defect in the compiler that was not there.

  • A flow-style implicit null is not preserved. yaml.Marshal re-emits {A}
    with an empty string for its value, so the second compile read a string where
    the first read null. Both arms are now compiled from the encoder's output,
    which leaves declaration order as the only difference between them.
  • Reversing a mapping can carry an alias above its anchor. YAML requires an
    anchor to be defined before it is referenced, so the permuted source did not
    parse and the parse failure was reported as order dependence. The rewrite now
    re-parses what it produced and declines a source whose permutation does not
    survive it, which covers any later ordering rule of the same kind without
    enumerating it.
  • diagnosticSet identified a finding partly by Provenance.Pointer, which
    ir/provenance.go documents as holding either a structural pointer or
    line:col. A permutation moves a construct to a different line by design. A
    positional pointer is now replaced rather than dropped, so a permutation that
    changes how many findings were reported still shows.

All three were latent. No committed spec reaches any of them and the corpus
sweep was green; they surface once a spec using one of these spellings is
committed, or the oracle is driven by generated input.

TestReverseMappings_AliasIsNotFollowed asserted the anchor-above-alias output
as correct — invalid YAML that the oracle was handing to the compiler. Its
fixture moves into a sequence, whose order the rewrite leaves alone, so it still
pins that an anchored mapping reverses once rather than once per alias; the shape
it used to assert is now covered by a test that requires it to be refused.

Closes #241

Test plan

  • Each of the three cases is pinned by a test that fails without its own fix and
    passes with it. Reverting the fixes one at a time reddens exactly one case
    each, so no test is standing in for another:
    • baseline normalization → flow-style implicit null
    • re-parsing the permutation → an alias reordered above its anchor and
      TestReverseMappings_AliasAboveItsAnchorIsRefused
    • positional pointers → a diagnostic located by line and column
  • The change adds skip paths, and a skip is how this oracle goes quiet without
    the sweep noticing, so reach was measured rather than assumed: 82 corpus specs
    still reach the comparison, and the new baseline skip fires on none of them.
    The seven that are newly declined are the deliberately-invalid anchor and
    duplicate-key fixtures, which stop at an earlier oracle and never reached this
    one.
  • TestOrderInvariant_ReachesTheCorpus now guards both arms rather than the
    rewrite alone. Forcing either arm to decline reddens the suite.
  • Full gate on the rebased branch: gofmt, go vet, golangci-lint run
    (0 issues), go build ./..., and ./scripts/check-coverage.sh at 100% of
    4373 statements.

The two-order oracle compiles a source as written and again with every
mapping reversed, and reports what the permutation changed. That rests on
the rewrite being meaning-preserving. reverseMappings documented one case
where it is not — a duplicate mapping key, whose meaning depends on the
order being changed — and excluded it. Three more went unexcluded, and in
each the two compiles differ because of the rewrite rather than because of
a lowering, so the oracle blamed the compiler.

yaml.Marshal does not preserve a flow-style implicit null: "{A}" comes
back with an empty string for its value, so the second compile read a
string where the first read null. Both arms are now compiled from the
encoder's output, leaving declaration order as the only difference.

Reversing a mapping can carry an alias above the anchor it names, which
YAML forbids, so the permuted source no longer parsed at all and the parse
failure was reported as order dependence. The rewrite now re-parses what
it produced and declines a source whose permutation does not survive it,
which covers any later ordering rule of the same kind without enumerating
it.

diagnosticSet identified a finding partly by Provenance.Pointer, which is
documented to hold either a structural pointer or line:col, and a
permutation moves a construct to a different line by design. A positional
pointer is now replaced rather than dropped, so a permutation that changes
how many findings were reported still shows.

All three were latent: the committed corpus reaches none of them and the
sweep was green. Each is pinned by a case that fails without its own fix
and by nothing else. The reachability guard now covers both arms, since
the baseline can go quiet on its own.

TestReverseMappings_AliasIsNotFollowed asserted the anchor-above-alias
output as correct; its fixture moves to a sequence, whose order the rewrite
leaves alone, so it still pins that an anchored mapping reverses once.

Closes #241
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.

internal/harness: the two-order oracle mistakes permutation artifacts for order dependence

1 participant