Uh oh!
There was an error while loading. Please reload this page.
Replace native AOT RuntimeCustomAttribute with CoreCLR's - #132999
Draft
MichalStrehovsky wants to merge 5 commits into
Draft
Replace native AOT RuntimeCustomAttribute with CoreCLR's#132999MichalStrehovsky wants to merge 5 commits into
MichalStrehovsky wants to merge 5 commits into
Conversation
Compile the CoreCLR-owned implementation in NativeAOT and keep the metadata-specific behavior behind NATIVEAOT conditionals. Remove the duplicated NativeAOT custom attribute data hierarchy and align the namespace with CoreCLR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 104a8c3f-92e7-4fe4-8e14-8cde7b18f20a
NativeAOT stores custom attribute constructors as qualified method definitions. Treat other handle kinds as malformed metadata instead of retaining dead MemberReference handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 104a8c3f-92e7-4fe4-8e14-8cde7b18f20a
Compile CoreCLR's pseudo custom attribute data construction for NativeAOT and use attribute instances at the NativeAOT call sites. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0005a0aa-9ff2-4b11-bed7-ea205ad136b1
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0005a0aa-9ff2-4b11-bed7-ea205ad136b1
|
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. |
Contributor
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Shares CoreCLR’s custom-attribute data implementation with NativeAOT and removes the duplicated NativeAOT hierarchy.
Changes:
- Adds NativeAOT support to
RuntimeCustomAttributeData. - Routes pseudo-attributes through the shared implementation.
- Removes obsolete NativeAOT implementations and imports.
Assessment: Generic attributes using MemberReference are broken, named arguments can bind to the wrong member kind, and attribute filtering now performs substantial eager work.
File summaries
| File | Description |
|---|---|
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs | Adds shared NativeAOT parsing and representation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeNamedTypeInfo.cs | Uses shared pseudo-attribute wrapper. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeGenericParameterTypeInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeGenericParameterTypeInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/NativeFormat/NativeFormatRuntimePropertyInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/RuntimeSyntheticParameterInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/RuntimeFatMethodParameterInfo.cs | Migrates parameter pseudo-attributes. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/ParameterInfos/NativeFormat/NativeFormatMethodParameterInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/Modules/NativeFormat/NativeFormatRuntimeModule.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeNamedMethodInfo.cs | Migrates PreserveSig representation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/NativeFormat/NativeFormatMethodCommon.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/Dispensers.NativeFormat.cs | Removes the old attribute factory. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/FieldInfos/RuntimeFieldInfo.cs | Migrates field pseudo-attributes. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/FieldInfos/NativeFormat/NativeFormatRuntimeFieldInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/EventInfos/RuntimeEventInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/EventInfos/NativeFormat/NativeFormatRuntimeEventInfo.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/CustomAttributes/RuntimePseudoCustomAttributeData.cs | Deletes the old pseudo-attribute implementation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/CustomAttributes/RuntimeCustomAttributeData.cs | Deletes the old common NativeAOT implementation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/CustomAttributes/NativeFormat/NativeFormatCustomAttributeData.cs | Deletes the old metadata-backed implementation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/Assemblies/NativeFormat/NativeFormatRuntimeAssembly.cs | Removes obsolete namespace import. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj | Compiles the shared file and removes deleted files. |
Review details
Suppressed comments (2)
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs:943
- The decoded field/property kind is only validated; it is never compared with
CustomAttributeNamedParameter.FieldOrProperty. Because fields are inserted before properties, a property named argument can bind to an inherited field with the same name and type, makingNamedArgumentsidentify—and attribute instantiation update—the wrong member. Include the member kind in the matching loop and cover field/property hiding with a regression test.
CustomAttributeEncoding namedArgFieldOrProperty = namedArgument.Flags switch
{
NamedArgumentMemberKind.Field => CustomAttributeEncoding.Field,
NamedArgumentMemberKind.Property => CustomAttributeEncoding.Property,
_ => CustomAttributeEncoding.Undefined,
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs:59
- This replaces the old handle-only, lazy NativeAOT representation with eager constructor resolution, parameter inspection, full field/property enumeration, descriptor allocation, and named-argument type resolution. NativeAOT's filtering path reads only
AttributeType(RuntimeCustomAttribute.NativeAot.cs:52-55), soIsDefinedand filteredGetCustomAttributesnow pay this cost for every candidate attribute. Preserve lazy initialization or provide representative measurements demonstrating that this regression is acceptable.
ReadOnlySpan<ParameterInfo> parameters = m_ctor.GetParametersAsSpan();
if (parameters.Length != 0)
{
m_ctorParams = new CustomAttributeCtorParameter[parameters.Length];
for (int i = 0; i < parameters.Length; i++)
m_ctorParams[i] = new CustomAttributeCtorParameter(new CustomAttributeType((RuntimeType)parameters[i].ParameterType));
- Files reviewed: 23/23 changed files
- Comments generated: 1
- Review effort level: Balanced
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Sep 1, 2026
Open
Also brings back the `ResolveAttributeConstructor` with the bugfix in the ParameterInfo/Type .Equals comparison. This reverts commit ea41324. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs:1017
- The NativeAOT metadata retains whether this named argument targets a field or a property, but the shared lookup below never compares that value with
namedParam.FieldOrProperty. If a derived attribute hides an inherited field with a property (or vice versa) of the same name and type, fields are listed first andCustomAttributeDatais bound to the wrongMemberInfo/IsField. Filter candidates by member kind before the existing type and name checks.
RuntimeType argumentType = (RuntimeType)namedArgument.Type.Resolve(reader, typeContext).ToType();
CustomAttributeType argType = new CustomAttributeType(argumentType);
string? argName = namedArgument.Name.GetString(reader);
- Files reviewed: 23/23 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+92
to
+95
| FieldInfo[] fields = m_ctor.DeclaringType!.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); | ||
| PropertyInfo[] properties = m_ctor.DeclaringType.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); | ||
| m_namedParams = new CustomAttributeNamedParameter[properties.Length + fields.Length]; |
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 freeto 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.
Work in progress, I think more cleanups and potentially sharing are possible.