Emit full "Impl" types for delegates in 'cswinrtgen' #2161
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
Introduces a check to ensure the local variable type matches the expected ABI return type in generated interop methods. Throws a specific exception if a type mismatch is detected, improving error reporting and debugging for interop method generation.
Updated the marshaller type selection logic to include checks for Type and Exception types, in addition to custom-mapped Windows Runtime interfaces and delegates.
Introduces ManagedParameter class to handle marshalling of managed parameters in interop method rewrites. Supports various parameter types including value types, strings, generics, and reference types, with appropriate validation and marshaller method calls.
Replaces ReturnTypeMethodRewriteInfo and RetValTypeMethodRewriteInfo with nested MethodRewriteInfo.ReturnValue and MethodRewriteInfo.RetVal classes. Updates all usages and improves organization by grouping related types under MethodRewriteInfo. This change simplifies the codebase and clarifies the distinction between managed and unmanaged return value rewrites.
Introduces the ManagedParameter class to MethodRewriteInfo for handling managed parameters in two-pass IL generation. Updates InteropGenerator.Emit to process ManagedParameter instances using the appropriate rewrite method.
Updated the delegate Invoke method to use ABI-specific sender and argument types instead of void pointers. Introduced tracking of managed parameter method rewrites in InteropGeneratorEmitState to support this change.
Extended the type comparison logic to include IEnumerable, IEnumerator, and IList interfaces in WindowsRuntimeExtensions. This ensures these interfaces are now considered in relevant type checks.
Added mappings from System.Collections.IEnumerator to Microsoft.UI.Xaml.Interop.IBindableIterator and from Microsoft.UI.Xaml.Interop.IBindableIterator to Windows.UI.Xaml.Interop.IBindableIterator. This improves type translation support for collection interfaces.
Added mappings for IBindableIterable, IBindableIterator, and IBindableVector interfaces for both Windows.UI.Xaml and Microsoft.UI.Xaml projections. Moved the corresponding GUID assignments for IEnumerable, IEnumerator, and IList to the XAML section to ensure correct interface resolution.
Adds explicit tracking of the IMapChangedEventArgs1 type when handling MapChangedEventHandler<K,V> generic delegate types. This ensures that the event args type is also discovered and processed during interop type discovery.
Removed redundant Marshaller methods from multiple InteropTypeDefinitionBuilder partial classes and consolidated marshaller creation into a single public static method. Updated all call sites to use the unified Marshaller method, ensuring emitState tracking is consistently applied. This reduces code duplication and simplifies maintenance.
Replaces direct marshaller method calls with calls to their imported versions using `Import(module)`. Ensures correct method resolution during code generation for managed parameter marshalling.
There was a problem hiding this comment.
Pull request overview
This PR centralizes the logic for generating marshaller types and implements two-pass parameter rewriting for delegates in the 'cswinrtgen' tool. The key changes include refactoring method rewrite info classes into nested types, adding support for marshalling managed parameters, and consolidating marshaller type tracking.
- Refactored method rewrite info classes into nested types within
MethodRewriteInfo - Added support for two-pass IL generation for managed parameters in delegates
- Centralized marshaller type tracking to ensure all generated marshallers are registered
- Added support for
IEnumerator,IEnumerable, andIListinterface mappings
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
WellKnownInterfaceIIDs.cs |
Reorganized interface IID mappings and GUID assignments for XAML-related interfaces |
ReturnTypeMethodRewriteInfo.cs |
Removed standalone class (moved to nested type) |
RetValTypeMethodRewriteInfo.cs |
Removed standalone class (moved to nested type) |
MethodRewriteInfo.cs |
Made class partial to support nested type definitions |
MethodRewriteInfo.ReturnValue.cs |
Created nested type for managed return value rewriting |
MethodRewriteInfo.RetVal.cs |
Created nested type for unmanaged retval rewriting |
MethodRewriteInfo.ManagedParameter.cs |
Created nested type for managed parameter rewriting |
TypeMapping.cs |
Added IBindableIterator mappings for both Microsoft.UI and Windows.UI XAML |
InteropGeneratorEmitState.cs |
Added method to track managed parameter rewrites |
InteropGenerator.Emit.cs |
Updated to use centralized marshaller generation and handle managed parameter rewrites |
InteropMethodRewriteFactory.cs |
Added support for Type and Exception types in marshaller lookup |
InteropMethodRewriteFactory.ReturnValue.cs |
Added validation for ABI type matching |
InteropMethodRewriteFactory.ManagedParameter.cs |
Created factory for marshalling managed parameters |
WindowsRuntimeExtensions.cs |
Added IEnumerable, IEnumerator, and IList to custom-mapped interfaces |
WellKnownInteropExceptions.cs |
Added exception methods for parameter validation errors |
InteropTypeDiscovery.Generics.cs |
Added tracking for IMapChangedEventArgs when discovering observable maps |
InteropTypeDefinitionBuilder.cs |
Made Marshaller method public and added marshaller type tracking |
InteropTypeDefinitionBuilder.KeyValuePair.cs |
Moved marshaller tracking to after method addition |
Multiple InteropTypeDefinitionBuilder.*.cs files |
Removed interface-specific Marshaller methods in favor of centralized implementation |
InteropTypeDefinitionBuilder.Delegate.cs |
Implemented full ABI type generation for delegate parameters with two-pass rewriting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// <summary> | ||
| /// Tracks a method rewrite that involves loading a parameter in the specified method. | ||
| /// </summary> | ||
| /// <param name="paraneterType"><inheritdoc cref="MethodRewriteInfo.Type" path="/node()"/></param> |
There was a problem hiding this comment.
Corrected spelling of 'paraneter' to 'parameter'.
| /// <param name="marker"><inheritdoc cref="MethodRewriteInfo.Marker" path="/node()"/></param> | ||
| /// <param name="parameterIndex"><inheritdoc cref="MethodRewriteInfo.ManagedParameter.ParameterIndex" path="/node()"/></param> | ||
| public void TrackManagedParameterMethodRewrite( | ||
| TypeSignature paraneterType, |
There was a problem hiding this comment.
Corrected spelling of 'paraneterType' to 'parameterType'.
|
|
||
| _methodRewriteInfos.Add(new MethodRewriteInfo.ManagedParameter | ||
| { | ||
| Type = paraneterType, |
There was a problem hiding this comment.
Corrected spelling of 'paraneterType' to 'parameterType'.
|
|
||
| if (parameterType.IsValueType) | ||
| { | ||
| // If the return type is blittable, we can just load it directly it directly (simplest case) |
There was a problem hiding this comment.
Removed duplicate word 'directly'.
Suggested change
| // If the return type is blittable, we can just load it directly it directly (simplest case) | |
| // If the return type is blittable, we can just load it directly (simplest case) |
Comment on lines
+160
to
+166
| paraneterType: senderType, | ||
| method: invokeMethod, | ||
| marker: nop_parameter1Rewrite, | ||
| parameterIndex: 1); | ||
|
|
||
| emitState.TrackManagedParameterMethodRewrite( | ||
| paraneterType: argsType, |
There was a problem hiding this comment.
Corrected spelling of 'paraneterType' to 'parameterType'.
Suggested change
| paraneterType: senderType, | |
| method: invokeMethod, | |
| marker: nop_parameter1Rewrite, | |
| parameterIndex: 1); | |
| emitState.TrackManagedParameterMethodRewrite( | |
| paraneterType: argsType, | |
| parameterType: senderType, | |
| method: invokeMethod, | |
| marker: nop_parameter1Rewrite, | |
| parameterIndex: 1); | |
| emitState.TrackManagedParameterMethodRewrite( | |
| parameterType: argsType, |
Comment on lines
+160
to
+166
| paraneterType: senderType, | ||
| method: invokeMethod, | ||
| marker: nop_parameter1Rewrite, | ||
| parameterIndex: 1); | ||
|
|
||
| emitState.TrackManagedParameterMethodRewrite( | ||
| paraneterType: argsType, |
There was a problem hiding this comment.
Corrected spelling of 'paraneterType' to 'parameterType'.
Suggested change
| paraneterType: senderType, | |
| method: invokeMethod, | |
| marker: nop_parameter1Rewrite, | |
| parameterIndex: 1); | |
| emitState.TrackManagedParameterMethodRewrite( | |
| paraneterType: argsType, | |
| parameterType: senderType, | |
| method: invokeMethod, | |
| marker: nop_parameter1Rewrite, | |
| parameterIndex: 1); | |
| emitState.TrackManagedParameterMethodRewrite( | |
| parameterType: argsType, |
Replaces direct use of the delegate's Invoke method signature with retrieval of parameter types from interop references. This streamlines parameter extraction and improves code clarity.
Introduces handling for parameters of type 'Type' by using 'TypeMarshaller' for conversion to managed types. This ensures correct ABI value type marshalling for 'Type' parameters in interop scenarios.
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.
Title. This centralizes the logic and implements two-pass rewriting for the managed parameters.
Note
Draft because there's 3 commits that should be dropped first after rebasing (cherry-picked to another PR).