Emit full 'IVectorMethods' types for lists in 'cswinrtgen' #2166
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
Replaces the use of _corLibTypeFactory.CorLibScope with SystemObjectModel for creating the TypeReference for System.Windows.Input.ICommand. This change may improve consistency or correctness in type reference creation.
Added support for mapping the IVectorChangedEventArgs interface to its well-known IID string in WellKnownInterfaceIIDs. This improves interface recognition for Windows.Foundation.Collections.IVectorChangedEventArgs.
Relocated the IID mappings for IDisposable and IServiceProvider to an earlier position in the switch statement within WellKnownInterfaceIIDs.cs. This change ensures these interfaces are matched before others, likely to address precedence or matching issues.
Updated GuidGenerator and WellKnownInterfaceIIDs to support resolving interface IIDs based on the useWindowsUIXamlProjections flag. This enables correct GUID selection for interfaces that differ between Windows.UI.Xaml and Microsoft.UI.Xaml projections, improving compatibility and correctness in generated interop signatures.
Added [Guid] attributes to several Windows.Foundation and Windows.Foundation.Collections interface definitions to specify their interface IDs. This change improves COM interop and aligns the interfaces with their corresponding WinRT specifications.
Introduces EnumerateAllInterfaces to TypeSignatureExtensions, allowing traversal of all interfaces implemented by a type, including those from base types. This method handles generic types and ensures robust traversal even with incomplete type hierarchies.
Updated the logic to yield base interfaces directly without re-instantiating their generic types, as they are already instantiated when returned. This prevents unnecessary operations and improves code clarity.
Added a call to emitState.TrackTypeDefinition for implType and vectorType in the InteropTypeDefinitionBuilder.IObservableVector1 implementation. This ensures the type is tracked, which may be required for COM interface entries involving user-defined types.
Renamed and updated warning methods in WellKnownInteropExceptions for improved clarity regarding interface and base type resolution failures. Refactored InteropGenerator.Discover.cs to use the new warning methods, streamline interface discovery, and ensure correct handling of generic interface instantiations.
Deleted the EnumerateAllInterfaces method, which enumerated all interface implementations for a type and its base types. This cleanup may be due to redundancy, lack of use, or a refactor in interface enumeration logic.
Introduced HasBaseType extension to encapsulate base type checks and refactored related logic in TypeDefinitionExtensions and InteropGenerator.Discover. This improves code clarity and reduces duplication when handling type hierarchies and base type resolution.
Moved the logic for tracking exposed user-defined types from InteropGenerator.Discover.cs into a new InteropTypeDiscovery helper class. This improves code organization and reusability by encapsulating the discovery process in a dedicated static method.
Moved logic for tracking constructed generic types from InteropGenerator.Discover.cs into a new InteropTypeDiscovery.Generics.cs file. This refactoring centralizes and organizes generic type discovery, improving maintainability and separation of concerns.
Extended the EnumerateTypeSignatures method to process 'newobj' and 'newarr' CIL instructions, ensuring that object and array instantiations are included in the signature enumeration. This improves coverage of type signatures encountered in method bodies.
Moved SZ array and type hierarchy tracking logic from InteropGenerator.Discover.cs into dedicated helper methods in InteropTypeDiscovery. This improves code reuse and maintainability by centralizing the logic for type analysis and tracking.
Updated InteropTypeDiscovery tracking methods to return void instead of bool, as their return values were not used. Adjusted all call sites and removed related comments and unreachable code for clarity.
Added checks to skip unconstructed generic type definitions during interop type discovery and generation. This ensures only constructed generic types are considered for marshalling code, improving accuracy and avoiding unnecessary processing of generic definitions.
Co-authored-by: Copilot <[email protected]>
Introduced new test cases to validate activation and interface QueryInterface (QI) for constructed generic types and their derived types. Refactored QI logic into a reusable ComHelpers.EnsureQueryInterface method. Added several generic and derived classes to exercise interface exposure and activation scenarios.
Introduces tests for IAsyncActionWithProgress<int> and IAsyncOperation<TimeSpan> using AsyncInfo.Run with explicit and transitive type arguments. Ensures correct COM interface querying and memory management for these async operations.
Added XML documentation remarks to the Keys and Values properties in WindowsRuntimeDictionary, WindowsRuntimeObservableMap, and WindowsRuntimeReadOnlyDictionary classes to clarify the concrete collection types returned. This improves API documentation and developer understanding of the property return types.
Replaces direct calls to TrackGenericInterfaceType with a new TryTrackWindowsRuntimeGenericInterfaceTypeInstance method. This centralizes and extends the logic for tracking constructed generic Windows Runtime interface types, ensuring all necessary related types and delegates are also tracked for code generation.
Deleted the InteropGeneratorDiscoveryStateExtensions.cs file, which contained extension methods for tracking generic interface and delegate types in the interop generator discovery process. This may be part of a refactor or cleanup to simplify or relocate this logic.
Updated generic type discovery logic to explicitly track constructed DictionaryKeyCollection, DictionaryValueCollection, ReadOnlyDictionaryKeyCollection, and ReadOnlyDictionaryValueCollection types when discovering IDictionary<TKey, TValue> and IReadOnlyDictionary<TKey, TValue> instantiations. This ensures correct marshaling of these types, which may not be present in input assemblies but are generated during emit. Also refactored method signatures to consistently pass the current module and args where needed.
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.
Clarified and corrected XML documentation for HasReferenceAbiType and GetAbiType methods to better describe their return values and behavior.
Consolidates logic for resolving marshaller types for custom-mapped Windows Runtime structs and classes, removing special cases for TimeSpan and DateTimeOffset. Updates checks to use IsCustomMappedWindowsRuntimeNonGenericStructOrClassType for consistency and maintainability.
Introduces a new TryGetNullableUnderlyingType method to extract the underlying type from a constructed Nullable<T> type. Also updates XML documentation to reference types without the System. prefix for clarity.
Moved marshaller type resolution logic from InteropMethodRewriteFactory to a new InteropMarshallerTypeResolver class. Updated all usages to use the new resolver, improving code organization and maintainability. Also fixed a bug in WindowsRuntimeExtensions to correctly check for value types in generic instance signatures.
Marked ExceptionMarshaller as unsafe and added BoxToUnmanaged and UnboxToManaged methods to support boxing and unboxing of System.Exception objects for interop scenarios.
Introduces InteropMarshallerType as a ref struct to encapsulate marshaller type and method resolution. Updates all usages to leverage strongly-typed marshaller method accessors, improving code clarity and reducing repeated logic. Refactors InteropMarshallerTypeResolver to return InteropMarshallerType instead of ITypeDefOrRef.
Removed special-case handling for generic instance types (such as KeyValuePair and constructed interfaces/delegates) in parameter and return value marshalling. Generic types are now handled by the standard marshaller resolution logic, simplifying the code and reducing duplication.
Introduces the CreateStind extension method to generate the appropriate indirect store (stind) instruction based on the provided type signature. This enhances the CilInstructionExtensions utility for handling various value types and custom types when emitting IL.
Replaces the switch statement for selecting the correct indirect store instruction for blittable types with a call to CilInstruction.CreateStind. This simplifies the code and centralizes the logic for determining the appropriate store instruction.
Replaced hardcoded HSTRING types in delegate comments with <ELEMENT_TYPE>, <KEY_TYPE>, and <VALUE_TYPE> placeholders to generalize the function pointer signatures for broader applicability. This improves clarity and maintainability when generating code for different types.
Changed a comment to indicate that arguments are loaded inside an outer 'try/finally' block instead of 'try/catch', improving code clarity.
Inserted the Conv_U opcode in both IAsyncInfoMethods and IVectorViewMethods to ensure correct type conversion during interop method generation. This change improves type safety and correctness in the generated interop code.
Introduces a local variable 'elementAbiType' to store the ABI type of the element and uses it consistently when defining locals and method signatures for the 'GetAt' method. This improves code clarity and ensures the correct ABI type is used throughout the method generation process.
Moved the construction of the HasKey method for IMapView<K,V> from InteropTypeDefinitionBuilder to a new InteropMethodDefinitionFactory.IMapViewMethods class. This centralizes method creation logic and enables more maintainable and reusable code for interop method definitions.
Refactored Lookup method creation in InteropTypeDefinitionBuilder to use InteropMethodDefinitionFactory.IMapViewMethods.Lookup. Added full implementation of the Lookup method in InteropMethodDefinitionFactory, including method body, local variables, and parameter/return value rewriting logic for IMapView<K, V> interop.
Replaced manual construction of HasKey and Lookup methods with calls to InteropMethodDefinitionFactory.IMapViewMethods. This improves maintainability and centralizes method creation logic.
Moved the Insert method definition for IDictionary2 from InteropTypeDefinitionBuilder to a new InteropMethodDefinitionFactory.IMapMethods helper. This centralizes Insert method creation logic and improves maintainability by encapsulating method body generation and parameter marshaling.
Refactored the Remove method definition for IMap<TKey, TValue> interop types to use a new factory method. The new implementation generates a complete method body with proper marshaling, exception handling, and resource cleanup, improving maintainability and correctness.
Replaces valueType with valueAbiType in the call to WellKnownTypeSignatureFactory.IReadOnlyDictionary2LookupImpl to ensure the correct ABI type is used for the value parameter.
Moved the SetAt method logic for IList<T> to InteropMethodDefinitionFactory.IVectorMethods, replacing the previous placeholder implementation. This centralizes method creation and provides a complete method body for SetAt, improving maintainability and consistency.
Introduces a shared SetAtOrInsertAt helper to generate both SetAt and InsertAt methods for IVector<T> interfaces, reducing code duplication and improving maintainability. Updates method body construction to use the new helper and parameterize method name and signature.
Moves InsertAt method definition logic from InteropTypeDefinitionBuilder.IList1 to InteropMethodDefinitionFactory.IVectorMethods. This centralizes method creation and reduces duplication, improving maintainability.
Moved the construction of the IList<T>.Append method to InteropMethodDefinitionFactory.IVectorMethods.Append, providing a full method body and parameter marshaling logic. This improves maintainability and consistency with other interop method definitions.
Refactored IList<T> interop builder to use a new InteropMethodDefinitionFactory.IVectorMethods.IndexOf method, which now provides a full implementation for the IndexOf method, including method body and parameter marshaling. This improves maintainability and consistency with other IVector methods.
Updated the XML documentation for CilOutParameterIndices to clarify that parameter indices are 1-based, with index 0 representing the implicit 'this' parameter.
There was a problem hiding this comment.
Pull request overview
This PR implements full code generation for IVectorMethods types in the cswinrtgen tool. Previously, these methods had placeholder implementations that threw exceptions; now they generate complete CIL method bodies with proper marshalling, exception handling, and resource cleanup.
Key changes:
- Adds factory methods for generating
SetAt,InsertAt,Append, andIndexOfimplementations - Implements proper try/finally blocks for resource cleanup
- Handles parameter marshalling with native type conversion
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
InteropMethodDefinitionFactory.IVectorMethods.cs |
New file containing factory methods for generating complete IVector<T> method implementations with CIL bodies |
MethodDefinitionExtensions.cs |
Added documentation clarifying that CilOutParameterIndices uses 1-based indexing |
InteropTypeDefinitionBuilder.IList1.cs |
Replaced placeholder method stubs with calls to the new factory methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Track rewriting the 'value' parameter for this method | ||
| emitState.TrackNativeParameterMethodRewrite( | ||
| paraneterType: elementType, |
There was a problem hiding this comment.
Corrected spelling of 'paraneterType' to 'parameterType'.
Suggested change
| paraneterType: elementType, | |
| parameterType: elementType, |
|
|
||
| // Track rewriting the 'value' parameter for this method | ||
| emitState.TrackNativeParameterMethodRewrite( | ||
| paraneterType: elementType, |
There was a problem hiding this comment.
Corrected spelling of 'paraneterType' to 'parameterType'.
Suggested change
| paraneterType: elementType, | |
| parameterType: elementType, |
|
|
||
| // Track rewriting the 'value' parameter for this method | ||
| emitState.TrackNativeParameterMethodRewrite( | ||
| paraneterType: elementType, |
There was a problem hiding this comment.
Corrected spelling of 'paraneterType' to 'parameterType'.
Suggested change
| paraneterType: elementType, | |
| parameterType: elementType, |
manodasanW
approved these changes
Jan 10, 2026
Sergio0694
force-pushed
the
user/sergiopedri/map-methods
branch
from
9dd39aa to
7205901
Compare
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.
Examples of codegen:
Note
Draft because there's some commits to move and this branch also needs to be rebased.