.NET 8 zero overhead private member mapping

[TimothyMakkison]

Previously, Mapperly hasn't had the option to access or set private members. Although private/hidden members have always been visible to source generators, there hasn't been a suitable way of accessing them. While workarounds such as Reflection.Emit, Linq.Expressions and reflection are available, upon closer inspection, they are insufficient for our needs due to poor performance and incompatibilities in AOT usage. For these reasons, Mapperly has never added private member mapping.

With the release of .NET 8, the UnsafeAccessorAttribute will be added. This supports code that accesses internal methods, constructors, fields, and properties with zero overhead while being AOT compatible. By applying it to an extern static method and configuring it, the runtime will attempt to find the corresponding field or method, to which the call will be forwarded.

Using the new attribute, Mapperly could add support for private member mapping, init-only/private setter mapping, and support private constructors.

Private member mapping

Enabling

Private member mapping could be enabled by default, although this would likely lead to nasty unexpected behaviour while breaking backwards compatibility. Instead I suggest that a property EnablePrivateMapping be added to [Mapper] and a method attribute [EnablePrivateMapping] be added. Alternatively private member mapping could always be enabled but only for explicit MapProperty mappings.

Generated code

The private access methods can be added as additional methods to the mapping class. These could be implemented like a MethodMapping and used like so: SetId(target, GetId(source)).
Alternatively, to make the code easier to read, the private accessors could be added to a file scoped static accessor class. This class way the methods could be implemented as extensions methods, with the resulting code reading left to right. target.SetId(source.GetId()).

target.Tires.SetSpareWheel(source.Tires.GetSpareWheel()) vs SetSpareWheel(target.Tires, GetSpareWheel(source.Tires))

Example

Mapper

public class Car
{
    private Tire _spareWheel { get; set; }
    // ...
}

public class CarDto
{
    private TireDto _spareWheel { get; set; }
    // ...
}

[Mapper(EnumMappingStrategy = EnumMappingStrategy.ByName)]
public static partial class CarMapper
{
    [MapProperty("_spareWheel", "_spareWheel")]
    [MapProperty(nameof(Car.Manufacturer), nameof(CarDto.Producer))] // Map property with a different name in the target type
    public static partial CarDto MapCarToDto(Car car);
}

Generated Code

static file class 
{
    [UnsafeAccessor(UnsafeAccessorKind.Method, Name="set__spareWheel")]
    public static extern Tire GetSpareWheel(this Car source)

    [UnsafeAccessor(UnsafeAccessorKind.Method, Name="set__spareWheel")]
    public static extern void SetSpareWheel(this CarDto target, TireDto tireDto)
}

public static partial class CarMapper
{
    public static partial CarDto MapCarToDto(Car car)
    {
        target.SetSpareWheel(MapToTireDto(car.GetSpareWheel()));
        // ...
    }
}

Related #599, #531

[latonz]

Relates #529

I like the idea 😊 Before this can be implemented we need to add support for Roslyn 4.7.
Since #597 Mapperly maps all fields and properties the mapper has access to. This can include private members.

Instead of [EnablePrivateMapping] i like a flags idea more:

public sealed class MapperAttribute
{
+    public MemberVisibility IncludedMembers { get; set; } = MemberVisibility.AllAccessible;
}

+[Flags]
+public enum MemberVisibility
+{
+    /// <summary>
+    /// Maps all accessible members.
+    /// </summary>
+    AllAccessible = All | Accessible,

+    /// <summary>
+    /// Maps all members.
+    /// </summary>
+    All = Public | Internal | Protected | Private,

+    /// <summary>
+    /// Maps only accessible members.
+    /// If not set, accessors for not directly accessible members using <see cref="UnsafeAccessorAttribute" /> are generated.
+    /// </summary>
+    Accessible = 1 << 0,

+    Public = 1 << 1,
+    Internal = 1 << 2,
+    Protected = 1 << 3,
+    Private = 1 << 4,
+}

Accessible should replicate the current behaviour (mainly introduced by #597) which maps all members accessible by the mapper.

For the accessors, to keep it simple, I'd reuse the same class and just append them at the end of the class.

A diagnostic should emit if UnsafeAccessorAttribute is not supported but the Accessible flag is not set (eg. .NET < 8.0).

[TimothyMakkison]

Since #597 Mapperly maps all fields and properties the mapper has access to. This can include private members.

Yeah that makes this much easier 😅

For the accessors, to keep it simple, I'd reuse the same class and just append them at the end of the class.

I might try the separate static UnsafeAccessor class first. I think that extension methods will be more idiomatic and easier to read.

A diagnostic should emit if UnsafeAccessorAttribute is not supported but the Accessible flag is not set (eg. .NET < 8.0).

👍

public MemberVisibility IncludedMembers { get; set; } = MemberVisibility.AllAccessible;

Do you think that private/internal mapping should be enabled by default? Wouldn't this be a massive breaking change?

[latonz]

I would enable private/internal fields by default, but also set the Accessible flag by default. This by default only maps members which are accessible by the mapper which I think is what most people expect. #597 will introduce exactly this behaviour (therefore this won't be breaking, but #597 is).

#597 maps all members which are accessible by the mapper instead of only public and internal members. This is a breaking change for members in assemblies with the InternalsVisibleToAttribute set for the assembly of the mapper and it is a breaking change for private members if the mapper has access to them (eg. the mapper class itself is mapped or the mapper is contained in another class with private members). At the time of implementing #597 I thought #354 was only implemented in 2.9.0-next.* and therefore mapping private members won't touch anyone with 2.8.0, but I just realised that #354 was already released in 2.8.0. I still think it wont touch many users, but I'm unsure whether we should release #597 as breaking change 🤔

[TimothyMakkison]

I would enable private/internal fields by default, but also set the Accessible flag by default. This by default only maps members which are accessible by the mapper which I think is what most people expect. #597 will introduce exactly this behaviour (therefore this won't be breaking, but #597 is).

I think I misunderstood you earlier. When you say private do you mean map any private members or just private members that are in the scope of the mapper (aka nested mappers, I'd assume this would fall under Accessible)? i.e.

[Mapper]
public statiic partial Mapper
{
    // would _value be mapped by default?
    public static partial B Map(A src);
}

public class A { private int _value { get; set; } }
public class B { private int _value { get; set; } }

#597 maps all members which are accessible by the mapper instead of only public and internal members. This is a breaking change for members in assemblies with the InternalsVisibleToAttribute set for the assembly of the mapper and it is a breaking change for private members if the mapper has access to them (eg. the mapper class itself is mapped or the mapper is contained in another class with private members).

Do other mapping libraries do this? I can't remember if reflection can use InternalsVisbleTo. Would it be worth introducing the above enum/config so users can control this.

At the time of implementing #597 I thought #354 was only implemented in 2.9.0-next.* and therefore mapping private members won't touch anyone with 2.8.0, but I just realised that #354 was already released in 2.8.0. I still think it wont touch many users, but I'm unsure whether we should release #597 as breaking change 🤔

I don't think its a massive problem. tbh I was suprised at how many people were using nested classes and wanted to map private properties. I have no idea what they are using it for

[latonz]

@TimothyMakkison the idea of the Accessible flag is to control whether UnsafeAccessorAttribute s are used or only members which are accessible by the mapper are considered.

I don't think InternalsVisibleTo affects a lot of people and there is an easy workaround with ignore attributes.
IMO the enum would help for sure... We could introduce it and just report a diagnostic if the Accessible flag is not set until Mapperly supports the new Roslyn version and UnsafeAccessorAttribute.

We'll release #597 as breaking change as it is actually a breaking change and we want to conform to the semantic release specification. I prepared #611 and #612 for this. The upgrade procedere for most of the users should be just as simple as upgrading to a new feature release.

[latonz]

Blocked by #682

[latonz]

@TimothyMakkison Have you already started working on this or do you have the desire to implement it? Otherwise I'd probably start working on it in the next few weeks 😊.

[TimothyMakkison]

Yeah I prototyped it a month ago. I have a working prototype but it needs revasing.

Might need some help coming up with a nice abstraction to the changes I made to MemberPath, it now needs to support normal assignments and invoking a setter method.

I can share the original if you want

[TimothyMakkison]

Oh yeah, should Mapperly let individual mapping methods set its visibility mapping? Or should it be configured in the MapperAttribute

[latonz]

A quick thought: Couldn't this be abstracted by introducing a new MethodAccessorMember which implements IMappableMember and simply delegates almost everything to a delegate field which is again an IMappableMember (the actual member). Then the MemberPath.BuildAccess could be split into BuildSetValue and BuildGetValue (for the whole path) and a similar method added to IMappableMember (for each individual member). Each time a MethodAccessorMember is instantiated, an accessor to be generated is added to the descriptor. In `SourceEmitter.Build' these accessors could then be converted to members similar to the mappings themselves.
I'd probably have to experiment a bit too, to get a good solution.

I think this might be helpful on method level, but IMO in a first version it is ok only on mapper level.

[latonz]

FYI in #718 support for Roslyn 4.7 is prepared.

[TimothyMakkison]

Iirc I added MemberPathSetter and MemberPathGetter and updated all users of MemberPath to use them. I had a reason why I didn't add BuildSetValue or BuildGetValue but I can't remember why 😆 (might be something to do with equality 🤷).
I think I kept MemberPath as a placeholder which, after validation, could be built into the Getter/Setter version using UnsafeAccessor to generate or resue methods.

Couldn't this be abstracted by introducing a new MethodAccessorMember which implements IMappableMember and simply delegates almost everything to a delegate field which is again an IMappableMember

Maybe I'll try it later

In `SourceEmitter.Build' these accessors could then be converted to members similar to the mappings themselves.

I did try the extension version because IMO it was much easier to read. I do have concerns about the extension method overlapping with the types methods.

[github-actions]

🎉 This issue has been resolved in version 3.3.0-next.2 🎉

The release is available on:

• GitHub release
• v3.3.0-next.2

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 3.3.0 🎉

The release is available on:

• GitHub release
• v3.3.0

Your semantic-release bot 📦🚀