Emit full "Impl" types for IReadOnlyList<T> in 'cswinrtgen'
#2168
+713
−143
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 new extension method for IReadOnlyList<string> to provide an IndexOf overload that accepts a ReadOnlySpan<char>, allowing callers to avoid string allocations. The extension is marked obsolete and editor-browsable never, indicating it is for internal or transitional use.
Introduced a static Count method to retrieve the number of items in an IReadOnlyList<T>, with aggressive inlining for performance. This provides a convenient and efficient way to access the count, aligning with IVectorView<T> API expectations.
Replaces instances of 'unboxedValue' with 'thisObject' across multiple ABI and interop files for improved code readability and consistency. No functional changes were made.
Introduces a factory for generating the GetAt method implementation for IReadOnlyList<T> interop types. Updates the type builder to include the generated GetAt method in the vtable, and extends InteropReferences with support for IReadOnlyListAdapter<T> and its GetAt method reference.
Introduces the get_Size method for IReadOnlyList<T> interop, including its definition in the generator, factory, and references. Also renames the Count method to Size in IReadOnlyListAdapter<T> to match the interop signature.
Introduces support for the IndexOf method in the IReadOnlyList<T> COM interop implementation. This includes the method definition, its factory logic, and a new reference accessor for IndexOf in InteropReferences.
Introduces handling for 'ReadOnlySpan<char>' parameters in interop method rewriting, using 'HStringMarshaller.ConvertToManagedUnsafe' for marshalling without materializing a string object. Also adds a reference to the new marshaller method in InteropReferences.
Refactors the interop method generation for IReadOnlyList<T>.IndexOf to use a specialized method for string element types, leveraging ReadOnlySpan<char> to avoid allocations. Adds new references and logic to support this optimization in the interop references and method body generation.
Removed an extra uint parameter from the GetMany method signatures in IVectorVftbl and IVectorViewVftbl, and updated WellKnownTypeSignatureFactory to match. This corrects the method signature to align with the expected delegate definition.
Introduces the 'GetMany' method to the IReadOnlyList<T> interop implementation, including its definition and integration into the vtable. This supports additional COM interop scenarios requiring bulk retrieval of elements.
Corrects the misspelled parameter name 'paraneterType' to 'parameterType' across multiple files for consistency and clarity in method signatures and documentation.
There was a problem hiding this comment.
Pull request overview
This PR implements full "Impl" type code generation for IReadOnlyList<T> in the cswinrtgen tool. The changes include correcting the GetMany vtable signature by removing an unused parameter, implementing adapter methods (GetAt, Size, IndexOf), adding a specialized string overload to avoid allocations, and renaming variables from unboxedValue to thisObject for consistency across the codebase.
Key Changes:
- Corrected
GetManyvtable signature inIVectorViewVftblandIVectorVftblby removing unuseduintparameter - Implemented
IReadOnlyList<T>adapter methods and code generation - Standardized variable naming from
unboxedValuetothisObjectacross ABI implementation files
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| IVectorViewVftbl.cs | Updated GetMany signature to remove unused uint parameter |
| IVectorVftbl.cs | Updated GetMany signature to remove unused uint parameter |
| IReadOnlyListAdapter{T}.cs | Added Size method for IReadOnlyList adapter |
| IReadOnlyListAdapterExtensions.cs | Added optimized IndexOf overload for string to avoid allocations |
| InteropReferences.cs | Added type and method references for IReadOnlyListAdapter |
| WellKnownTypeSignatureFactory.cs | Updated GetMany signatures and corrected comments |
| InteropMethodRewriteFactory.ManagedParameter.cs | Added support for marshalling ReadOnlySpan parameters |
| InteropMethodDefinitionFactory.IReadOnlyList1Impl.cs | New file implementing GetAt, get_Size, IndexOf, and GetMany methods |
| InteropTypeDefinitionBuilder.IReadOnlyList1.cs | Integrated new impl methods into type builder |
| Multiple ABI files | Renamed unboxedValue to thisObject for consistency |
| InteropGeneratorEmitState.cs | Fixed parameter name typo from paraneterType to parameterType |
Comments suppressed due to low confidence (1)
src/WinRT.Interop.Generator/Builders/InteropTypeDefinitionBuilder.IReadOnlyList1.cs:1
- This checks if
Ldarg_2(index parameter) is null, butLdarg_3(result parameter) should be checked instead. The null check should verify the output parameter pointer, not the uint index value.
// Copyright (c) Microsoft Corporation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/WinRT.Runtime2/InteropServices/Collections/IReadOnlyListAdapterExtensions.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Interop.Generator/Factories/InteropMethodDefinitionFactory.IReadOnlyList1Impl.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
GetManyis an empty stub for now, will implement in a follow up PR.Examples of codegen:
Note
Draft because there's some commits to move and this branch also needs to be rebased.