Deduplicate warnings for attribute on property dataflow#127254
Deduplicate warnings for attribute on property dataflow#127254eduardo-vp wants to merge 2 commits intodotnet:mainfrom
Conversation
dotnet-policy-service
Bot
added
the
linkable-framework
There was a problem hiding this comment.
Pull request overview
This PR fixes NativeAOT duplicate trim-analysis warnings emitted when analyzing custom attributes on properties/events (dataflow/capability warnings), aligning NativeAOT behavior with the analyzer/ILLink and addressing #120004.
Changes:
- Add a deduplication mechanism in the NativeAOT metadata manager to ensure property/event custom attributes are analyzed only once per metadata handle.
- Update NativeAOT linker/analyzer test cases by removing
UnexpectedWarningannotations that were tracking the duplicate-warning bug.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnAttribute.cs | Removes NativeAOT-only UnexpectedWarning entries now that warnings should not be duplicated. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs | Removes NativeAOT-only UnexpectedWarning entries for attribute-on-property/event dataflow scenarios. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs | Tracks processed property/event custom attribute handles to support deduplication. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs | Uses the new tracking to skip re-analyzing the same property/event custom attributes (and re-emitting the same diagnostics). |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| if (mdManager.TryRegisterPropertyCustomAttributesProcessed(method.Module, propertyHandle)) | ||
| AddDependenciesDueToCustomAttributes(ref dependencies, propertyCondition, factory, method.Module, property.GetCustomAttributes(), new PropertyPseudoDesc(method.OwningType, propertyHandle)); | ||
| } |
There was a problem hiding this comment.
We don't model things like this because when looking at the dependency graph, the first accessor gets to be blamed for bringing these dependencies into the graph whereas all accessors are to blame. It also runs the risk of being non-determinsitic in the presence of threading within the compiler.
We'd want to instead address the comment block above: "We don't model properties and events as separate entities within the compiler, so ensuring we can generate custom attributes for the associated events and properties from here is as good as any other place.". Instead of each accessor bringing in dependencies for custom attributes on the property definition, each accessor should just bring a dependency on the property.
We'd only scan the property once (because it's just one node in the graph) so the rest will fall out and we can still attribute each dependency in the graph to what really depends on it.
So add a PropertyMetadataNode (will look similar to the existing FieldMetadataNode). Remove the call to AddDependenciesDueToCustomAttributes from here (instead we'll call it in the new PropertyMetadataNode.GetConditionalStaticDependencies, same as FieldMetadataNode does). We also remove property/method handling from here completely and instead do this in MethodMetadataNode.GetStaticDependencies (check if the method is an accessor and if it is, add a dependencies.Add(factory.PropertyMetadata(new PropertyPseudoDesc(method.OwningType, propertyHandle)) into the graph).
Avoid processing the custom attributes of properties/events more than once by using a
HashSetof(EcmaModule, DefinitionHandle).Fixes #120004.