Refactor code and annotations to fix all warnings in NativeAOTCompatibility. #1207
Conversation
…bility. There are a couple patterns here: - Use GetMemberWithSameMetadataDefinitionAs to reflect on generic types/methods in a statically verifiable fashion. This method only exists in .NET 6+. - Suppress warnings when using GetInterfaces when the Type is annotated as preserve 'All'. dotnet/linker#1731 - When looking for a hard-coded interface like IProgress<T>, any preserved Type that implements the interface won't have the interface trimmed. - MethodNameMap uses GetInterfaceMap, which may not work in all cases. See dotnet/runtime#89157
| #pragma warning disable IL3050 // Calling members annotated with 'RequiresDynamicCodeAttribute' may break functionality when AOT compiling. | ||
| [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Using the Json source generator.")] | ||
| [UnconditionalSuppressMessage("AOT", "IL3050", Justification = "Using the Json source generator.")] | ||
| IJsonRpcMessageFormatter CreateFormatter() => new SystemTextJsonFormatter() |
There was a problem hiding this comment.
Any way we could solve this in JsonSerializer? I get scared about suppressions of library code as it relies on both trimming details and the precise details of the library. Those things can easily drift over time.
There was a problem hiding this comment.
I think in order to do this "right" we will need to make some form of SystemTextJsonFormatter (either a whole new class, or a new "trim/aot safe" constructor) that is guaranteed to work consistently if you use the source generator. As it stands, the existing SystemTextJsonFormatter class has methods that will generate IL on the fly, and do other incompatible behaviors.
At least for now the SystemTextJsonFormatter is marked as RUC and RDC, so callers get warnings and can learn about what isn't compatible.
src/StreamJsonRpc/Reflection/MessageFormatterProgressTracker.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Looks reasonable, I share Andy's concerns in general. I think this is not the first place where we had to suppress warnings about accessing interfaces lists and looking for a generic instantiation of a type we know exists. Maybe another new API candidate so that it can be done without suppressions.
…te on the Type parameter.
- Use for loop to avoid Roslyn analyzer warning. - Condition the suppression on !NET10.
|
Thanks for the feedback. I believe I've addressed it all, if you want to take a relook. |
4 participants
There are a couple patterns here:
cc @AArnott
FYI - @agocke @MichalStrehovsky - would you mind checking the suppressions I needed to add here? The biggest one is the GetInterfaceMap issue.