Shared expression-visitor base for codegen backends (PChecker migration)#950
Merged
Conversation
First step of consolidating the imperative code-generator backends (PChecker, PEx, PObserve), which each independently walk the AST with a parallel ~31-case WriteExpr switch. A missed node fails silently (wrong output) rather than at compile time, so the backends drift apart as the language evolves. Introduces ExpressionGenerator<TContext>, which owns the IPExpr dispatch once and declares one abstract Write*Expr method per expression node. Backends extend it and implement the per-node emission; adding a new IPExpr node (plus a dispatch arm) now forces every backend to handle it or fail to compile. This commit migrates PChecker: its WriteExpr switch is replaced by 31 overrides whose bodies are moved verbatim, so generated code is unchanged. Verified by compiling tutorials 1-6 with the old and new compiler and diffing the generated C# -- byte-for-byte identical. PEx and PObserve will be migrated in follow-up PRs; PVerifier stays on its own functional ExprToString (different paradigm) by design.
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a shared ExpressionGenerator<TContext> base class to centralize IPExpr dispatch for imperative codegen backends, and migrates the PChecker backend to implement per-expression emission via overrides (eliminating the local WriteExpr switch).
Changes:
- Added
ExpressionGenerator<TContext>with a singleIPExprdispatch switch and one abstractWrite*Exprmethod per supported expression node. - Refactored
PCheckerCodeGeneratorto inherit fromExpressionGenerator<CompilationContext>and replace its expression switch withprotected override Write*Exprimplementations. - Adjusted
PCheckerCodeGeneratorvisibility frompublictointernalto align accessibility with the internal base/context usage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Src/PCompiler/CompilerCore/Backend/PChecker/PCheckerCodeGenerator.cs | Migrates PChecker expression emission to ExpressionGenerator overrides and updates generator visibility. |
| Src/PCompiler/CompilerCore/Backend/ExpressionGenerator.cs | Adds shared expression dispatch base class to enforce backend exhaustiveness at compile time. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced May 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First step of consolidating the imperative code-generator backends (PChecker, PEx, PObserve). Today each one independently walks the AST with its own parallel ~31-case
WriteExprswitch. They handle the identical set of 31 expression nodes, but nothing enforces that — a missed node fails silently (wrong generated output) instead of at compile time, so the backends drift as the language evolves.This PR introduces
ExpressionGenerator<TContext>(Backend/ExpressionGenerator.cs), which owns theIPExprdispatch once and declares oneabstract Write*Exprmethod per expression node. Backends extend it and implement the per-node emission. Adding a newIPExprnode (plus its dispatch arm) now forces every backend to implement it or the build breaks — that compile-time exhaustiveness is the whole point.This commit migrates PChecker onto the base; PEx and PObserve follow in separate PRs. PVerifier stays separate by design — it's functional (
ExprToStringreturns UCLID5 terms) and handles a different, verification-specific node set, so forcing it under this base would be a leaky abstraction.What changed
ExpressionGenerator<TContext>base (dispatch + 31 abstract methods).PCheckerCodeGenerator'sWriteExprswitch replaced by 31overridemethods, bodies moved verbatim. Class changedpublic→internal(it's only referenced fromTargetLanguage, same assembly) to satisfy accessibility for theprotectedoverrides over the internalCompilationContext.Verification
CompilerCorebuilds, 0 errors (only the pre-existing unrelatedTransformASTPass.cs:796CS0162 warning).diff -rqon the generated C#: identical, no differences.Notes
Context: implements the "shared visitor base for the imperative backends" simplification (PVerifier kept separate, phased rollout) discussed for reducing multi-backend maintenance cost. Follow-ups: PEx expressions, then statements, then PObserve.
Generated by Claude Code