Skip to content

Reconcile AllowPrivate behavior for v3 #1899

Description

@AArnott

In v3, AOT formatter generation is on by default via roslyn source generators. These formatters are able to include non-public members in the data types. This is a very useful feature, but it opens up a few design questions that we should finalize on before stabilizing v3:

API breaking changes

Thus far, I believe v3 is API compatible with v2. This is very valuable because large applications that consume MessagePack (directly or indirectly) and frameworks such as ASP.NET Core that use MessagePack internally will have a much harder time upgrading to v3 if by doing so they break existing code that uses v2.

If we decide that certain APIs are no longer needed, I suggest we keep them working and attribute them with [Obsolete] to help everyone more readily move to v3.

StandardResolver vs. StandardResolverAllowPrivate

The default behavior ignores non-public members during (de)serialization. The *AllowPrivate variety of several resolvers add non-public member access, at the cost of doubling up on all our dynamic codegen.
When we also consider the AOT formatters, we have three entirely independent implementations of automatic formatter generation. That is a heavy load to carry when we think about ways to improve automatic formatters going forward. For example, #1851 to name just one requested improvement should be done so that it works across all automatic formatters.

Suppose we eliminate the public-only variety of Ref-Emit formatter generation so everything supports private members. That could be a good thing. We'd come down from 3 to 2 systems. But we'd probably still need a mechanism in the ref-emit system to ignore the non-public members so as to not break compatibility. Or maybe we just consider [Key] attributed members and ignore anything else. Today I believe we explicitly break when a member we consider "visible enough" lacks both of these attributes.

Contractless

From my observation, contractless support is not great. It's heuristic for figuring which members to serialize is not 100% accurate (probably an unachievable goal) and does not match other serialization libraries' support either.
Worse yet, I believe the AllowPrivate variety serializes both the properties and the fields that back them, which seems terrible.

While I don't want to change how Contractless works to avoid breaking changes, I wonder if some changes are appropriate. Or should be mark anything in this area obsolete and discourage its use?

Analyzers

The analyzers want users to attribute just their public members of [MessagePackObject] types. v3 analyzers will want them to attribute all fields and properties regardless of visibility. In v3 with AOT on by default, private members can be serialized. But if the user deliberately avoids the AOT generation and wants to use StandardResolver at runtime (why would they?) then attributing private members has no effect and the analyzer is having them fix non-issues.
But enhancing the analyzer to be configurable or auto-detect this situation seems like a lot of work and complexity that may not be worth it for a scenario that we don't want v3 users to do.

AOT stabilization period

With AOT on by default, more users than ever will be using the T4, TypeCollector and related code that was previously used mostly just for unity. It has historically had bugs that were not present in the dynamic IL generator system. Switching everyone to the AOT world is a bold move, and one I think we should expect an influx of bugs from.
I'd very much like us to succeed at this move. But we should expect this bug influx and prepare to respond quickly to it to keep faith and confidence in the library. In the past, I personally have been reluctant to address AOT-related bugs since unity wasn't a scenario I needed and I didn't understand all the code. I still don't understand all the code, but I understand much more of it than before.

Metadata

Metadata

Assignees

Labels

blockerBlocks the next release within its milestone

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions