Skip to content
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

Address issues discovered after fixing AOT warnings #1511

Merged
merged 4 commits into from
Feb 22, 2024

Conversation

manodasanW
Copy link
Member

This PR addresses the issues that came up after AOT checks were added to all our MakeGenericType calls. The main issue that came up is the helper type for generic types can no longer be constructed. This was addressed in two ways.

  • For the scenarios that need it due to the MarshalInterface calls, the helper type is registered before it gets there via the Methods static classes in their static constructor which gets triggered as part of the generic type instantiations.
  • The other scenario that needed it is IID creation where it found the helper type as part of determining the IID. Given the IID we are getting from the helper type in generic type scenarios is for the unbound type, that code path has been changed to get the helper type for the unbound type instead which avoids the MakeGenericType calls. The generic itself is still used as part of the IID generation but is done separately as it was before too as part of combining the signature that gets hashed.
  • Array marshaling was also disabled so disabling the test for AOT for now until we figure out how much of an impact it is and if we need another way to handle it.

Copy link
Member

@Sergio0694 Sergio0694 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a first review pass, looks good so far! Couple questions 😄

src/WinRT.Runtime/ComWrappersSupport.cs Show resolved Hide resolved
src/WinRT.Runtime/GuidGenerator.cs Show resolved Hide resolved
src/cswinrt/code_writers.h Show resolved Hide resolved
Copy link
Member

@Sergio0694 Sergio0694 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! :shipit:

@manodasanW manodasanW merged commit d310fe1 into user/sergiopedri/guard-mgt-calls Feb 22, 2024
9 checks passed
@manodasanW manodasanW deleted the manodasanw/aotwarningfix branch February 22, 2024 21:27
Sergio0694 pushed a commit that referenced this pull request Feb 22, 2024
* Fix build breaks from AOT warning fixes

* Improvements to GUID helper type lookup to avoid needing instantiated generic type

* Move around where helper type is initialized to handle more scenarios

* Remove commented code
manodasanW added a commit that referenced this pull request Feb 29, 2024
* Fix build breaks from AOT warning fixes

* Improvements to GUID helper type lookup to avoid needing instantiated generic type

* Move around where helper type is initialized to handle more scenarios

* Remove commented code
Sergio0694 pushed a commit that referenced this pull request Feb 29, 2024
* Fix build breaks from AOT warning fixes

* Improvements to GUID helper type lookup to avoid needing instantiated generic type

* Move around where helper type is initialized to handle more scenarios

* Remove commented code
Sergio0694 pushed a commit that referenced this pull request Mar 12, 2024
* Fix build breaks from AOT warning fixes

* Improvements to GUID helper type lookup to avoid needing instantiated generic type

* Move around where helper type is initialized to handle more scenarios

* Remove commented code
Sergio0694 added a commit that referenced this pull request Mar 13, 2024
* Guard MGT(Type) calls on NativeAOT

* Set IsAotCompatible, treat AOT warnings as errors

* Add throw paths on AOT for MarshalNonBlittable<T>

* Make ManagedIPropertyValueImpl.UnboxValue AOT-safe

* Fix all AOT warnings in WinRT.Runtime

* Centralize [RDC] messages in TrimmingAttributeMessages

* Fail build for AOT warnings in authoring test project

* Replace Marshal.SizeOf<T>() with sizeof(T) on .NET 6+

* Address issues discovered after fixing AOT warnings (#1511)

* Fix build breaks from AOT warning fixes

* Improvements to GUID helper type lookup to avoid needing instantiated generic type

* Move around where helper type is initialized to handle more scenarios

* Remove commented code

* Fix IL2073 warning in 'FindHelperType'

* Centralize attribute constants in 'AttributeMessages'

* Fix nullable and IPropertySet scenarios (#1519)

* Experiment with changes to avoid nullable.value going down the helper type route

* Fix build

* Fix nullable structs and delegates

* Fix

* Fix tests

* Fix issue where the class implementing the interface can be trimmed and we rely on IDIC cast to create an instance of the interface.  But that doesn't work with the interface inherits generic interfaces on AOT.  This addresses that by making sure there is a fallback class available to use for the interface.

* Fix warning

* Improvements to nullable scenario (caching and removing old code paths)

* Move where we do generic type initialization for derived generic interfaces impl class

* Fix issue with Interface<> missing in impl classes by converting most of the remaining ones using it to the new format

* Add test

* PR feedback

* PR feedback for IID

---------

Co-authored-by: Manodasan Wignarajah <mawign@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants