Bucket failing Xml trimming test apps #102449
Conversation
| <NativeAotIncompatible>true</NativeAotIncompatible> | ||
| </TestConsoleAppSourceFiles> | ||
| <TestConsoleAppSourceFiles Include="XmlSerializer.Deserialize.cs"> | ||
| <NativeAotIncompatible>true</NativeAotIncompatible> |
There was a problem hiding this comment.
Should we list a reason for XmlSerializer.Deserialize.cs and XmlSerializer.Serialize.cs?
There was a problem hiding this comment.
I struggled with what to put there and still struggle. I don't understand the reason we have this test in the first place. If we add a new optimization to IL Link that will make this test break same as it's broken with native AOT, would we consider that "illegal" optimization? What is the purpose of trimming tests for something that is marked as trim unsafe?
(My idea always was that we write trimming test if something is annotated as trim safe, but there's a warning suppression in place.)
There was a problem hiding this comment.
I don't understand the reason we have this test in the first place.
It is for Xamarin / Blazor WASM style trimming. See #41389. Even though the API is marked as "unsafe" - we wanted to ensure it works if the developer does the work to preserve/mark their own types. (Sort of like how DiagnosticSource and EventSource work.) It is ensuring that we have our internals annotated correctly.
I struggled with what to put there and still struggle.
What's the current problem we run into on native AOT? If we annotated in the test more, would that help?
There was a problem hiding this comment.
What's the current problem we run into on native AOT? If we annotated in the test more, would that help?
There are two problems. I'm fixing one now in this PR, but the other problem requires #102482 (I put it into a separate PR so expect failures here for now).
There was a problem hiding this comment.
@eerhardt do you have a preference on how to proceed here? XmlSerializer doesn't seem to be actively maintained by anyone so nobody is looking at that PR and I would like to get this PR in so we can finally close the tracking issue.
| <NativeAotIncompatible>true</NativeAotIncompatible> | ||
| </TestConsoleAppSourceFiles> | ||
| <TestConsoleAppSourceFiles Include="XmlSerializer.Deserialize.SealerOpt.cs" | ||
| ExtraTrimmerArgs="--enable-opt sealer" /> |
There was a problem hiding this comment.
The ExtraTrimmerArgs is kind of odd in the context of native AOT. Should this test be disabled too?
There was a problem hiding this comment.
Sealing is an optimization that native AOT does automatically so it doesn't seem too irrelevant. Native AOT's version should not really be observable.
|
@eerhardt this should be ready for review now |
eerhardt
left a comment
eerhardt
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the fixes here!
2 participants
Resolves #101228.