Share CoreCLR RuntimeParameterInfo with NativeAOT - #134027
MichalStrehovsky merged 3 commits into
Conversation
Replace the NativeAOT parameter-info hierarchy with a sealed implementation sharing the existing CoreCLR source in place. Add native metadata handling and update method, property, and synthetic parameter construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b689a562-006b-4ec0-8607-f5a91479388e
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
🔵 Needs a closer look
It broadly changes NativeAOT reflection behavior, intentionally inherits a known bug, and relies on a separate unmerged regression-test PR.
Pull request overview
Shares CoreCLR’s RuntimeParameterInfo implementation with NativeAOT, reducing duplicated reflection code and aligning parameter behavior.
Changes:
- Adds NativeAOT metadata, default-value, and modified-type support to the shared implementation.
- Updates method, property, and synthetic parameter construction.
- Removes the former NativeAOT parameter hierarchy and helpers.
File summaries
| File | Description |
|---|---|
src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs |
Removes obsolete parameter namespace import. |
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs |
Adds shared NativeAOT paths. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.NativeAot.cs |
Implements NativeAOT-specific parameter metadata handling. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs |
Separates parameter and return-parameter creation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs |
Uses shared parameter objects for indexers. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/NativeFormat/NativeFormatRuntimePropertyInfo.cs |
Exposes index-parameter signatures. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/RuntimeThinMethodParameterInfo.cs |
Removes superseded thin implementation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/RuntimeSyntheticParameterInfo.cs |
Removes superseded synthetic implementation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/RuntimePropertyIndexParameterInfo.cs |
Removes superseded indexer wrapper. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/RuntimeParameterInfo.cs |
Removes former base hierarchy. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/RuntimeMethodParameterInfo.cs |
Removes former method-parameter base. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/RuntimeFatMethodParameterInfo.cs |
Removes former metadata-backed base. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/NativeFormat/NativeFormatMethodParameterInfo.cs |
Removes replaced native-format implementation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/VirtualRuntimeParameterInfoArray.cs |
Removes obsolete parameter-array helper. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/SyntheticMethodId.cs |
Removes obsolete import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeSyntheticMethodInfo.cs |
Constructs shared synthetic parameters. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeSyntheticConstructorInfo.cs |
Constructs shared constructor parameters. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimePlainConstructorInfo.cs |
Uses the shared parameter factory. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeNamedMethodInfo.cs |
Uses separate shared factories. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeMethodHelpers.cs |
Removes duplicated parameter construction. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeDummyMethodInfo.cs |
Implements revised abstract contract. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeConstructorInfo.cs |
Removes obsolete import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeConstructedGenericMethodInfo.cs |
Delegates revised parameter factories. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/OpenMethodInvoker.cs |
Removes obsolete import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/NativeFormat/NativeFormatMethodCommon.cs |
Exposes parameter metadata handles. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/IRuntimeMethodCommon.cs |
Revises the metadata contract. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/QSignatureTypeHandle.cs |
Supports modified types without metadata. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/Dispensers.NativeFormat.cs |
Removes obsolete factories. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/Dispensers.cs |
Removes obsolete parameter factories. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/EventInfos/RuntimeEventInfo.cs |
Removes obsolete import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/EventInfos/NativeFormat/NativeFormatRuntimeEventInfo.cs |
Removes obsolete import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/ModifiedType.NativeAot.cs |
Handles metadata-free modified types. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj |
Includes shared sources and removes deleted files. |
Review details
- Files reviewed: 33/33 changed files
- Comments generated: 0
- Review effort level: Balanced
Use file-local token and metadata-name adapters to share the original CoreCLR expressions without changing CoreCLR behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b689a562-006b-4ec0-8607-f5a91479388e
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
🔵 Needs a closer look
The broad reflection refactor changes observable NativeAOT behavior and intentionally inherits known issue #134022, warranting final maintainer validation.
Review details
- Files reviewed: 33/33 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
The failure in outerloop looks real, looks like RuntimeParameterInfo on CoreCLR doesn't override Equals/GetHashCode but native AOT did. We now run into the differences between how CoreCLR and native AOT cache MethodInfos. I expect us to unify that too at some point, but right now I guess we'll need to add Equals/GetHashCode to RuntimeParameterInfo on native AOT side at minimum. However this honestly looks like a CoreCLR bug. RuntimeParameterInfo should override these too. There's nothing that guarantees referential integrity even today - there is an existing race when creating these, and a hot reload cache flush can make us lose and recreate the associated MethodBase. There is a reason why MethodBase overloads operators ==/!= and RuntimeMethodInfo overrides Equals/GetHashCode. We probably also need a new API (the operators). |
Overloading operators ==/!= is a convenience. Nothing says that these operators have to be overloaded for every type that overrides Equals. In fact, most types that override Equals do not come with overloaded operator ==/!=. |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Minus 570 lines of product code.
Minus several bugs (failing tests are in #134025)
Plus a bug that now matches the bug in CoreCLR (#134022)
Replace the NativeAOT parameter-info hierarchy with a sealed implementation sharing the existing CoreCLR source in place. Add native metadata handling and update method, property, and synthetic parameter construction.