Handle nullable-unwrapped generic parameter values#127305
Handle nullable-unwrapped generic parameter values#127305MichalStrehovsky wants to merge 3 commits intodotnet:mainfrom
Conversation
Introduce NullableUnwrappedGenericParameterValue to represent a typeof(T) that was unwrapped via Nullable.GetUnderlyingType.
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
|
After many iterations on this with several models, the only "bug" this has is that: Previously didn't generate a warning, now it does generate warning, even though the codepath is actually trim safe (we know GetUnderlyingType will return null). I've decided not to fix this "regression". |
There was a problem hiding this comment.
Pull request overview
This PR improves trim/AOT dataflow modeling for the Nullable.GetUnderlyingType(typeof(T)) pattern by introducing a dedicated value kind to represent “nullable-unwrapped generic parameter” results, preventing incorrect generic instantiation assumptions and enabling the appropriate NativeAOT warning behavior.
Changes:
- Add
NullableUnwrappedGenericParameterValueto representtypeof(T)afterNullable.GetUnderlyingTypeunwrapping in the trim analysis value lattice. - Update trim-analysis logic to recognize constraint-based satisfactions (e.g.,
new()/Enum) through the new wrapper and to unwrap it for diagnostic formatting. - Add/linker tests covering the
MakeGenericType(Nullable.GetUnderlyingType(typeof(T)))NativeAOT warning corner case and constraint propagation scenarios.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/NullableAnnotations.cs | Adds new constraint-focused test cases for Nullable.GetUnderlyingType(typeof(T)) flows. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MakeGenericDataflowIntrinsics.cs | Adds a regression test expecting NativeAOT-specific IL3050 for the nullable unwrapping + MakeGenericType corner case. |
| src/tools/illink/src/ILLink.Shared/TrimAnalysis/RequireDynamicallyAccessedMembersAction.cs | Teaches DAM-requirement validation to treat the new wrapper like its underlying GenericParameterValue for constraint-based allowances. |
| src/tools/illink/src/ILLink.Shared/TrimAnalysis/NullableUnwrappedGenericParameterValue.cs | Introduces the new value type representing a generic parameter after nullable unwrapping. |
| src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs | Produces NullableUnwrappedGenericParameterValue for Nullable.GetUnderlyingType when the input is a GenericParameterValue. |
| src/tools/illink/src/ILLink.Shared/Annotations.cs | Unwraps the new wrapper when generating annotation-mismatch diagnostics (argument formatting). |
| src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/DiagnosticContext.cs | Unwraps the new wrapper when mapping diagnostics back to Roslyn symbols/locations. |
sbomer
left a comment
There was a problem hiding this comment.
Could you add the "regression" cases as tests too? Thank you!
Introduce NullableUnwrappedGenericParameterValue to represent a typeof(T) that was unwrapped via Nullable.GetUnderlyingType.
This fixes an issue where
typeof(Gen<>).MakeGenericType(Nullable.GetUnderlyingType(typeof(T)))would make ILC precompileGen<T>, not produce a warning, and then fail at runtime because we actually needGen<Nullable<T>>.Cc @dotnet/illink