Refactor projection generator phases #2233
Open
+180
−78
Conversation
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
Split the projection generator into distinct processing, generation and emit phases and introduce a processing state object to pass data between them. Key changes: - Add ProjectionGeneratorProcessingState to hold sources folder, response file path and non-projection references. - Extract reference processing into ProcessReferences which generates the .rsp and returns the processing state. - Move CsWinRT invocation into GenerateSources which now takes the processing state and runs the tool, throwing on non-zero exit. - Change Emit to accept the processing state, read generated .cs files from the sources folder, build the Roslyn compilation and emit the projection DLL. - Remove duplicated helper methods (CreateCompilationForProjection and RunCsWinRT) and inline logic; update logging and error contexts. - Minor cleanup of usings and improved cancellation/error handling across the pipeline. This refactor clarifies responsibilities of each phase and makes the flow more explicit and testable.
There was a problem hiding this comment.
Pull request overview
Refactors the WinRT projection generator pipeline into clearer phases (processing → source generation → emit) and centralizes user-facing status messages in ProjectionGenerator.Run, aligning behavior more closely with cswinrtgen.
Changes:
- Introduces
ProjectionGeneratorProcessingStateto carry state between phases. - Splits reference processing (
ProcessReferences), CsWinRT execution (GenerateSources), and Roslyn emit (Emit) into separate steps orchestrated byRun. - Updates progress/status messages and phase names used for error wrapping.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/WinRT.Projection.Generator/Generation/ProjectionGeneratorProcessingState.cs | Adds a small state container for cross-phase data (sources folder, rsp path, filtered references). |
| src/WinRT.Projection.Generator/Generation/ProjectionGenerator.cs | Refactors Run into explicit phases with updated top-level logging and error-phase labels. |
| src/WinRT.Projection.Generator/Generation/ProjectionGenerator.Generate.cs | Splits reference processing from running CsWinRT; rehomes CsWinRT process launch logic. |
| src/WinRT.Projection.Generator/Generation/ProjectionGenerator.Emit.cs | Updates emit logic to consume state from the processing phase and emits the final DLL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/WinRT.Projection.Generator/Generation/ProjectionGenerator.Generate.cs
Show resolved
Hide resolved
src/WinRT.Projection.Generator/Generation/ProjectionGenerator.Generate.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Projection.Generator/Generation/ProjectionGenerator.Emit.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Projection.Generator/Generation/ProjectionGenerator.cs
Outdated
Show resolved
Hide resolved
Make the projection generator responsive to cancellation by calling args.Token.ThrowIfCancellationRequested at key points: inside the source file processing loop, after loading metadata references, after compilation creation, and before emit. This prevents long-running emit work from continuing when the operation has been cancelled.
Append the .dll extension to the generated projection assembly path in the console log to make the logged filename explicit. This is a logging-only change to improve clarity; no functional behavior is altered.
Improve clarity and resource handling in the projection generator: reword XML doc comments to better describe processing and emit phases; parse sources into syntax trees (comment fix) and adjust comment wording for building references; emit compilation using an explicit using block and a separate EmitResult variable to ensure proper disposal; reformat GenerateRspFile signature and clarify its parameter docs; initialize PathAssemblyResolver in the rsp generation loop; add inline comments marking processing, generation and compilation phases. These are primarily readability and resource-management improvements without changing core logic.
Add explicit error handling and well-known exceptions for cswinrt.exe failures. - New exceptions in WellKnownProjectionGeneratorExceptions: CsWinRTProcessStartError (with/without inner exception) and CsWinRTProcessError (includes exit code). - Update ProjectionGenerator.Generate to safely start the cswinrt process, detect a null Process return, wrap start failures (excluding existing well-known exceptions), read StandardError, wait for exit, and throw the new well-known exceptions when the process fails to start or exits with a non-zero code. - Add required using and pragma to accommodate the changes. These changes improve diagnostics and robustness when invoking the external cswinrt.exe tool.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Title. This makes it more consistent with 'cswinrtgen' and moves all output messages to the top level
Runmethod.I also reworked the messages to be clearer and to better match the actual work: