Skip to content

ir+pass+irverify: give the shared document walk one home #247

Description

@OmarAlJarrah

Summary

Three pieces of document-inspection mechanism are written more than once across pass,
ir/irverify and compilers/compile, because there is no package below all three to hold them.
There is one: ir is Layer 0, imports only the stdlib ("ir": {} in internal/archtest), and
CLAUDE.md already lists traversal among what it owns, beside nodes, IDs and JSON round-trip.

Each copy is individually well-argued — the comments in both walkers cross-reference each other and
explain why the two must agree. That agreement is currently maintained by hand.

1. The bounded reflection walk exists twice

walk machinery file total
pass/refs.go 141 333
ir/irverify/refs.go 128 265

Two of the helpers are byte-for-byte identical, doc comments included — diff over the extracted
ranges is empty:

  • mapEntry (7 lines)
  • orderedEntries (13 lines)

The rest is the same logic written twice: the pointer/struct/sequence/map arms, the embedded-field
path rule, the 4096-entry depth cap and its rationale, and the byte-slice skip. The comments already
name the coupling — "matching pass.refWalk.walkSequence" in one, "as ir/irverify's walk spells
them"
in the other — so the duplication is known and deliberate; what it lacked was somewhere to go.

Each copy also carries its own guard tests, since a test can only observe the walk it imports:
pass/refs_internal_test.go, ir/irverify/refs_test.go, ir/irverify/walkdepth_test.go, and the
byte-skip descent counters in both.

2. isNilTypeDef exists three times

pass/validate.go:319, ir/irverify/irverify.go:110, compilers/compile/types.go:34 — identical
eight-line bodies, differing only in doc comment. It asks a question about ir.TypeDef, a sealed
ir interface, and all three packages already import ir.

3. irverify hand-maintains a list pass already derives

irverify.refKindByType enumerates four ID classes by hand and needs
TestStringTypes_AreAllClassified (ir/irverify/refkinds_test.go:76) to stop it drifting.
pass.documentRegistries derives the same set from Document's own shape — a map keyed by a named
string type is an ID-keyed registry — and so covers a registry added to Document the moment it
exists, with no guard test needed.

The integer-index checks split the same way: irverify.checkIndices and
pass.checkServerIndices/checkResponseIndices each hand-name the same three carriers, and each
needs its own drift guard (indexCarrierFields in ir/irverify/indices_test.go:113, and
pass/validate_indices_test.go).

Expected

One bounded, cycle-guarded, deterministically-ordered walk in ir, used by both checkers; one
isNilTypeDef; one derivation of the registry set, with the hand-written list and its guard test
deleted.

Notes

  • No internal/archtest rules change is needed. "pass" and "ir/irverify" are both
    {module + "/ir"}; compilers/compile is audited under "compilers", also {module + "/ir"}.
    reflect is stdlib, so "ir": {} still holds.
  • This shares mechanism only. pass keeps returning ir.Diagnostic (problems in the source
    spec) and irverify keeps returning Violation (our bugs). The separation of those two channels
    is not affected and must not be.
  • The visitor signature is already settled by the more general copy. irverify's
    visit(v, path) bool prunes on false, which subsumes pass's isRef predicate and onStruct
    hook — both become closures over one visitor.
  • Overlaps pass+irverify: silent truncation and drift hazards in the verification harness #55. Item 2 there (checkNaming discards the walk-truncation flag) sits in exactly
    the code this touches, and item 3 asks for the same derive-don't-enumerate change in a
    neighbouring oracle. Worth sequencing the two together rather than landing them blind to each
    other.

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