diff --git a/architecture-audit-masterprompt.md b/architecture-audit-masterprompt.md deleted file mode 100644 index 739913fb..00000000 --- a/architecture-audit-masterprompt.md +++ /dev/null @@ -1,415 +0,0 @@ -# Master prompt: Peeper compiler architecture audit - -Use this prompt to resume architecture audit in another agent session. Repository source proves current implementation, not desired architecture. Tests prove current contracts, not that those contracts are well designed. Every markdown document—including `RULES.md`, `RULEBOOK.md`, specifications, architecture guides, and generated maps—is evidence to evaluate, not unquestioned design authority. - -## Start state - -Run from compiler repository supplied by user. Never assume saved path, branch, commit, or worktree snapshot remains current. - -Before work: - -1. report repository root, branch, HEAD, and `git status --short`; -2. read `architecture-audit.localplan.md` when present; -3. identify existing user changes and unrelated files; -4. do not reset, discard, stage, commit, or overwrite existing work. - -## Task - -Perform end-to-end architectural review of Peeper compiler. This is not ordinary bug review and not request for immediate refactoring. - -Evaluation goals: - -- high cohesion and low coupling; -- one canonical owner where one owner reduces inconsistency; -- simple, linear, self-explanatory control flow; -- fewer repeated walks, switches, conversions, validations, and test setups where repetition has no distinct purpose; -- explicit contracts where they clarify real boundaries; -- minimal rediscovery of decisions already proven elsewhere; -- adding features changes genuinely responsible code, not unrelated packages; -- abstractions remove work and concepts rather than moving code behind interfaces; -- tests prove module contracts and observable behavior. - -These are evaluation principles, not a prescribed component graph. Dependency injection, query contracts, immutable results, visitors, direct concrete calls, or another pattern may be recommended only after comparing alternatives against actual code. Do not assume current phase boundaries, phase order, result models, package splits, identity schemes, or compiler terminology are correct because documentation names them. - -Goal is **minimum coupling with maximum clarity**. Maximum modularity, maximum reuse, and any specific architecture pattern are not goals by themselves. - -## Hard semantic constraints - -Do not propose or make semantic changes during audit. - -In particular: - -- `anyfn() -> ?T` is required language behavior. -- Do not restore or design around obsolete `Next()` iterator protocol. -- Current iteration design uses explicit call iteration and canonical optionals. -- Older docs/issues that still describe `Next()` are outdated and should be identified separately from architecture defects. -- Definite initialization follows the approved rule in `docs/language-spec.md`: declarations without initializers create uninitialized storage, never implicit zero values; only whole-value assignment initializes that storage; projected writes into an uninitialized root are rejected; uninitialized storage owns nothing and receives no cleanup. -- Necessary coordinated edits across parser, semantics, lowering, and tests are valid. Do not label every repeated syntax switch as duplication. - -## Audit inputs and evidence rules - -Read before analysis: - -1. `AGENTS.md` -2. `RULES.md` -3. `RULEBOOK.md` -4. `go-style.md` -5. `docs/language-spec.md` -6. `docs/compiler-architecture.md` -7. `architecture-audit.localplan.md` -8. relevant files under `docs/architecture/` - -Do not use precedence declared inside these documents to settle architecture questions. Evaluate conflicts instead. - -Evidence roles: - -1. Explicit user requirements define audit goals and confirmed language requirements. -2. Source shows what implementation currently does. -3. Tests and fixtures show behavior currently protected; they may preserve a mistake and must also be evaluated. -4. Specifications, rules, architecture documents, comments, issues, and names show intended design claims; verify those claims against behavior and engineering purpose. -5. Established engineering guidance supplies reasoning tools, not automatic answers. - -Separate three questions for every important claim: - -- What exists now? -- What behavior is intentionally required? -- Is current design best way to implement that behavior? - -`RULES.md` should contain durable guidance about how work is judged and performed. Flag rules that freeze one current pipeline, representation, package boundary, or design choice without enduring reason. Factual architecture belongs in architecture maps; proposed architecture belongs in design documents; neither becomes correct merely by being documented. - -Reconstruct actual pipeline and dependencies from source. Then evaluate whether pipeline order, boundaries, models, and handoffs are justified. Do not hardcode documented phase order into findings or target design. - -Audit is read-only unless user explicitly authorizes writes. Do not edit source, tests, generated files, docs, local plans, GitHub issues, or roadmap items during read-only work. Put handoff state in response when writes are not authorized. - -Do not fabricate findings. Every reported finding needs source verification and design reasoning. - -## What user values - -Favor: - -- less code and fewer concepts; -- ordinary `if`, `switch`, and `for range` when flow becomes clearer; -- visible tree/graph/collection walks; -- descriptive domain names; -- one phase-owned decision reused downstream; -- stable identity and explicit evidence; -- direct call sites over pass-through wrappers; -- deletion/reuse before new abstraction; -- boundaries justified by ownership, lifetime, phase, or independent reuse. - -Reject: - -- clever pointer-identity or traversal-order tricks; -- one-field wrappers without a present domain distinction, invariant, ownership boundary, or public API contract; -- old-name wrappers after renames; -- ignored parameters retained to avoid call-site updates; -- speculative interfaces and registries; -- generic visitor/dataflow frameworks that save lines but hide semantics; -- comments that restate code; -- test-only abstractions that obscure behavior; -- recommendations based only on aesthetics. - -## Review scopes - -Review all scopes. Parallelize independent scopes when subagents are available. - -### Scope A: frontend and binding/type semantics - -Inspect: - -- `internal/frontend/lexer` -- `internal/frontend/token` -- `internal/frontend/parser` -- `internal/frontend/ast` -- collector, binder, resolver -- symbols, scopes, type information, places, constants -- base typechecker and call/iteration evidence - -Questions: - -- Are syntax walks duplicated without semantic need? -- Is node identity stable and intentionally owned? -- Are symbol/type/place lookups canonical? -- Do generated nodes have explicit identity and scope rules? -- Does typechecker publish evidence rather than force later rediscovery? -- How many true extension points does new syntax/type require? - -### Scope B: CFG and semantic analyses - -Inspect: - -- `internal/ir/cfg` -- flow typing -- effects -- definite initialization -- ownership and cleanup -- usage -- their result models and validators - -Questions: - -- Do analyses consume canonical CFG and semantic evidence? -- Are repeated worklists/walks shared mechanics or distinct lattices? -- Are reads, writes, moves, borrows, definitions, calls, iteration, discard, and cleanup represented generically enough downstream? -- Does any analysis rediscover AST meaning already known earlier? -- Are `ast.NodeID`, `ir.NodeID`, `symbols.SymbolID`, and `cfg.SiteID` used for correct roles? -- Are normal, `break`, `continue`, `return`, and failure cleanup paths explicit? - -Prior conversation says one subagent completed this scope, but its report may not survive. Treat `docs/architecture/semantics-analyses.md` as lead only. Re-verify important claims in source. - -### Scope C: HIR, MIR, backend, and target - -Inspect: - -- shared `internal/ir` model -- HIR model, validation, lowering, folding -- MIR model, validation, lowering -- backend target/layout/ABI ownership -- LLVM emission - -Questions: - -- Does HIR consume semantic evidence rather than repeat typechecking? -- Does MIR consume HIR/CFG/ownership instead of rediscovering AST semantics? -- Are type lowering, ABI, mangling, physical layout, and runtime-carrier fields canonical? -- Are backend values and addresses carrying sufficient physical type evidence? -- Are validators checking each phase's own invariant? -- Which repeated type switches are required closed-world completeness checks? - -### Scope D: infrastructure, LSP, CLI, packages, and tests - -Inspect: - -- `internal/project` -- `internal/pipeline` -- `internal/driver` -- diagnostics, graph, phase, module identity, source, toolchain -- `internal/lsp` -- `cmd` and `cmd/cli` -- manifests, registry, remotes, distribution -- scripts, test helpers, and `x_test/` - -Questions: - -- Is module lifecycle and phase reset ownership clear? -- Are snapshots taken before concurrent work and conditionally published by generation/version? -- Are pipeline setup and source discovery duplicated across CLI/LSP/tests? -- Do command aliases share handlers instead of wrappers? -- Are dependency graph/cache operations canonical? -- Is repeated test setup hiding missing production APIs or only fixture noise? - -Prior conversation says one subagent completed infrastructure/LSP, but report may not survive. Treat architecture maps as leads only and verify source. - -### Scope E: cross-cutting repetition and extension cost - -Search across repository for: - -- repeated AST walks; -- repeated exhaustive node/type/effect switches; -- duplicate scope or symbol lookup; -- duplicate type text/relations; -- duplicate call argument shaping and default expansion; -- duplicate receiver adaptation; -- duplicate place/projection decomposition; -- duplicate name mangling and ABI rules; -- repeated semantic effect discovery; -- repeated cleanup logic; -- repeated target/layout decisions; -- repeated diagnostics or near-identical messages; -- repeated compiler pipeline setup in tests and entry points; -- broad result structs whose consumers reach through unrelated fields; -- interfaces with one implementation and no real boundary; -- functions that only forward, rename, or retain ignored parameters. - -Measure extension cost using concrete recent features where useful: optional call iteration, structural iteration, match payloads, arrays/slices, interfaces, ownership, or cleanup. Do not infer cost from file count alone. - -## Delegation protocol - -When user requests a model class or exact model and discovery tooling exists, inspect available models before delegation. Do not invent model IDs or silently substitute an unavailable explicit model unless user allowed fallback. Without user preference, preserve environment's default routing. - -Suggested independent delegations: - -1. frontend + binding/type semantics; -2. CFG + analyses; -3. HIR/MIR + backend; -4. infrastructure + LSP; -5. CLI/packages/tests; -6. cross-cutting repetition. - -Each subagent receives: - -- repository path; -- exact scope and exclusions; -- read-only requirement; -- semantic constraints above; -- output format below; -- instruction to report only verified findings; -- instruction to include useful code that should **not** be refactored. - -If usage limit stops agent: - -1. record label, scope, model, and session ID if available; -2. mark scope incomplete in `architecture-audit.localplan.md` when writes are authorized; -3. otherwise include incomplete scope and resume context in response; -4. preserve any completed report verbatim in handoff notes or dedicated temporary report when writes are authorized; -5. resume same session when possible; -6. otherwise start replacement agent with this master prompt and incomplete scope; -7. never present partial coverage as complete audit. - -Main agent must verify subagent findings. Subagent output is evidence lead, not final authority. - -## Required finding format - -Each finding must contain: - -```text -ID: stable short ID -Severity: hard violation | high-value debt | medium debt | low-value cleanup -Confidence: high | medium -Classification: accidental duplication | leaky boundary | shotgun surgery | misplaced responsibility | unhelpful abstraction | missing abstraction | hidden state | test duplication | necessary extension point -Files/symbols: exact paths and names -Current behavior: factual description -Why it matters: maintenance/extension/correctness cost -Evidence: callers, consumers, repeated logic, or data-flow trace -Option A: smallest viable correction -Option B: alternative design -Recommendation: chosen option and reason -Semantic impact: must be none unless separately approved -Validation needed: exact tests/fixtures/contracts -Do not refactor: nearby code that should remain explicit -``` - -Omit finding when evidence cannot distinguish defect from intentional design. Put uncertainty in investigation notes, not final claims. - -## Classification rules - -### Necessary extension point - -Keep explicit when phase has genuine distinct job, such as: - -- parser recognizing syntax; -- typechecker defining semantics; -- CFG representing control topology; -- HIR/MIR lowering representation; -- backend emitting physical operations; -- exhaustive closed-world switches enforcing completeness. - -### Accidental duplication - -Report when same semantic decision is independently recomputed and can diverge, especially when earlier phase can publish evidence once. - -### Leaky boundary - -Report when consumer imports or traverses producer internals unrelated to its job rather than using narrow evidence/query contract. - -### Unhelpful abstraction - -Report wrappers, one-field structs, speculative interfaces, tiny files/modules always edited together, or helpers that hide simple control flow without reducing concepts. - -### Missing abstraction - -Recommend only when repeated domain logic has at least two real consumers or protects non-obvious invariant. Prefer function/query/result-field over interface unless multiple implementations or ownership boundary exists. - -## Final synthesis - -Final report must include: - -1. **Executive assessment** - - current architecture health; - - strongest existing boundaries; - - top risks; - - no unsupported score or claim. - -2. **Documentation and rulebook assessment** - - claims confirmed by source and required behavior; - - stale or incorrect claims; - - rules that encode a current design rather than durable engineering guidance; - - recommended moves between rules, specifications, architecture maps, and design proposals. - -3. **Current implementation map** - - source-to-output data flow reconstructed from code; - - current owner, input, output, identity, and consumers without assuming they should remain; - - infrastructure and LSP lifecycle. - -4. **Verified findings** - - ordered by severity and leverage; - - exact file/symbol references; - - multiple options for major findings; - - recommended choice. - -5. **Repetition inventory** - - repeated mechanics; - - repeated semantic decisions; - - intentional exhaustive handling; - - test setup repetition. - -6. **Coupling and extension-cost map** - - packages changed for representative features; - - which edits are legitimate; - - which indicate shotgun surgery. - -7. **Target architecture alternatives** - - compare multiple viable shapes before choosing; - - retain only boundaries that earn keep; - - avoid enterprise DI and speculative plugin systems unless evidence establishes real need. - -8. **Ranked remediation plan** - - Stage 0: correctness or hard-rule violations; - - Stage 1: safe deletions/direct reuse/local cleanup; - - Stage 2: evidence and query-boundary improvements; - - Stage 3: larger representation or phase changes; - - dependency order, risk, expected payoff, validation, stop condition for each step. - -9. **Do not refactor list** - - explicit switches/walks/boundaries already serving clarity or completeness; - - reasons to keep them. - -10. **Outdated documentation/issues list** - - remaining `Next()` assumptions; - - distinguish docs cleanup from compiler architecture work. - -11. **Implementation recommendation** - - first single step worth implementing; - - why it has best confidence/payoff ratio; - - exact behavior and invariants to preserve; - - ask user approval before source edits. - -## Search and verification commands - -Use repository-aware tools first. Useful terminal checks: - -```sh -git status --short --branch -git log -5 --oneline -rg -n 'Next\(|\.Next\b|iterator|iteration' . -rg -n 'Inspect|Walk|Visit|forEachChild|switch .*\.\(type\)' internal cmd pkg -rg -n 'TypeText|ABI|mangle|receiver|EffectiveCallArguments|ValueUse|ReferenceArgument' internal -rg -n 'BuildQueries|Result struct|type .*Result struct|Validate\(' internal -rg -n 'CompileFile|pipeline\.Run|DiscoverSourceFiles|NewWithConfig' cmd internal pkg -``` - -Scope searches to relevant directories after discovery. Read callers and consumers, not only declarations. - -Do not run expensive full test suites during read-only audit unless needed to verify disputed behavior. No validation result should be claimed unless command was run and passed. - -## Resume protocol - -At session start: - -1. read audit inputs without treating them as unquestioned design authority; -2. inspect Git state; -3. read `architecture-audit.localplan.md` when present; -4. list completed reports and missing scopes; -5. state which scope you will inspect; -6. remain read-only. - -Before stopping: - -1. when writes are authorized, update local plan with completed coverage, verified findings, unresolved questions, and exact next step; -2. when writes are forbidden, include same handoff state in response; -3. record subagent session IDs when available; -4. state files read and commands run; -5. mark partial work partial; -6. leave source tree unchanged. - -When all scopes finish, main agent performs independent final review and removes duplicates/false positives from report before presenting it. diff --git a/docs/architecture/backend.md b/docs/architecture/backend.md index e002b883..648c93ba 100644 --- a/docs/architecture/backend.md +++ b/docs/architecture/backend.md @@ -30,32 +30,34 @@ and an optional source location. Each block contains MIR instructions and one terminator. `Instr` and `Terminator` are sealed interfaces; the emitter uses exhaustive type switches and panics if a new MIR node is not classified. -The result is LLVM module text. Invalid target or invalid runtime-symbol use -returns an empty string after diagnostics. Unsupported layouts and lowering -errors set emitter invalid state; `finalLLVMText` then returns an empty string. +The result is LLVM module text. Invalid target, malformed MIR, or invalid +runtime-symbol use returns an empty string after diagnostics. Unsupported +layouts and lowering errors set emitter invalid state; `finalLLVMText` then +returns an empty string. ## Module emission job `GenerateLLVMIR` emits in this order: 1. Validate non-nil module and `target.Info.Valid()`. -2. Validate reserved runtime symbols with `ValidateRuntimeSymbols`. -3. Create one `llvmEmitter` for the module. -4. Emit `source_filename` and `target triple`. -5. Resolve and emit identified named type definitions. -6. Emit print format globals when print is used. -7. Emit MIR static data: raw byte arrays or typed constants. -8. Discover interface constructions and emit unique itab globals. -9. Emit declared MIR functions. -10. Emit print, allocator, free, and overflow-intrinsic declarations as needed. -11. Emit default allocator descriptor thunks when allocator runtime is needed. -12. Collect and emit direct-call declarations. -13. If no function has blocks, finalize. -14. Emit named drop helpers. -15. Emit MIR interface thunks. -16. Emit interface payload drop/release thunks. -17. Emit each defined function and its blocks. -18. Append late external globals and debug metadata. +2. Validate backend-independent MIR invariants with `mir.Module.Validate`. +3. Validate reserved runtime symbols with `ValidateRuntimeSymbols`. +4. Create one `llvmEmitter` for the module. +5. Emit `source_filename` and `target triple`. +6. Resolve and emit identified named type definitions. +7. Emit print format globals when print is used. +8. Emit MIR static data: raw byte arrays or typed constants. +9. Discover interface constructions and emit unique itab globals. +10. Emit declared MIR functions. +11. Emit print, allocator, free, and overflow-intrinsic declarations as needed. +12. Emit default allocator descriptor thunks when allocator runtime is needed. +13. Collect and emit direct-call declarations. +14. If no function has blocks, finalize. +15. Emit named drop helpers. +16. Emit MIR interface thunks. +17. Emit interface payload drop/release thunks. +18. Emit each defined function and its blocks. +19. Append late external globals and debug metadata. Static strings are already interned by `mir.Module.InternString`; raw entries are escaped by `llvmEscapeString`. Typed constants are lowered by `staticConstant`. diff --git a/docs/architecture/ir.md b/docs/architecture/ir.md index 4c5ddd90..035edcec 100644 --- a/docs/architecture/ir.md +++ b/docs/architecture/ir.md @@ -153,7 +153,11 @@ signed indexes also reject negative values independently of unsigned length. - effect and ownership result validators reject inconsistent analysis evidence. - `mir.Module.Validate` rejects invalid runtime types, places, instructions, terminators, calls, and return shapes before backend emission. -- LLVM emission treats validated MIR violations as internal invariants. +- The pipeline validates MIR before entering backend phase, and LLVM emission + revalidates at its direct entry boundary so callers that bypass the pipeline + receive diagnostics instead of sending malformed MIR into lowering. +- After these validation boundaries, LLVM emission treats unclassified backend + invariants as internal failures. ## Adding a language construct diff --git a/internal/backend/llvm/emitter.go b/internal/backend/llvm/emitter.go index a31a1fd5..0dd3fcdf 100644 --- a/internal/backend/llvm/emitter.go +++ b/internal/backend/llvm/emitter.go @@ -32,6 +32,12 @@ func GenerateLLVMIR(mod *mir.Module, diag *diagnostics.DiagnosticBag, targetInfo } return "" } + if err := mod.Validate(); err != nil { + if diag != nil { + diag.AddError(diagnostics.ErrInvalidEvidence, "MIR is malformed: "+err.Error(), nil, "") + } + return "" + } if !ValidateRuntimeSymbols([]*mir.Module{mod}, diag) { return "" } diff --git a/internal/backend/llvm/emitter_test.go b/internal/backend/llvm/emitter_test.go index d91aa43c..c9c3968b 100644 --- a/internal/backend/llvm/emitter_test.go +++ b/internal/backend/llvm/emitter_test.go @@ -351,11 +351,7 @@ func TestTypedLLVMBuilderRejectsOperandMismatches(t *testing.T) { } } -// mir.Instr and mir.Terminator are sealed, so a node this package could pass to -// emission no longer exists: the earlier fabricated one cannot implement either -// interface. What remains reachable is a block that carries no terminator at -// all, which would otherwise emit an unterminated basic block with no signal. -func TestGenerateLLVMIRPanicsForMalformedMIRBlocks(t *testing.T) { +func TestGenerateLLVMIRRejectsMalformedMIRBlocks(t *testing.T) { for _, tt := range []struct { name string block *mir.Block @@ -364,16 +360,12 @@ func TestGenerateLLVMIRPanicsForMalformedMIRBlocks(t *testing.T) { { name: "missing terminator", block: &mir.Block{ID: 7}, - want: "LLVM emission: block b7 has no terminator", + want: "MIR is malformed: function test block b7 has no terminator", }, } { t.Run(tt.name, func(t *testing.T) { - defer func() { - if recovered := recover(); recovered != tt.want { - t.Fatalf("GenerateLLVMIR panic = %#v; want %q", recovered, tt.want) - } - }() - GenerateLLVMIR(&mir.Module{ + diag := diagnostics.NewDiagnosticBag() + if out := GenerateLLVMIR(&mir.Module{ Name: "unknown-node", Types: llvmTypes.table, Funcs: []*mir.Function{{ @@ -381,7 +373,12 @@ func TestGenerateLLVMIRPanicsForMalformedMIRBlocks(t *testing.T) { ReturnType: llvmTypes.void, Blocks: []*mir.Block{tt.block}, }}, - }, diagnostics.NewDiagnosticBag(), testLinuxAMD64, false) + }, diag, testLinuxAMD64, false); out != "" { + t.Fatalf("GenerateLLVMIR() = %q, want empty output", out) + } + if !diag.HasErrors() || !strings.Contains(diag.EmitAllToString(), tt.want) { + t.Fatalf("diagnostics = %q, want %q", diag.EmitAllToString(), tt.want) + } }) } } @@ -1477,7 +1474,7 @@ func TestGenerateLLVMIRRejectsIncompleteNamedType(t *testing.T) { if out := GenerateLLVMIR(mod, diag, testLinuxAMD64, false); out != "" { t.Fatalf("incomplete named type emitted LLVM:\n%s", out) } - if !diag.HasErrors() || !strings.Contains(diag.EmitAllToString(), "unsupported llvm type: Pending") { + if !diag.HasErrors() || !strings.Contains(diag.EmitAllToString(), "MIR is malformed: function use_pending parameter 0 has invalid type#") { t.Fatalf("incomplete named type diagnostic missing:\n%s", diag.EmitAllToString()) } } @@ -3583,7 +3580,7 @@ func TestGenerateLLVMIRRejectsIncompleteVariantSwitch(t *testing.T) { if out := GenerateLLVMIR(mod, diag, testLinuxAMD64, false); out != "" { t.Fatalf("incomplete variant switch must suppress LLVM output, got:\n%s", out) } - if !diag.HasErrors() || !strings.Contains(diag.EmitAllToString(), "cover every case") { + if !diag.HasErrors() || !strings.Contains(diag.EmitAllToString(), "MIR is malformed: function select block b0 variant switch has 1 targets, want 2") { t.Fatalf("incomplete variant switch diagnostic missing:\n%s", diag.EmitAllToString()) } } diff --git a/internal/ir/mir/validate.go b/internal/ir/mir/validate.go index f57031a7..2641a446 100644 --- a/internal/ir/mir/validate.go +++ b/internal/ir/mir/validate.go @@ -134,7 +134,7 @@ func validateInstruction(types *ir.TypeTable, fn *Function, block *Block, index case *Store: problems := validatePlace(types, node.Place, where+" store") problems = append(problems, validateValueRef(types, node.Value, where+" store value")...) - if node.Place != nil && node.Value != nil && node.Place.Type != node.Value.TypeID() { + if node.Place != nil && !typednil.IsNil(node.Value) && node.Place.Type != node.Value.TypeID() { problems = append(problems, fmt.Sprintf("%s stores type#%d into place type#%d", where, node.Value.TypeID(), node.Place.Type)) } return problems @@ -150,7 +150,7 @@ func validateInstruction(types *ir.TypeTable, fn *Function, block *Block, index if !arrayOK || arrayType.Kind != ir.TypeArray || arrayType.Length != "" { problems = append(problems, fmt.Sprintf("%s operates on non-dynamic-array type#%d", where, node.ArrayType)) } - if node.Array != nil { + if !typednil.IsNil(node.Array) { carrier, carrierOK := types.Type(node.Array.TypeID()) if !carrierOK || carrier.Kind != ir.TypeReference || carrier.Elem != node.ArrayType { problems = append(problems, fmt.Sprintf("%s array value type#%d does not reference array type#%d", where, node.Array.TypeID(), node.ArrayType)) @@ -179,14 +179,14 @@ func validateTerminator(types *ir.TypeTable, fn *Function, block *Block, blocks switch term := block.Term.(type) { case *Branch: problems = append(problems, validateValueRef(types, term.Cond, where+" branch condition")...) - if term.Cond != nil && types != nil { + if !typednil.IsNil(term.Cond) && types != nil { if typ, ok := types.Type(term.Cond.TypeID()); !ok || typ.Kind != ir.TypeBool { problems = append(problems, fmt.Sprintf("%s branches on non-bool type#%d", where, term.Cond.TypeID())) } } case *SwitchVariant: problems = append(problems, validateValueRef(types, term.Value, where+" variant switch value")...) - if term.Value != nil && types != nil { + if !typednil.IsNil(term.Value) && types != nil { if variant, ok := types.Type(term.Value.TypeID()); !ok || variant.Kind != ir.TypeVariant { problems = append(problems, fmt.Sprintf("%s switches on non-variant type#%d", where, term.Value.TypeID())) } else { @@ -215,6 +215,9 @@ func validateTerminator(types *ir.TypeTable, fn *Function, block *Block, blocks break } problems = append(problems, validateValueRef(types, term.Value, where+" return value")...) + if typednil.IsNil(term.Value) { + break + } if returnType.Kind == ir.TypeVoid { problems = append(problems, fmt.Sprintf("%s returns a value from void function", where)) } else if term.Value.TypeID() != fn.ReturnType { @@ -326,7 +329,7 @@ func validateValueExpr(types *ir.TypeTable, expr ValueExpr, where string) []stri } case *Field: problems = append(problems, validateValueRef(types, node.Base, where+" field base")...) - if node.Base != nil && types != nil { + if !typednil.IsNil(node.Base) && types != nil { if base, ok := types.Type(node.Base.TypeID()); !ok || base.Kind != ir.TypeStruct || node.Index < 0 || node.Index >= len(base.Fields) { problems = append(problems, fmt.Sprintf("%s selects invalid field %d from type#%d", where, node.Index, node.Base.TypeID())) } else if base.Fields[node.Index].Type != node.Type { @@ -347,7 +350,7 @@ func validateValueExpr(types *ir.TypeTable, expr ValueExpr, where string) []stri problems = append(problems, validateValueRef(types, node.Allocator, where+" allocator")...) } if types != nil { - if allocated, ok := types.Type(node.Type); ok && allocated.Kind == ir.TypeOwnedPtr && node.Value != nil && allocated.Elem != node.Value.TypeID() { + if allocated, ok := types.Type(node.Type); ok && allocated.Kind == ir.TypeOwnedPtr && !typednil.IsNil(node.Value) && allocated.Elem != node.Value.TypeID() { problems = append(problems, fmt.Sprintf("%s owns type#%d but allocates type#%d", where, allocated.Elem, node.Value.TypeID())) } } @@ -368,7 +371,7 @@ func validateValueExpr(types *ir.TypeTable, expr ValueExpr, where string) []stri for index, slot := range node.Slots { problems = append(problems, validateValueRef(types, slot, fmt.Sprintf("%s interface slot %d", where, index))...) method, ok := types.InterfaceMethod(node.Type, index) - if ok && slot != nil && slot.TypeID() != method.SlotType { + if ok && !typednil.IsNil(slot) && slot.TypeID() != method.SlotType { problems = append(problems, fmt.Sprintf("%s interface slot %d has type#%d, want published type#%d", where, index, slot.TypeID(), method.SlotType)) } } @@ -391,7 +394,7 @@ func validateCall(types *ir.TypeTable, call *Call, where string) []string { for index, arg := range call.Args { problems = append(problems, validateValueRef(types, arg, fmt.Sprintf("%s argument %d", where, index))...) } - if call.Callee == nil || types == nil { + if typednil.IsNil(call.Callee) || types == nil { return problems } function, ok := types.Type(call.Callee.TypeID()) @@ -406,7 +409,7 @@ func validateCall(types *ir.TypeTable, call *Call, where string) []string { return problems } for index, arg := range call.Args { - if arg != nil && arg.TypeID() != function.Params[index] { + if !typednil.IsNil(arg) && arg.TypeID() != function.Params[index] { problems = append(problems, fmt.Sprintf("%s argument %d has type#%d, want type#%d", where, index, arg.TypeID(), function.Params[index])) } } @@ -422,7 +425,7 @@ func validateInterfaceCall(types *ir.TypeTable, call *InterfaceCall, where strin for index, arg := range call.Args { problems = append(problems, validateValueRef(types, arg, fmt.Sprintf("%s interface argument %d", where, index))...) } - if call.Base == nil || types == nil { + if typednil.IsNil(call.Base) || types == nil { return problems } method, ok := types.InterfaceMethod(call.Base.TypeID(), call.Slot) @@ -446,7 +449,7 @@ func validateInterfaceCall(types *ir.TypeTable, call *InterfaceCall, where strin } for index, arg := range call.Args { param := index + 1 - if param < len(slotType.Params) && arg != nil && arg.TypeID() != slotType.Params[param] { + if param < len(slotType.Params) && !typednil.IsNil(arg) && arg.TypeID() != slotType.Params[param] { problems = append(problems, fmt.Sprintf("%s interface argument %d has type#%d, want type#%d", where, index, arg.TypeID(), slotType.Params[param])) } } @@ -491,7 +494,7 @@ func validateStructLiteral(types *ir.TypeTable, value *StructLit, where string) } for index, field := range value.Fields { problems = append(problems, validateValueRef(types, field, fmt.Sprintf("%s field %d", where, index))...) - if index < len(structure.Fields) && field != nil && field.TypeID() != structure.Fields[index].Type { + if index < len(structure.Fields) && !typednil.IsNil(field) && field.TypeID() != structure.Fields[index].Type { problems = append(problems, fmt.Sprintf("%s field %d has type#%d, want type#%d", where, index, field.TypeID(), structure.Fields[index].Type)) } } @@ -509,7 +512,7 @@ func validateArrayLiteral(types *ir.TypeTable, value *ArrayLit, where string) [] } for index, element := range value.Values { problems = append(problems, validateValueRef(types, element, fmt.Sprintf("%s element %d", where, index))...) - if element != nil && element.TypeID() != array.Elem { + if !typednil.IsNil(element) && element.TypeID() != array.Elem { problems = append(problems, fmt.Sprintf("%s element %d has type#%d, want type#%d", where, index, element.TypeID(), array.Elem)) } } @@ -529,13 +532,16 @@ func validateVariantMake(types *ir.TypeTable, value *VariantMake, where string) return []string{fmt.Sprintf("%s constructs invalid case %d of type#%d", where, value.Case, value.Type)} } if variantCase.Payload == ir.InvalidType { - if value.Payload != nil { - return []string{fmt.Sprintf("%s gives payload to payloadless case %d", where, value.Case)} + if value.Payload == nil { + return nil } - return nil + if typednil.IsNil(value.Payload) { + return []string{where + " variant payload is nil"} + } + return []string{fmt.Sprintf("%s gives payload to payloadless case %d", where, value.Case)} } problems := validateValueRef(types, value.Payload, where+" variant payload") - if value.Payload != nil && value.Payload.TypeID() != variantCase.Payload { + if !typednil.IsNil(value.Payload) && value.Payload.TypeID() != variantCase.Payload { problems = append(problems, fmt.Sprintf("%s payload has type#%d, want type#%d", where, value.Payload.TypeID(), variantCase.Payload)) } return problems @@ -543,7 +549,7 @@ func validateVariantMake(types *ir.TypeTable, value *VariantMake, where string) func validateVariantIs(types *ir.TypeTable, value *VariantIs, where string) []string { problems := validateValueRef(types, value.Value, where+" variant value") - if types == nil || value.Value == nil { + if types == nil || typednil.IsNil(value.Value) { return problems } variant, ok := types.Type(value.Value.TypeID()) @@ -565,7 +571,7 @@ func validatePlace(types *ir.TypeTable, place *Place, where string) []string { } problems := validateValueRef(types, place.Root, where+" root") problems = append(problems, validateKnownType(types, place.Type, where+" result type")...) - if types == nil || place.Root == nil { + if types == nil || typednil.IsNil(place.Root) { return problems } current := place.Root.TypeID() diff --git a/internal/ir/mir/validate_test.go b/internal/ir/mir/validate_test.go index 70fbba32..972632c9 100644 --- a/internal/ir/mir/validate_test.go +++ b/internal/ir/mir/validate_test.go @@ -187,6 +187,297 @@ func TestValidateReportsDefects(t *testing.T) { } } +func typedNilValueRef() ValueRef { + var ref *RefName + return ref +} + +func typedNilValueExpr() ValueExpr { + var expr *Unary + return expr +} + +func assertValidationError(t *testing.T, module *Module, want string) { + t.Helper() + defer func() { + if recovered := recover(); recovered != nil { + t.Fatalf("Validate() panicked: %v", recovered) + } + }() + err := module.Validate() + if err == nil { + t.Fatalf("Validate() = nil, want a report containing %q", want) + } + if !strings.Contains(err.Error(), want) { + t.Fatalf("Validate() = %v, want a report containing %q", err, want) + } +} + +func TestValidateRejectsNestedTypedNilValues(t *testing.T) { + tests := []struct { + name string + module func() *Module + want string + }{ + { + name: "assignment expression", + module: func() *Module { + module := wellFormed() + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{ + Name: "broken", Value: typedNilValueExpr(), + }} + return module + }, + want: "assignment is nil", + }, + { + name: "unary operand", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{ + Name: "broken", Value: &Unary{Arg: typedNilValueRef(), Type: i32}, + }} + return module + }, + want: "assignment operand is nil", + }, + { + name: "store value", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Store{ + Place: &Place{Root: &RefName{Name: "slot", Type: i32}, Type: i32}, + Value: typedNilValueRef(), + }} + return module + }, + want: "store value is nil", + }, + { + name: "dynamic array value", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + array := module.Types.Intern(ir.Type{Kind: ir.TypeArray, Elem: i32}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&DynamicArrayOp{Array: typedNilValueRef(), ArrayType: array}} + return module + }, + want: "dynamic-array value is nil", + }, + { + name: "branch condition", + module: func() *Module { + module := wellFormed() + module.Funcs[0].Blocks[0].Term = &Branch{Cond: typedNilValueRef(), ThenID: 1, ElseID: 2} + return module + }, + want: "branch condition is nil", + }, + { + name: "variant switch value", + module: func() *Module { + module := wellFormed() + module.Funcs[0].Blocks[0].Term = &SwitchVariant{Value: typedNilValueRef()} + return module + }, + want: "variant switch value is nil", + }, + { + name: "return value", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + module.Funcs[0].ReturnType = i32 + module.Funcs[0].Blocks[3].Term = &Ret{Value: typedNilValueRef()} + return module + }, + want: "return value is nil", + }, + { + name: "field base", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &Field{Base: typedNilValueRef(), Index: 0, Type: i32}}} + return module + }, + want: "field base is nil", + }, + { + name: "allocation value", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + owned := module.Types.Intern(ir.Type{Kind: ir.TypeOwnedPtr, Elem: i32}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &Alloc{Value: typedNilValueRef(), Type: owned}}} + return module + }, + want: "allocated value is nil", + }, + { + name: "call callee", + module: func() *Module { + module := wellFormed() + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &Call{Callee: typedNilValueRef(), Type: module.Funcs[0].ReturnType}}} + return module + }, + want: "callee is nil", + }, + { + name: "call argument", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + fnType := module.Types.Intern(ir.Type{Kind: ir.TypeFunction, Params: []ir.TypeID{i32}, Return: module.Funcs[0].ReturnType}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &Call{ + Callee: &RefName{Name: "callee", Type: fnType}, Args: []ValueRef{typedNilValueRef()}, Type: module.Funcs[0].ReturnType, + }}} + return module + }, + want: "argument 0 is nil", + }, + { + name: "interface call base", + module: func() *Module { + module, _, _, _ := interfaceValidationFixture() + call := module.Funcs[0].Blocks[0].Instrs[0].(*InterfaceCall) + call.Base = typedNilValueRef() + return module + }, + want: "interface base is nil", + }, + { + name: "interface call argument", + module: func() *Module { + module, _, slotType, _ := interfaceValidationFixture() + call := module.Funcs[0].Blocks[0].Instrs[0].(*InterfaceCall) + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + slot, _ := module.Types.Type(slotType) + call.SlotType = module.Types.Intern(ir.Type{Kind: ir.TypeFunction, Params: []ir.TypeID{slot.Params[0], i32}, Return: slot.Return}) + call.Args = []ValueRef{typedNilValueRef()} + return module + }, + want: "interface argument 0 is nil", + }, + { + name: "interface construction slot", + module: func() *Module { + module, carrier, _, _ := interfaceValidationFixture() + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &InterfaceMake{ + Value: &RefName{Name: "value", Type: carrier}, DataType: carrier, + Slots: []ValueRef{typedNilValueRef()}, Type: carrier, + }}} + return module + }, + want: "interface slot 0 is nil", + }, + { + name: "struct literal field", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + structure := module.Types.Intern(ir.Type{Kind: ir.TypeStruct, Fields: []ir.TypeField{{Name: "value", Type: i32}}}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &StructLit{Fields: []ValueRef{typedNilValueRef()}, Type: structure}}} + return module + }, + want: "field 0 is nil", + }, + { + name: "array literal element", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + array := module.Types.Intern(ir.Type{Kind: ir.TypeArray, Elem: i32}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &ArrayLit{Values: []ValueRef{typedNilValueRef()}, Type: array}}} + return module + }, + want: "element 0 is nil", + }, + { + name: "variant payload", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + variant := module.Types.Intern(ir.Type{Kind: ir.TypeVariant, Cases: []ir.VariantCase{{Name: "Some", Payload: i32}}}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &VariantMake{Case: 0, Payload: typedNilValueRef(), Type: variant}}} + return module + }, + want: "variant payload is nil", + }, + { + name: "payloadless variant payload", + module: func() *Module { + module := wellFormed() + variant := module.Types.Intern(ir.Type{Kind: ir.TypeVariant, Cases: []ir.VariantCase{{Name: "None"}}}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &VariantMake{Case: 0, Payload: typedNilValueRef(), Type: variant}}} + return module + }, + want: "variant payload is nil", + }, + { + name: "variant test value", + module: func() *Module { + module := wellFormed() + boolean := module.Types.Intern(ir.Type{Kind: ir.TypeBool}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &VariantIs{Value: typedNilValueRef(), Case: 0, Type: boolean}}} + return module + }, + want: "variant value is nil", + }, + { + name: "place root", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &Load{Place: &Place{Root: typedNilValueRef(), Type: i32}, Type: i32}}} + return module + }, + want: "load place root is nil", + }, + { + name: "place index", + module: func() *Module { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + array := module.Types.Intern(ir.Type{Kind: ir.TypeArray, Elem: i32, Length: "1"}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &Load{Place: &Place{ + Root: &RefName{Name: "values", Type: array}, Projections: []PlaceProjection{{ + Kind: PlaceProjectionIndex, Index: typedNilValueRef(), Type: i32, + }}, Type: i32, + }, Type: i32}}} + return module + }, + want: "projection 0 index is nil", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assertValidationError(t, test.module(), test.want) + }) + } +} + +func TestValidatePreservesPlainNilSemantics(t *testing.T) { + t.Run("missing return value remains diagnostic", func(t *testing.T) { + module := wellFormed() + i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + module.Funcs[0].ReturnType = i32 + module.Funcs[0].Blocks[3].Term = &Ret{} + assertValidationError(t, module, "returns no value") + }) + + t.Run("payloadless variant may omit payload", func(t *testing.T) { + module := wellFormed() + variant := module.Types.Intern(ir.Type{Kind: ir.TypeVariant, Cases: []ir.VariantCase{{Name: "None"}}}) + module.Funcs[0].Blocks[0].Instrs = []Instr{&Assign{Value: &VariantMake{Case: 0, Type: variant}}} + if err := module.Validate(); err != nil { + t.Fatalf("Validate() = %v, want nil for payloadless variant", err) + } + }) +} + func TestValidateAcceptsMixedWidthShiftCount(t *testing.T) { module := wellFormed() i32 := module.Types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index bb89908e..18ed109b 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -491,13 +491,13 @@ func advanceModulePhase(ctx *project.CompilerContext, module *module.Module, dia if module.MIR == nil { return false } - // Emission assumes the MIR it is handed is well formed, and says so by - // panicking. The validator above is what makes that assumption safe, so - // nothing may reach emission once an error is recorded. if diag != nil && diag.HasErrors() { return false } module.LLVMIR = llvm.GenerateLLVMIR(module.MIR, phaseDiag, ctx.Target, ctx.Config.IsDebugBuild) + if phaseDiag.HasErrors() || module.LLVMIR == "" { + return false + } module.Phase = phase.Backend ctx.Metrics.AddPhaseAdvance() return true diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index 1ff8a8db..f0cd64e8 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -14,6 +14,7 @@ import ( "compiler/internal/frontend/lexer" "compiler/internal/frontend/parser" "compiler/internal/graph" + "compiler/internal/ir" "compiler/internal/ir/cfg" "compiler/internal/ir/mir" "compiler/internal/ir/thir" @@ -861,6 +862,45 @@ func TestPipelineSkipsIncompleteAnalysisValidationDuringRecovery(t *testing.T) { } } +func TestPipelineStopsBeforeBackendWhenEmitterRejectsMalformedMIR(t *testing.T) { + diag := diagnostics.NewDiagnosticBag() + types := ir.NewTypeTable() + voidType := types.Intern(ir.Type{Kind: ir.TypeVoid}) + i32 := types.Intern(ir.Type{Kind: ir.TypeInteger, IsSigned: true, Bits: 32}) + var value *mir.RefName + broken := &mir.Module{ + Name: "broken", Types: types, + Funcs: []*mir.Function{{ + Name: "broken", ReturnType: voidType, EntryID: 0, + Blocks: []*mir.Block{{ + ID: 0, + Instrs: []mir.Instr{&mir.Store{ + Place: &mir.Place{Root: &mir.RefName{Type: i32}, Type: i32}, Value: value, + }}, + Term: &mir.Ret{}, + }}, + }}, + } + + ctx := project.NewWithConfig(project.Config{RootDir: ".", Extension: peeper.SourceExt}, diag) + entry := &module.Module{ + ID: moduleid.ID{Origin: string(project.ModuleOriginLocal), ImportPath: "broken"}, + Phase: phase.MIR, THIR: &thir.Module{}, CFG: &cfg.Module{}, MIR: broken, + } + if advanceModulePhase(ctx, entry, diag) { + t.Fatal("advanceModulePhase() advanced after emitter rejected malformed MIR") + } + if !diag.HasErrors() || !strings.Contains(diag.EmitAllToString(), "MIR is malformed") { + t.Fatalf("diagnostics = %q, want malformed MIR diagnostic", diag.EmitAllToString()) + } + if entry.Phase != phase.MIR { + t.Fatalf("phase = %v, want MIR", entry.Phase) + } + if entry.LLVMIR != "" { + t.Fatalf("LLVM IR = %q, want no emission after MIR validation error", entry.LLVMIR) + } +} + func TestPipelinePreservesDiagnosticsForIdenticalRedeclarations(t *testing.T) { diag := buildPipelineTestWithConfig(t, project.Config{RootDir: ".", Extension: peeper.SourceExt}, "", ` fn same() -> i32 {