-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
XmlSerializer.GenerateSerializer should not be in System.Private.Xml #1413
Comments
I maintain code that uses System.Xml.Serialization.XmlSerializer.GenerateSerializer (directly, not using reflection) for a case sgen is unable to handle. Could completely ripping this function out break f.ex. the Microsoft.XmlSerializer.Generator nuget package? |
@mconnew Please comment on this issue and resolve as appropriate. |
@tamlin-mike - can you explain this more? Are you only doing that on the .NET Framework? This method is runtime/src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializer.cs Line 596 in 4f9ae42
|
Yeah, that turned out to be the problem. It was public in Framework. Unfortunately the code started to depend on that functionality. Then the need arose to use some other assembly compiled to netstandard 2.0 IIRC, and much gnashing of teeth ensued. Ended up throwing the MS tool out and writing our own serialization generator (-frontend) to get the required cross-platform compatibility we needed (fw + netstd). Unfortunately that now depends on that older specific version of the mentioned nuget package -- that will likely be removed sooner or later, or become incompatible with later versions of some runtime -- so probability is high we'll either be forced to rewrite it all from scratch or simply burn that use of XML at the stake. To quote Alien "Nuke it from space. Only way to be sure". |
When I spoke with @mconnew it seemed that this issue would take a lot of work, and is pretty low on the priority list. With #35547, we have addressed the size concern with having to root |
Also for ARM use D0-D31 registers for encoding S0-S31
#56589 seems to be a duplicate of this issue. Microsoft.XmlSerializer.Generator has I think the C# code generation should be moved into the Microsoft.XmlSerializer.Generator package. The XmlSerializer.GenerateSerializer method would then be called only by older versions of Microsoft.XmlSerializer.Generator, and its implementation should be reverted to a maximally compatible version, perhaps to how it was in .NET Core 3.1.0. Each project that wants Microsoft.XmlSerializer.Generator to generate code that uses newer features of C# or .NET Runtime, such as Span<T>, would have to upgrade Microsoft.XmlSerializer.Generator. |
System.Private.Xml contains all of the code for generating an assembly as the core logic behind the sgen tool:
https://github.com/dotnet/corefx/tree/master/src/Microsoft.XmlSerializer.Generator
The vast majority of the functionality here isn't used by anything other than sgen, which calls XmlSerializer.GenerateSerializer via reflection, and is the only caller of that method. This functionality should be removed from XmlSerializer in System.Private.Xml and moved into sgen.
The text was updated successfully, but these errors were encountered: