Don't emit analyzer warnings for MakeGeneric understood by ILC#100858
Don't emit analyzer warnings for MakeGeneric understood by ILC#100858MichalStrehovsky merged 2 commits intodotnet:mainfrom
Conversation
Follow up to dotnet#99037. Resolves dotnet#81204. When `MakeGenericXXX` API is used in the "bridging constraints" pattern and all types/members involved are statically known, don't emit warning from the Roslyn analyzer since ILC will do the right thing and ensure this works.
ghost
added
the
area-Tools-ILLink
dotnet-policy-service
bot
added
the
linkable-framework
sbomer
left a comment
sbomer
left a comment
There was a problem hiding this comment.
Did you think about sharing some of these tests between ILC and the analyzer?
| if (!instance.IsEmpty ()) { | ||
| foreach (var value in instance.AsEnumerable ()) { | ||
| if (value is SystemTypeValue) { | ||
| if (!IsKnownInstantiation (arguments[0])) { |
There was a problem hiding this comment.
Should this also check that the instantiation length matches the method?
There was a problem hiding this comment.
If the instantiation length doesn't match, we'd get an exception at runtime both before and after AOT compilation, so maybe we don't need an AOT warning. The code is wrong, but we don't concern ourselves with bugs that exist before and after publish.
There was a problem hiding this comment.
So should this return true if it's not a known instantiation, but the length doesn't match? I think that's what ILC does if I read #99037 correctly.
There was a problem hiding this comment.
Oh never mind, it triggers the warning if TryGetMakeGenericInstantiation returns false. Probably not an important edge case anyway.
Do we have a good spot for that? For the AOT testing, there's more concern about whether it actually works at runtime, so I wrote tests that actually execute. We don't typically execute the code in illink/analyzer testing. |
|
https://github.com/dotnet/runtime/blob/main/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MakeGenericDataFlow.cs might be a good place, except this suppresses IL3050 currently. You could fix up the ExpectedWarnings for AOT or add a new file next to that one. Just a suggestion, I think the current approach is OK too. |
…t#100858) Follow up to dotnet#99037. Resolves dotnet#81204. When `MakeGenericXXX` API is used in the "bridging constraints" pattern and all types/members involved are statically known, don't emit warning from the Roslyn analyzer since ILC will do the right thing and ensure this works.
2 participants
Follow up to #99037.
Resolves #81204.
When
MakeGenericXXXAPI is used in the "bridging constraints" pattern and all types/members involved are statically known, don't emit warning from the Roslyn analyzer since ILC will do the right thing and ensure this works.Cc @dotnet/ilc-contrib