Skip to content

Skip setting fields when the ctor already took the value#1314

Merged
AArnott merged 3 commits intoMessagePack-CSharp:masterfrom
AArnott:fix1308
Sep 2, 2021
Merged

Skip setting fields when the ctor already took the value#1314
AArnott merged 3 commits intoMessagePack-CSharp:masterfrom
AArnott:fix1308

Conversation

@AArnott
Copy link
Copy Markdown
Collaborator

@AArnott AArnott commented Aug 21, 2021

This re-enables setting readonly fields (which used to work till #1218) was fixed.

To avoid regressing the test that the fix for #1218 added, I also suppress setting fields with values that have already been passed to constructors.

This is a behaviorally observable change.

Fixes #1308

MPC comparison

The new behavior is consistent with mpc-generated code. For this type:

https://github.com/neuecc/MessagePack-CSharp/blob/14556ae3bb41a8807b5a93c9d6a7793bd5d9ab3e/src/MessagePack.UnityClient/Assets/Scripts/Tests/Class1.cs#L679-L693

mpc generates code that only invokes the constructor and returns the result:

https://github.com/neuecc/MessagePack-CSharp/blob/14556ae3bb41a8807b5a93c9d6a7793bd5d9ab3e/sandbox/Sandbox/Generated.cs#L3455-L3457

@AArnott AArnott added this to the v2.3 milestone Aug 21, 2021
@AArnott AArnott requested a review from neuecc August 21, 2021 23:48
@AArnott AArnott self-assigned this Aug 21, 2021
Copy link
Copy Markdown
Member

@neuecc neuecc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Readonly Dictionary with DataMember.Order is not being deserialized

2 participants