Skip to content

cl: preserve zero values for blank struct fields - #2231

Open
cpunion wants to merge 1 commit into
xgo-dev:mainfrom
cpunion:codex/xfail-blank-static-20260730
Open

cl: preserve zero values for blank struct fields#2231
cpunion wants to merge 1 commit into
xgo-dev:mainfrom
cpunion:codex/xfail-blank-static-20260730

Conversation

@cpunion

@cpunion cpunion commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • emit zero constants for blank fields in static struct initializers
  • discard dynamic stores into descendants of blank aggregate fields while preserving function-local right-hand-side side effects
  • remove the Darwin/arm64 fixedbugs/issue31546.go run xfail

Review counterexamples

The tests directly cover blank fields, nested blank structs, array descendants reached through IndexAddr, adjacent non-blank fields, and side-effecting function-local initializers. IndexAddr traversal is limited to pointer-to-array storage and has a defensive slice/backing-array boundary counterexample that failed before the review fix. The runtime fixture uses unsafe layout checks instead of reflect so it also builds for embedded targets.

Validation

  • go test ./ssa -count=1
  • full go test ./cl -count=1 -timeout=35m on the initial implementation (2036.211s)
  • final focused cl tests, blankfield IR/runtime fixture, and TestXFailMatch
  • focused isBlankFieldStore coverage: 94.1%
  • blankfield fixture with host Go 1.24.11, 1.25.7, and 1.26.5
  • esp32c3-basic compile/link of the blankfield fixture
  • fixedbugs/issue31546.go on Go 1.24.11, 1.25.0, 1.26.0, and 1.26.5
  • git diff --check

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: preserve zero values for blank struct fields

The change is well-constructed. The isBlankFieldStore walk-up correctly handles the intended cases: a store into a descendant of an aggregate blank field is recognized (the inner non-blank FieldAddr continues up to the blank ancestor and returns true), while a store to a non-blank sibling terminates at default and returns false. Side effects are preserved via _ = p.compileValue(b, v.Val) at the call site, and the static-init path forces blank struct fields to zero. Coverage (littest + internal tests) exercises scalar, nested struct, and array blank fields, plus side-effect evaluation. Removing the fixedbugs/issue31546.go darwin/arm64 xfail is consistent with the fix.

Two non-blocking findings are left inline. No blocking issues.

Comment thread cl/compile.go
Comment thread cl/rewrite_internal_test.go
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cpunion
cpunion force-pushed the codex/xfail-blank-static-20260730 branch from 9a17f36 to 263a408 Compare July 30, 2026 09:12
@cpunion
cpunion force-pushed the codex/xfail-blank-static-20260730 branch from 263a408 to 2c4cc7b Compare July 30, 2026 13:08
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.

1 participant