-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Box value types implementing IXmlSerializable
in XmlSerializer
generated IL
#117473
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
base: main
Are you sure you want to change the base?
Box value types implementing IXmlSerializable
in XmlSerializer
generated IL
#117473
Conversation
b03bd96
to
9ab4028
Compare
9ab4028
to
33799e7
Compare
@dotnet-policy-service agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a missing boxing step for value types implementing IXmlSerializable
during IL generation in XmlSerializationReader
, preventing runtime errors when deserializing such structs. It also adds test types and unit tests to verify correct behavior for structs with and without parameterless constructors.
- Added a new branch in the IL generator to convert (box) value types to
IXmlSerializable
. - Introduced two struct types in tests to exercise both cases (with and without parameterless constructor).
- Added corresponding unit tests to validate serialization and deserialization of those structs.
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.cs | Added two IXmlSerializable structs (with and without parameterless ctor) |
src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs | Added tests for serializing/deserializing the new structs |
src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializationReaderILGen.cs | Updated IL gen to box value types implementing IXmlSerializable |
33799e7
to
2187f92
Compare
@filipnavara Thanks for the explicit label! I also notice that there's an entry called
area-Serialization
which also includes |
area-Serialization recently changed ownership, but it can be more appropriate. cc @StephenMolloy |
…nerated IL Value types implementing the `IXmlSerializable` interface were not being property boxed when they declared a parameterless constructor. This caused the `XmlSerializer` to fail to deserialize them with an `InvalidProgramException` exception when the generated IL tried to call the `XmlSerializationReader.ReadSerializable` method with the value. Fixes dotnet#99613
2187f92
to
301991c
Compare
Value types implementing the
IXmlSerializable
interface were not being property boxed when they declared a parameterless constructor. This caused theXmlSerializer
to fail to deserialize them with anInvalidProgramException
exception when the generated IL tried to call theXmlSerializationReader.ReadSerializable
method with the value.Fixes #99613
I would like to note that I am not really that familiar with IL generation, so all corrections are welcome☺️ .
I took the C# output of the serializer produced by the
Microsoft.XmlSerializer.Generator
tool for a different types and simplified it down to determine what the IL should look like for the call to theXmlSerializationReader.ReadSerializable
method.My simplified snippet was the following:
The following is the resulting IL for each method:
No boxing was being written by the IL generator for the value types with parameterless constructors when the XML serializer was generating the runtime assembly, so a simple addition of that boxing seems to resolve the issue.
Value types without parameterless constructors do not run into this issue because they fall into the
sm.TypeDesc.CannotNew
condition above the new condition I've added, and avoid the style of instantiation that's demonstrated in my simplified example.