-
Notifications
You must be signed in to change notification settings - Fork 121
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
…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
| if (settings.reference_projection) | ||
| { | ||
| w.write(R"( | ||
| private static WindowsRuntimeObjectReference % |
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.
WindowsRuntimeObjectReference is a private implementation detail type, so we shouldn't use it in the ref assembly. Can we skip emitting this property entirely, since it's private anyway? Like, we shouldn't need to emit any non-public members in general.
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.
My goal was to leave as much as to the .NET reference assembly thing to remove things that are not on the public API surface. So these should still be removed in the reference assembly. But I will note that the protected constructors do show up which do reference WindowsRuntimeObjectReference. I wonder if we should remove those in ref assemblies or not.
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.
Mmmhhhhh. I would think we should be able to remove them, since at runtime the method would still be there anyway. The only difference is that user code would not be able to see them. Which honestly is even better. @jkoritzinsky can you confirm that it's fine for us to omit those constructors in the ref assemblies?
| [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?
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.
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.