-
Notifications
You must be signed in to change notification settings - Fork 122
Add support for reference projections #2143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging/3.0
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for reference projections in CsWinRT, enabling a new build configuration where projection projects can generate reference assemblies and forwarder assemblies, with a merged projection created at the final application level.
Key Changes:
- Adds new
CsWinRTGenerateReferenceProjectionproperty andCSWINRT_REFERENCE_PROJECTIONconditional compilation symbol - Introduces code generation modifications to exclude ABI namespace types when generating reference projections
- Implements new generator tools (
cswinrtprojectiongenandcswinrtimplgen) with supporting infrastructure
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/cswinrt/settings.h |
Adds reference_projection boolean flag to settings |
src/cswinrt/main.cpp |
Implements command-line argument parsing and conditional ABI namespace generation |
src/cswinrt/code_writers.h |
Modifies code writers to conditionally generate throw null stubs and exclude ABI-related attributes |
src/cswinrt/strings/ComInteropHelpers.cs |
Wraps interop helper implementations with preprocessor directives |
src/cswinrt/strings/additions/* |
Conditionally excludes marshaller attributes for reference projections |
src/WinRT.Projection.Generator/* |
New projection generator tool for creating merged projections |
src/RunCsWinRTGeneratorTask/* |
New MSBuild tasks for running generator tools |
nuget/Microsoft.Windows.CsWinRT.targets |
Configures build properties and targets for reference projection support |
src/cswinrt.slnx |
Updates solution configuration |
src/Directory.Packages.props |
Updates Roslyn package versions |
💡 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/Errors/WellKnownImplException.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Projection.Generator/Errors/WellKnownImplExceptions.cs
Outdated
Show resolved
Hide resolved
…Generate.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
src/WinRT.Projection.Generator/Generation/ProjectionGenerator.Emit.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Projection.Generator/WinRT.Projection.Generator.csproj
Outdated
Show resolved
Hide resolved
| [WindowsRuntimeMetadata("Microsoft.UI")] | ||
| [WindowsRuntimeClassName("Windows.Foundation.IReference<Microsoft.UI.Xaml.Media.Animation.KeyTime>")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these attributes should also be hidden from reference assemblies, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically we use these in cswinrtgen right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but I feel like we should still find a way to hide them. In theory both the metadata and the runtime class name can be derived at compile time anyway. We could check if a type is a projected type just by checking if the defining assembly is a projection assembly, and the runtime class name we can just compute in 'cswinrtgen' easily, we already have most of the logic for it anyway. That would allow us to just keep these attributes in the merged projection and hide them from here.
src/WinRT.Projection.Generator/Generation/ProjectionGenerator.Emit.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Projection.Generator/Generation/ProjectionGenerator.Emit.cs
Outdated
Show resolved
Hide resolved
| /// </summary> | ||
| public AssemblyReference WinRTProjection => field ??= new AssemblyReference( | ||
| name: "WinRT.Projection"u8, | ||
| version: new Version(0, 0, 0, 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking this should rather match the .dll version with WinRT.Interop.dll? You can copy the logic we use for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically we can, but I don't pass the project dll to the projection generator today and it is generated differently from WinRT.Interop due to it uses the CSharpCompilation to do so. We can technically pass a parameter for the version and then add a version attribute which is passed to the CSharpCompilation if we want, but does it add any benefit?
… by making this only build reference assemblies and then removing the constructors that reference it.
…ing implementation assemblies other than for projection DLLs
Introduces IsWindowsRuntimeReferenceAssembly property to check for the WindowsRuntimeReferenceAssemblyAttribute. Adds related metadata names to WellKnownMetadataNames for attribute detection.
Refined logic for filtering and processing assemblies in Interop and Impl generators, clarified XML documentation, and added comments to explain handling of Windows Runtime projections and implementation assemblies. Improved method signatures and parameter documentation for better maintainability and clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 63 out of 63 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/WinRT.Projection.Generator/Errors/WellKnownImplExceptions.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Projection.Generator/Errors/WellKnownImplExceptions.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Projection.Generator/Errors/WellKnownImplExceptions.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Interop.Generator/Generation/InteropGenerator.DebugRepro.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Interop.Generator/Generation/InteropGenerator.DebugRepro.cs
Outdated
Show resolved
Hide resolved
src/RunCsWinRTGeneratorTask/RunCsWinRTMergedProjectionGenerator.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Projection.Generator/Generation/ProjectionGeneratorArgs.Parsing.cs
Show resolved
Hide resolved
Corrected minor typos in comments to consistently refer to 'assembly paths' and 'output assembly path' for improved clarity.
|
There was an error handling pipeline event f7a99059-766f-4842-a682-03c3361647ac. |
CsWinRTGenerateReferenceProjectionset to true. When this is set, a reference assembly and a forwarder assembly is generated (replaces the output DLL that gets built).ProduceOnlyReferenceAssemblywhich is possible but that relied on an assumption that if we generated sources which referenced ABI namespaces that aren't there that they will continue to build like they do today and also have more msbuild target changes to enable (seen here). So, to keep it straight forward, we do the equivalent in codewriters instead.