Add annotations to System.Private.DataContractSerialization #73337
Conversation
| } | ||
|
|
||
| [RequiresUnreferencedCode(DataContract.SerializerTrimmerWarning)] | ||
| [RequiresDynamicCode(DataContract.SerializerAOTWarning)] |
There was a problem hiding this comment.
Is this one RDC even with the knowledge that the DataContractSerializer.Option == SerializationOption.ReflectionOnly codepath will always be taken when AOT compiling?
DataContractSerializer.Option basically controls whether we're going to take the ref.emit code paths.
There was a problem hiding this comment.
Both implementations are flagged by the analyzer as annotated with RDC, so the option seems to not affect the end result. I tried to go depper on the ReflectionOnly call and at several points it calls multiple methods annotated with RDC
vitek-karas
left a comment
vitek-karas
left a comment
There was a problem hiding this comment.
I think this is a good start.
It might be worth filing an issue that even asking for reflection-only (no ref.emit) doesn't make it AOT friendly. Maybe it's not possible, but currently we don't know why.
|
Thanks for the heads up on this guys. We've been working on a huge PR for DCR for months and this has just caused a slew of conflict resolutions for that PR in our area at the last minute before we were set to checkin. |
|
Actually, after looking at all the conflicts that need resolving, there is so much that has changed here, I don't want to try and figure out how to keep your annotations correct in the new code. I'm reverting this PR. |
|
@StephenMolloy can you add a reference to the big PR you mentioned to know when is safe to reenable the analyzer? |
|
Sure. #71752 Once I can merge the revert, I'll merge that PR and ping back here. |
|
#71752 has been merged. Thanks for understanding. |
ghost
locked as resolved and limited conversation to collaborators
4 participants
Enables AOT analyzer and adds RDC annotations