Optimize ConvertToUnmanaged codegen for sealed runtime classes#2345
Open
Sergio0694 wants to merge 6 commits intostaging/3.0from
Open
Optimize ConvertToUnmanaged codegen for sealed runtime classes#2345Sergio0694 wants to merge 6 commits intostaging/3.0from
Sergio0694 wants to merge 6 commits intostaging/3.0from
Conversation
For projected sealed runtime classes the RCW can never be subclassed, so unwraps are always valid. Replace the TryUnwrapObjectReference block with a direct access to value?.NativeObjectReference.AsValue() and add an explanatory comment in src/cswinrt/code_writers.h to simplify and optimize the logic. Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates cswinrt.exe codegen to optimize the generated ConvertToUnmanaged implementation for sealed projected runtime classes, aiming to skip TryUnwrapObjectReference and directly use the underlying object reference.
Changes:
- Update
write_class_marshallersealed-class branch to emit a directNativeObjectReferenceaccess forConvertToUnmanaged. - Add an explanatory comment describing why sealed runtime classes can skip unwrapping logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR review feedback: - Replace direct NativeObjectReference access (which is protected internal) with WindowsRuntimeObjectMarshaller.ConvertToUnmanaged, a public helper that takes the fast path for sealed types since HasUnwrappableNativeObjectReference is always true. - Remove 'return default;' from the shared template and move it into each non-sealed branch, so the sealed branch's unconditional return no longer produces unreachable code (CS0162). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a new public method that directly unwraps the NativeObjectReference from a WindowsRuntimeObject and returns it as a value, without validating HasUnwrappableNativeObjectReference. This enables callers that already know the object is unwrappable (e.g. sealed projected runtime classes) to skip the TryUnwrapObjectReference overhead. Also add class-level XML remarks documenting that no method in this class performs input validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…degen Update the sealed branch to emit a call to the new UnwrapObjectReferenceUnsafe API, guarded by a null check. This avoids both the TryUnwrapObjectReference overhead and the accessibility issue with NativeObjectReference (which is protected internal). Also restore the centralized 'return default;' in the template, since all three branches now use the same conditional-return pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Have UnwrapObjectReferenceUnsafe return the raw object reference directly, and let the generated caller do .AsValue() on it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
This PR optimizes the
ConvertToUnmanagedmethod generated bycswinrt.exefor sealed projected runtime classes.Motivation
For sealed runtime classes, the RCW type is always unwrappable (since the type can never be subclassed). The current codegen unnecessarily goes through
TryUnwrapObjectReference, which involves extra branching and an out parameter. We can skip this entirely and directly accessNativeObjectReferenceon the value.Changes
In
write_class_marshaller(code_writers.h), the sealed branch now emits:\\csharp
// Before (eg. FontWeightsMarshaller)
public static WindowsRuntimeObjectReferenceValue ConvertToUnmanaged(FontWeights value)
{
if (WindowsRuntimeComWrappersMarshal.TryUnwrapObjectReference(value, out WindowsRuntimeObjectReference? objectReference))
{
return objectReference.AsValue();
}
return default;
}
// After
public static WindowsRuntimeObjectReferenceValue ConvertToUnmanaged(FontWeights value)
{
return value?.NativeObjectReference.AsValue() ?? default;
}
\\
Non-sealed runtime classes are not affected by this change, as they may be subclassed and the RCW may not always be directly unwrappable.