UnsafeAccessor and structs with no parameterless constructor

[MichalPetryka]

UnsafeAccessor (from #81741) seems to currently throw MissingMethodException when trying to construct a struct with no paramters when it doesn't contain a parameterless constructor.
Since you can do so fine with new() in C# and with Activator.CreateInstance, this got me wondering if this behaviour is intended, or if it should work fine.

cc @jkotas

Code:

public static class Program
{
    public static void Main()
    {
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S1 Constructor1();
        
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S1 Constructor2(int i);
		
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S2 Constructor3();
        
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S2 Constructor4(int i);
        
        Console.WriteLine(Constructor2(5).i);
        Console.WriteLine(Constructor1().i);
        Console.WriteLine(Constructor4(5).i);
        Console.WriteLine(Constructor3().i);
    }
}

public readonly struct S1
{
    public readonly int i;
    public S1() => this.i = 0;
    public S1(int i) => this.i = i;
}

public readonly struct S2
{
    public readonly int i;
    public S2(int i) => this.i = i;
}

Output:

5
0
5
Unhandled exception. System.MissingMethodException: Method not found: 'S2..ctor'.
   at Program.<Main>g__Constructor3|0_2()
   at Program.Main() in L:\fdg\dotnet-sdk-8.0.100-rc.1.23404.1-win-x64\Test\Program.cs:line 22

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices
See info in area-owners.md if you want to be subscribed.

Issue Details

---

UnsafeAccessor seems to currently throw MissingMethodException when trying to construct a struct with no paramters when it doesn't contain a parameterless constructor.
Since you can do so fine with new() in C# and with Activator.CreateInstance, this got me wondering if this behaviour is intended, or if it should work fine.

cc @jkotas

Code:

public static class Program
{
    public static void Main()
    {
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S1 Constructor1();
        
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S1 Constructor2(int i);
		
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S2 Constructor3();
        
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S2 Constructor4(int i);
        
        Console.WriteLine(Constructor2(5).i);
        Console.WriteLine(Constructor1().i);
        Console.WriteLine(Constructor4(5).i);
        Console.WriteLine(Constructor3().i);
    }
}

public readonly struct S1
{
    public readonly int i;
    public S1() => this.i = 0;
    public S1(int i) => this.i = i;
}

public readonly struct S2
{
    public readonly int i;
    public S2(int i) => this.i = i;
}

Output:

5
0
5
Unhandled exception. System.MissingMethodException: Method not found: 'S2..ctor'.
   at Program.<Main>g__Constructor3|0_2()
   at Program.Main() in L:\fdg\dotnet-sdk-8.0.100-rc.1.23404.1-win-x64\Test\Program.cs:line 22

Author:	MichalPetryka
Assignees:	-
Labels:	area-System.Runtime.CompilerServices, untriaged
Milestone:	-

[MichalPetryka]

Worth noting that UnsafeAccessorKind.Constructor already isn't equivalent to newobj since it works with arrays:

public static class Program
{
    public static unsafe void Main()
    {
        [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
        extern static S[] Constructor(int i);
        
        Console.WriteLine(Constructor(5).Length);
    }
}

public readonly struct S
{
    public readonly int i;
    public S(int i) => this.i = i;
}

despite the ECMA saying All zero-based, one-dimensional arrays are created using newarr, not newobj.

[jkotas]

this got me wondering if this behaviour is intended, or if it should work fine.

This looks like intended behavior to me. This feature does not do any transformations that C# does with a method call (overload resolution, look for the method in the type hierarchy, substitute default arguments, decide whether it is call or callvirt, ....). It makes sense that it does not replace a call with initobj either.

despite the ECMA saying All zero-based, one-dimensional arrays are created using newarr, not newobj.

newobj should work fine for zero-based, one-dimensional arrays. The behavior of UnsafeAccessor matches behavior of newobj IL instruction for this case.

It is wasteful to create zero-based one-dimensional arrays using newobj instruction. On the other hand, all arrays have runtime provided constructors that can be used with newobj instruction. It does not make sense to block one of the constructors just because there is more efficient alternative. We do not block less efficient constructs anywhere else. For example, ldc.i4 -1 IL instruction is valid, even though it is not the most efficient way to load -1 onto IL stack.

We can add a spec augment to say "should be created" instead of "are created" here.

[jkotas]

cc @AaronRobinsonMSFT

[MichalPetryka]

This looks like intended behavior to me. This feature does not do any transformations that C# does with a method call (overload resolution, look for the method in the type hierarchy, substitute default arguments, decide whether it is call or callvirt, ....). It makes sense that it does not replace a call with initobj either.

I'm fine with such behaviour but I think it might be potentially confusing for some users and might lead to some serialization libraries having unexpected behaviour with structs since they'd expect it to behave the same as new() in generics (especially since the new() constraint would accept such types).

However in such case I think that having an intrinsic like:

public class Type
{
    public bool HasParameterlessConstructor { get; }
}

would be useful to know whether a value type needs to use UnsafeAccessor for construction. Alternatively, some way to handle method not being found without catching a MissingMethodException would also solve this.

For context, the question about this behaviour stems from me wondering whether UnsafeAccessor could be used to implement a more efficient Activator.CreateInstance<T>. As far as I can see, the issues here would be this behaviour with structs, #89439 and the fact that CreateInstance throws for private ctors.

[AaronRobinsonMSFT]

this got me wondering if this behaviour is intended, or if it should work fine.

This looks like intended behavior to me. This feature does not do any transformations that C# does with a method call (overload resolution, look for the method in the type hierarchy, substitute default arguments, decide whether it is call or callvirt, ....). It makes sense that it does not replace a call with initobj either.

It is true, that this was intended, a constructor access always results in a call instruction. However, it would be a simple bug fix to special case a value type and call initobj if the parameterless constructor doesn't exist.

It does not make sense to block one of the constructors just because there is more efficient alternative.

Is this worth special casing in a future release? If not, I think we should update the doc and close this issue. If so, I can do this in .NET 8 or .NET 9.

[AaronRobinsonMSFT]

@lambdageek @fanyang-mono as FYI

[jkotas]

However in such case I think that having an intrinsic like:
public bool HasParameterlessConstructor { get; }

UnsafeAccessor is meant to be used by Roslyn source generators and similar tools. The source generators should inspect the methods on the type that they are calling and emit the UnsafeAccessor for the right method. I do not think it makes sense to mix the UnsafeAccessor with reflection.

the question about this behaviour stems from me wondering whether UnsafeAccessor could be used to implement a more efficient Activator.CreateInstance.

It is not clear whether we would want to allow [UnsafeAccessor(UnsafeAccessorKind.Constructor)] extern static T ParameterLessNew<T>(); in the generics support. And if we decided to allow it, the straightforward implementation would not be a lot more efficient than Activator.CreateInstance<T> for generic Ts. It can skip the try/catch that wraps exceptions with TargetInvocationExceptions, but doing more would be same type of work as the work needed to optimize Activator.CreateInstance<T> further.

Is this worth special casing in a future release?

It may be worth special casing for the type visibility skipping if/once we implement that. There is no point in using UnsafeAccessor for creating arrays without type visibility skipping. It is better to do a simple new MyType[...] instead of going through UnsafeAccessor.

we should update the doc and close this issue

Agree. Submitted #90070 with the doc update.