Skip to content

test(archtest): record a lowering handed on as a value - #225

Merged
OmarAlJarrah merged 1 commit into
mainfrom
fix/recursion-pin-value-edges
Aug 2, 2026
Merged

test(archtest): record a lowering handed on as a value#225
OmarAlJarrah merged 1 commit into
mainfrom
fix/recursion-pin-value-edges

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Closes #210. Stacked on #223#222#221#220#219#218.

Summary

The lowering call graph was built from call expressions alone, so a lowering named at a site without being called there was described as no dependency at all. Three live handoffs were invisible — classifyUnionSiblings, unionBranchesDeclareShape and oneOfAnyOfHasNull each decide which lowering runs by passing one to slices.ContainsFunc:

if branches, _, _ := unionBranches(s); slices.ContainsFunc(branches, isInlineBranch) {

(#210 was filed against two l.method values that #217 has since deleted. The mechanism outlived its examples, which is the reason to fix the mechanism.)

An identifier now counts in value position too. Position is judged; scope is not — and getting that wrong is the interesting part of this change. A selector's field, a composite literal's key and a parameter's own name are identifiers as well, and every one of them collides with some lowering here. My first attempt recorded them all, and .Ref on a parsed schema — spelled exactly like the schema walk's entry point — pulled nine unrelated functions into the walk's cycle. The skip set is what makes the pass usable.

Scope is deliberately not judged: a local shadowing a lowering's name would read as an edge. Nothing here does that, and the error runs in the safe direction for a pin whose job is to notice dependencies — a spurious member shows up in a pinned set and gets read, where a missing one is the silence this issue was about.

The pinned sets are unchanged, which is what makes the fix legible: the edges were missing, not the answers.

The receiver branch goes with it. It matched methods on the lowerer type that #217 deleted, so it had been dead since. What that leaves unseen — anchorWalk.walk and walkMapping are genuinely mutually recursive and this graph cannot see them — is filed as #224 rather than folded in, because adding methods makes anchorWalk a fourth set and re-baselining the pin in the same change would hide whether the value edges moved anything.

Test plan

  • gofmt, go vet, golangci-lint run (0 issues), go build ./..., ./scripts/check-coverage.sh — all pass, 4188/4188.
  • Proven to reach, not just to run. TestLoweringCallGraph_RecordsAHandoffAsAnEdge asserts the three edges directly — it asserts edges rather than cycles because none of them closes one today, and a dependency the graph cannot see is unheld whether or not it currently changes an answer.
  • Proven to redden. Planting a cycle closed only by a value handoff (isNullSchema referencing oneOfAnyOfHasNull, whose only reference back is the ContainsFunc argument) makes the pin fail with a fourth set. Before this change that cycle was invisible.
  • The negative half is pinned on planted sources, not on the tree: TestCalleesOf_SeparatesAValueFromWhatIsNotOne walks eight shapes — a call, a value argument, a value assignment, a selector's field, a literal key, a parameter list, self-reference, and one name appearing both ways.

@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from e93db1b to ae988fb Compare August 1, 2026 22:41
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 0ac57f8 to 1eead21 Compare August 1, 2026 22:41
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from ae988fb to 79fc0ce Compare August 1, 2026 23:04
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 1eead21 to 53cd85d Compare August 1, 2026 23:04
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from 79fc0ce to fd79b40 Compare August 1, 2026 23:06
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 53cd85d to 627f6dc Compare August 1, 2026 23:06
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from fd79b40 to 9507c5d Compare August 1, 2026 23:11
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 627f6dc to 233cc84 Compare August 1, 2026 23:11
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from 9507c5d to b7b3daf Compare August 1, 2026 23:40
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 233cc84 to 279cb9a Compare August 1, 2026 23:40
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from b7b3daf to 575683c Compare August 1, 2026 23:45
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 279cb9a to e5a931d Compare August 1, 2026 23:45
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from 575683c to a616e0e Compare August 1, 2026 23:56
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from e5a931d to 0670aa0 Compare August 1, 2026 23:56
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from a616e0e to f15c353 Compare August 2, 2026 00:08
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 0670aa0 to 3d80f64 Compare August 2, 2026 00:08
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from f15c353 to d6b1be1 Compare August 2, 2026 00:20
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 3d80f64 to aebda3e Compare August 2, 2026 00:20
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/component-schema-at-guard branch from d6b1be1 to ea384fd Compare August 2, 2026 00:30
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from aebda3e to a072c1f Compare August 2, 2026 00:30
Base automatically changed from fix/component-schema-at-guard to main August 2, 2026 00:35
Closes #210.

The lowering call graph was built from call expressions alone, so a
lowering named at a site without being called there was described as no
dependency at all. Three live handoffs were invisible: classifyUnionSiblings,
unionBranchesDeclareShape and oneOfAnyOfHasNull each decide which lowering
runs by passing one to slices.ContainsFunc.

An identifier now counts in value position too. Position is judged, not
scope: a selector's field, a composite literal's key and a parameter's own
name are identifiers as well, and every one of them collides with some
lowering here. Reading them all first tied nine unrelated functions into
the schema walk's cycle, because a parsed schema's `.Ref` field is spelled
the same as the walk's entry point.

The pinned sets are unchanged, which is what makes the fix legible: the
edges were missing, not the answers.

The receiver branch is gone with it. It matched methods on the lowerer
type, which #217 deleted, so it had been dead since. What it leaves
unseen is filed as #224 rather than folded in here — adding methods makes
anchorWalk a fourth set, and re-baselining the pin in the same change
would have hidden whether the value edges moved anything.

The list of packages the graph reads is derived rather than trusted. A
lowering is a function over the immutable context, so a package
declaring one holds lowerings by definition; a package added without
being listed is a directory the pin silently stops reading, with its
recursions absent and the pinned sets still matching. That is the same
unheld-invariant shape as the value edges, one level up — in the input
rather than in the graph. The list now fails both ways: a package that
lowers and is not read, and one that is read and no longer lowers.

The planted-source cases gain the shape #210 was filed about — a
lowering named as a composite literal's field value — and one where the
same name appears as both key and value, which is what makes the skip
positional rather than by name.
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from a072c1f to f71cddb Compare August 2, 2026 00:40
@OmarAlJarrah

Copy link
Copy Markdown
Member Author

Rebased onto the new main (the #223 squash), which clears the conflict. Then a deeper pass, which found two gaps in what this PR itself asserts.

1. The shape #210 was filed about was not among the planted cases. The issue's own example is a lowering named as a struct literal's field value — merge.Merger{Resolve: l.types.Node, Report: l.diag}. The table covered a composite literal's key as a negative and had no positive for its value, so a skip that dropped the whole key-value pair rather than just the key would take the issue's canonical example back out of the graph with the test still green. Verified: that mutation now fails.

2. Nothing held the skip as positional. It is keyed on the identifier node, not its text, which matters when one name appears in both positions — T{lower: lower}. A by-name skip set passes every case the table had. There is a case for it now, and the by-name implementation fails it.

The two catch different regressions, which is why both are there rather than one.

3. The list of packages the graph reads was trusted rather than checked. The doc comment named the risk itself — "a new one that is not is a directory this pin stops seeing" — and then left it as prose. That is the same unheld-invariant shape as the value edges, one level up: in the graph's input rather than in the graph.

It is derived now. A lowering is a function over the immutable context (micro-compiler-design §4), so a package declaring one holds lowerings by definition; walking compilers/ for such declarations produces exactly the four listed packages today. The check fails in both directions — a package that lowers and is not read, and a package that is read and no longer lowers — and both were confirmed by planting.

The value-edge fix itself was re-verified after the restructure: a cycle closed only by a handoff still reddens the pin, and the three earlier probes (selector fields, parameter names, the name-collision refusal) still hold.

Unrelated finding, filed rather than fixed: running the coverage gate repeatedly across this stack produced two different statement counts for the same commit — 4186 and 5278 — because cover.out is a fixed path and concurrent runs can duplicate blocks in it. It cannot cause a false pass (verified by duplicating a profile and zeroing a block: still fails), so it is a reporting defect. Recorded on #59, which already covers this script's robustness.

@OmarAlJarrah
OmarAlJarrah merged commit c15ad0d into main Aug 2, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the fix/recursion-pin-value-edges branch August 2, 2026 00:44
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.

archtest: the recursion pin cannot see method-value edges

1 participant