Box value types implementing IXmlSerializable in XmlSerializer generated IL - #117473
Box value types implementing IXmlSerializable in XmlSerializer generated IL#117473StephenMolloy merged 1 commit into
IXmlSerializable in XmlSerializer generated IL#117473Conversation
|
@dotnet-policy-service agree |
There was a problem hiding this comment.
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 |
|
@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 [#99613](#99613)
There was a problem hiding this comment.
LGTM.
Just for clarity, this is a scenario which did not work in .Net 4.8. But I think it's one that should. So here we go.
Also another fun fact - this already works with the reflection-based serializer in .Net Core.
(Finally, it's taken me quite a while to get back to serialization. So if the Expected.SerializableAssembly.XmlSerializers.cs is out of date, I'll create a separate PR to fix that up.)
Value types implementing the
IXmlSerializableinterface were not being property boxed when they declared a parameterless constructor. This caused theXmlSerializerto fail to deserialize them with anInvalidProgramExceptionexception when the generated IL tried to call theXmlSerializationReader.ReadSerializablemethod 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.Generatortool for a different types and simplified it down to determine what the IL should look like for the call to theXmlSerializationReader.ReadSerializablemethod.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.CannotNewcondition above the new condition I've added, and avoid the style of instantiation that's demonstrated in my simplified example.