-
Notifications
You must be signed in to change notification settings - Fork 122
Emit full "Impl" types for IDictionary<TKey, TValue> in 'cswinrtgen' #2173
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: user/sergiopedri/read-only-dictionary-impl
Are you sure you want to change the base?
Emit full "Impl" types for IDictionary<TKey, TValue> in 'cswinrtgen' #2173
Conversation
Introduces IDictionaryAdapter<TKey, TValue> as a stateless adapter for IDictionary to expose Windows.Foundation.Collections.IMap functionality. Adds IDictionaryAdapterExtensions for string-keyed dictionaries to optimize lookup, removal, and key existence checks using ReadOnlySpan<char> to avoid string allocations.
Introduces support for generating the 'Lookup' method for IDictionaryAdapter<TKey, TValue> in the IReadOnlyDictionary2 implementation. Adds new type and member references for IDictionaryAdapter2 and its Lookup method, and updates the method factory to use these references for non-string key types.
Introduces the definition and vtable inclusion of the 'get_Size' method for IDictionary2 implementations in the interop type builder. Adds a new helper in InteropReferences to retrieve the MemberReference for IDictionaryAdapter2.Size, supporting correct method generation for COM interop.
Introduces the HasKey method to the IDictionary2 implementation and updates related method signatures and references for consistency. Also corrects parameter naming from 'hasKeyMethod' to 'containsKeyMethod' and adds a new method reference for IDictionaryAdapterOfStringHasKey in InteropReferences.
Introduces a new interop method definition for the GetView method on IDictionary<TKey, TValue> implementations. Updates type discovery to track IReadOnlyDictionary and ReadOnlyDictionary types needed for marshalling GetView results. Adds required references and method factories to support this functionality.
Eliminated unnecessary using statements for System.Collections and System.Diagnostics to clean up the code and improve maintainability.
Updated the XML comment to reference TryMarkUserDefinedType instead of the incorrect MarkUserDefinedType, improving documentation accuracy.
Introduces the Insert method implementation for IDictionary2 interop types by defining the method in InteropMethodDefinitionFactory, wiring it into InteropTypeDefinitionBuilder, and adding the necessary reference in InteropReferences. This enables proper handling of Insert operations for projected IDictionary2 types.
Implements the 'Remove' method for IDictionary2 interop type definitions, including method creation in InteropMethodDefinitionFactory and reference resolution in InteropReferences. This enables proper COM interface generation for dictionary removal operations.
Introduces a Clear method to the IDictionary2 interop type builder and its method factory. This enables support for clearing dictionary instances in the generated interop code.
Uncommented the check to enforce that all vtable entries are initialized by throwing an ArgumentOutOfRangeException if the counts do not match.
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 implements full "Impl" type generation for IDictionary<TKey, TValue> in the cswinrtgen code generator, enabling proper marshaling of dictionary types between managed and unmanaged code. It includes optimizations for string keys to avoid unnecessary allocations by using ReadOnlySpan<char> lookup methods.
Key changes:
- Added adapter classes (
IDictionaryAdapterandIDictionaryAdapterExtensions) for marshaling dictionary operations - Implemented all required vtable methods (Lookup, Size, HasKey, GetView, Insert, Remove, Clear) for dictionary impl types
- Added string-optimized overloads using
ReadOnlySpan<char>to avoid allocations
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
IDictionaryAdapter{TKey, TValue}.cs |
New adapter for marshaling IDictionary<TKey, TValue> operations to WinRT IMap interface |
IDictionaryAdapterExtensions.cs |
String-optimized extension methods using ReadOnlySpan<char> for dictionary lookups |
InteropReferences.cs |
Added references to new adapter types and methods for code generation |
InteropMethodDefinitionFactory.IReadOnlyDictionary2Impl.cs |
Fixed TODO comments and updated to use correct dictionary adapter methods |
InteropMethodDefinitionFactory.IDictionary2Impl.cs |
New factory methods generating impl type vtable methods for IDictionary<TKey, TValue> |
InteropTypeDiscovery.Generics.cs |
Added tracking for IReadOnlyDictionary<TKey, TValue> and ReadOnlyDictionary<TKey, TValue> dependencies |
InteropTypeDefinitionBuilder.cs |
Uncommented vtable validation check |
InteropTypeDefinitionBuilder.IReadOnlyDictionary2.cs |
Minor cleanup and parameter name correction |
InteropTypeDefinitionBuilder.IDictionary2.cs |
Implemented full vtable method generation for dictionary impl types |
InteropGeneratorDiscoveryState.cs |
Fixed comment reference to renamed method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/WinRT.Interop.Generator/Builders/InteropTypeDefinitionBuilder.IDictionary2.cs
Show resolved
Hide resolved
src/WinRT.Runtime2/InteropServices/Collections/IDictionaryAdapterExtensions.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Interop.Generator/Builders/InteropTypeDefinitionBuilder.IDictionary2.cs
Show resolved
Hide resolved
| MethodDefinition hasKeymethod = InteropMethodDefinitionFactory.IReadOnlyDictionary2Impl.HasKey( | ||
| readOnlyDictionaryType: dictionaryType, |
Copilot
AI
Jan 5, 2026
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.
Variable name uses inconsistent casing: 'hasKeymethod' should be 'hasKeyMethod' (capital M) to follow C# naming conventions.
Renamed 'hasKeymethod' to 'hasKeyMethod' in both IDictionary2 and IReadOnlyDictionary2 interop builders for consistency and clarity.
Title. Also added some optimizations for
stringkeys.Examples of codegen:
Note
Draft because there's some commits to move and this branch also needs to be rebased.