Skip to content

Preserve resolved bindings during constant evaluation - #156

Merged
itsfuad merged 1 commit into
mainfrom
fix/constant-binding-identity
Oct 6, 2026
Merged

itsfuad merged 1 commit into
mainfrom
fix/constant-binding-identity

Conversation

@itsfuad

@itsfuad itsfuad commented Oct 5, 2026

Copy link
Copy Markdown
Member

Summary

  • What changed: Constant evaluation consumes resolver-bound symbols. Removed obsolete scope plumbing, reused one symbol snapshot during finalization, and added binding-sensitive regressions.
  • Why this change is needed: Later shadowing could reinterpret an earlier constant initializer, producing wrong array-element selection and misleading condition warnings.
  • Owning phase/package/component: Semantic typechecker's constant evaluator and AST-to-THIR publication.
  • Scope boundaries / non-goals: Binding identity and associated cleanup. Numeric conversion/publication and wide-index evaluation remain follow-up work.

Production ownership

  • Current non-test production consumer(s) of each new production symbol: No new production symbols.
  • Execution path showing where the new behavior is reached in production: Pipeline → typechecker.Check → constant queries/finalization → THIR → CFG diagnostics and MIR lowering.
  • Existing implementation reused, extended, replaced, or intentionally left unchanged: Reused SymbolIndex.Symbol, existing evaluator caches, and authoritative constant publication.
  • Wrappers, aliases, duplicate paths, compatibility shims, or experimental scaffolding removed: Removed repeated identifier name lookup, evaluator scope arguments, THIR builder's temporary scope tracking, single-use evalModuleConstants, and duplicate warning-test setup.
  • Any new production symbol without a current production consumer: None.

Design and behavior

  • Previous behavior: An initializer resolved to outer index = 0 could later fold against shadowing index = 2. values[saved] printed 30 while saved printed 0. Shadowing could also suppress a known bounds error or reverse a constant-condition warning.
  • New behavior: Folding follows the original resolved declaration. The repro prints 10, then 0; known out-of-bounds indexes are rejected; condition warnings agree with resolved values.
  • Key invariants preserved: SymbolID-keyed cache/cycle detection, foreign constant ownership, authoritative module publication, numeric normalization, deferred diagnostics, and durable THIR block scopes. All cached module constants are invalidated before evaluating any dependency.
  • Intentional behavioral changes: Later declarations cannot redirect earlier constant references. Unbound identifiers no longer fold through a lexical lookup fallback.
  • Error / edge-case handling: Constant-kind checks and cycle diagnostics retained. Tests cover both constant and variable shadows.
  • Why this implementation belongs in this layer rather than another phase/package: Resolver already owns declaration identity; evaluator should consume that decision rather than resolve names again.

Validation

Commands ran with CCACHE_DISABLE=1 and GOCACHE=/tmp/omnirush/peeper-go-cache.

go test -count=1 ./internal/semantics/typechecker
PASS

go test -count=1 -v -run '^TestPipeline(ReportsConstantConditionsInCFGPhase|DefersConstantEvaluationCycleToCFG|LowersSequenceIndexesAsUsizeAcrossTargets)$' ./internal/pipeline
PASS — seven diagnostic cases, cycle deferral, and linux/386 + linux/amd64 LLVM/Clang validation

go test -count=1 ./internal/pipeline ./internal/semantics/analysis ./internal/semantics/resolver ./internal/project ./internal/ir/... ./internal/backend/llvm
PASS

go run ./scripts/bundle.go
PASS

PEEPER_BIN=/home/itsfuad/Dev/Peeper/compiler/build/bin/peeper go test -count=1 ./...
PASS — includes executable x_test fixtures

go vet ./...
PASS

go test -race -count=1 ./internal/semantics/typechecker ./internal/pipeline
PASS

git diff --check
PASS
  • Targeted tests added or updated: THIR binding/index/condition regressions; unbound-reference test; CFG warning table; 32-/64-bit pipeline coverage; positive runtime and negative bounds fixtures.
  • Existing regression coverage exercised: Numeric folding, constant cycles, local/foreign caches, module publication, and full compiler/runtime fixture suite.
  • Manual verification, if applicable: Compared pre-fix and rebuilt compiler outputs and diagnostics. External allocation probe measured finalization of a 64-function module dropping from two allocations/1,024 bytes to one allocation/512 bytes.
  • Checks not run: Hosted PR CI pending. Native Windows/macOS execution not performed locally.

Diff sanity

  • Unrelated production changes: None.
  • Duplicated logic introduced: None. Separate invalidation/evaluation passes retained for correctness; warning-test setup consolidated.
  • Dead or unreachable code introduced: None.
  • Temporary/debug code remaining: None in committed files. Measurement probes stayed external.
  • Comments/docs that became stale because of this change: Updated semantic architecture note; documented finalization ordering invariant.

Risks and follow-up

  • Known risks or assumptions: Generated/cloned expressions must retain canonical symbol bindings. Finalization must complete cache invalidation before dependency evaluation.
  • Behavior intentionally changed: Constant folding respects resolver-bound declaration identity under shadowing.
  • Behavior intentionally not changed: Numeric operation/conversion rules, ownership analysis, and constant publication lifetimes.
  • Remaining gaps: Module constant conversion/publication and lossless wide-index evaluation are separate confirmed audit findings.
  • Linked issues / follow-up work: #155 — Preserve checked numeric conversions during constant publication; #154 — Evaluate wide constant indexes without truncation.
  • Anything that should block merge: Required CI failures or outstanding review findings; approving review from a non-author human collaborator required.

Use resolver-bound symbols so later shadowing cannot change earlier constant initializers. Remove obsolete scope plumbing and reuse one symbol snapshot across both finalization passes, keeping cache invalidation before evaluation.

Cover runtime indexing, rejected constant bounds, CFG warning phases, and 32-/64-bit LLVM lowering with focused regressions and source fixtures.
@itsfuad itsfuad added this to the 0.2 Language Foundations milestone Oct 5, 2026
@itsfuad
itsfuad merged commit 05092be into main Oct 6, 2026
17 checks passed
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