UnsafeAccessor throws MissingFieldException on generic types

[AndriySvyryd]

Description

UnsafeAccessor throws MissingFieldException on generic types

Reproduction Steps

using System.Runtime.CompilerServices;

var baseObject = new DependentBase<long?>(1);
Console.WriteLine(DependentBase<long?>.UnsafeId(baseObject));

public class DependentBase<TKey>(TKey id)
{
    private TKey Id { get; } = id;

    [UnsafeAccessor(UnsafeAccessorKind.Field, Name = "<Id>k__BackingField")]
    public static extern ref long? UnsafeId(DependentBase<long?> @this);
}

Expected behavior

Gets field

Actual behavior

Unhandled exception. System.MissingFieldException: Field not found: 'DependentBase`1.k__BackingField'.

Regression?

No

Known Workarounds

This works:

public class DependentBase<TKey>(TKey id)
{
    private TKey Id { get; } = id;

    [UnsafeAccessor(UnsafeAccessorKind.Field, Name = "<Id>k__BackingField")]
    public static extern ref TKey UnsafeId(DependentBase<TKey> @this);
}

But this doesn't:

public class DependentBase<TKey>(TKey id)
{
    private TKey Id { get; } = id;

    [UnsafeAccessor(UnsafeAccessorKind.Field, Name = "<Id>k__BackingField")]
    public static extern ref T UnsafeId<T>(DependentBase<T> @this);
}

Configuration

.NET SDK 9.0.100-preview.7.24351.2

Other information

No response

[AndriySvyryd]

@AaronRobinsonMSFT

[dotnet-policy-service]

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

[vcsjones]

Looks like a duplicate of #92633.

[AndriySvyryd]

Looks like a duplicate of #92633.

Yes, but this wasn't fixed in #99468

[vcsjones]

Ah, sorry, I missed .NET SDK 9.0.100-preview.7.24351.2 in the initial post.

[jkoritzinsky]

I believe this failure is expected. The design in #99468 requires that the "generic context" of the type and method that has the UnsafeAccessor attribute be structured the same way as the target (type generic parameters on the type, method generic parameters on the method). In each of your examples, they don't meet that requirement. Let's look at each of them:

using System.Runtime.CompilerServices;

var baseObject = new DependentBase<long?>(1);
Console.WriteLine(DependentBase<long?>.UnsafeId(baseObject));

public class DependentBase<TKey>(TKey id)
{
    private TKey Id { get; } = id;

    [UnsafeAccessor(UnsafeAccessorKind.Field, Name = "<Id>k__BackingField")]
    public static extern ref long? UnsafeId(DependentBase<long?> @this);
}

This looks for a field on DependentBase<long?> with a metadata type of DependentBase<long?>, not DependentBase<TKey> where TKey is long?. This is a meaningful distinction.

public class DependentBase<TKey>(TKey id)
{
    private TKey Id { get; } = id;

    [UnsafeAccessor(UnsafeAccessorKind.Field, Name = "<Id>k__BackingField")]
    public static extern ref T UnsafeId<T>(DependentBase<T> @this);
}

This looks for a field of type DependentBase<MT0> where MT0 is a method generic parameter (which would never be on a field) on DependentBase<T>.

Your workaround that works is the only one to meet the design of the feature.

[AndriySvyryd]

I understand that this feature was never meant to be intuitive but consider either changing the design to make it "just work" for this case or throw a more informative exception.

[ranma42]

I would add that if it is expected to fail, it would be best if it did so consistently.
In my env all of the programs posted by @AndriySvyryd print 1.

.NET SDK:
 Version:           9.0.100-preview.5.24307.3
 Commit:            35b2c21ea6
 Workload version:  9.0.100-manifests.6407b7e4
 MSBuild version:   17.11.0-preview-24279-02+b963c24ef

Runtime Environment:
 OS Name:     kali
 OS Version:  2024.2
 OS Platform: Linux
 RID:         linux-x64
 Base Path:   /usr/share/dotnet/sdk/9.0.100-preview.5.24307.3/

[AaronRobinsonMSFT]

I understand that this feature was never meant to be intuitive but consider either changing the design to make it "just work" for this case or throw a more informative exception.

What would be a more informative exception in this case?

[AndriySvyryd]

What would be a more informative exception in this case?

System.MissingFieldException: Field not found: 'DependentBase`1.k__BackingField'. The first parameter for the unsafe accessor method 'UnsafeId' must be of type 'DependentBase<TKey>' where 'TKey' is a generic class type parameter.

Avoid using "generic context" or "bound"/"unbound"

[AaronRobinsonMSFT]

That would be more helpful. It would also require substantially more analysis to determine "why" the generic parameter didn't match. Appreciate the complexity here, but as noted this is a very advanced feature. We did try to document proper usage in the official documentation. Can you review the documentation and perhaps suggest a way to add some clarity?

https://learn.microsoft.com/dotnet/api/system.runtime.compilerservices.unsafeaccessorattribute?view=net-8.0#remarks

[AndriySvyryd]

That would be more helpful. It would also require substantially more analysis to determine "why" the generic parameter didn't match.

It seems that using a specific type as the generic argument is always wrong and I assume that it's easy to detect.

We did try to document proper usage in the official documentation. Can you review the documentation and perhaps suggest a way to add some clarity?

It's mostly clear, though it doesn't mention how generic methods on generic types should be handled.
As a simple improvement you could add an aka.ms link to that section in the exception message.

[AaronRobinsonMSFT]

It seems that using a specific type as the generic argument is always wrong and I assume that it's easy to detect.

This isn't quite correct. It is only correct if the generic argument is also the type of a field. For example, looking up the int _size field on List<T> can be done using List<long>. The issue being faced here is the look-up is performed on metadata signatures not on instantiated types. I realize this may not be a great distinction at the user level, but this does get to the complexity of using this API. The signature comparison is also why understanding precisely what higher level aspect is failing is difficult to express in an error message.

It's mostly clear, though it doesn't mention how generic methods on generic types should be handled.

The following was intended to capture that:

Generic parameters must match the target in form and index (that is, type parameters must be type parameters and method parameters must be method parameters).

This is also captured in the use examples for the API. See Examples

Snippet from the Examples section:

public class Class<T>
{
    private void GM<U>(U u) { }
}

class Accessors<V>
{
    ...
    [UnsafeAccessor(UnsafeAccessorKind.Method, Name = "GM")]
    public extern static void CallGM<X>(Class<V> c, X x);
    ...
}

As a simple improvement you could add an aka.ms link to that section in the exception message.

Not sure embedding links to official documentation for the type someone is using is appropriate. I would imagine if an API isn't doing what one expects, looking at the official documentation would be the step (1). Perhaps adding a comment about comparing signatures is how look up is done?

[AndriySvyryd]

Perhaps adding a comment about comparing signatures is how look up is done?

Also consider showing the full signature being looked up in the exception message.

[AaronRobinsonMSFT]

Perhaps adding a comment about comparing signatures is how look up is done?

Also consider showing the full signature being looked up in the exception message.

The best we could get to here would be the metadata encoded signature, which means a sequence of bytes, there is no obvious way to reconstitute the C# the user wrote. I assume you are not suggesting we print out the full signature of each attempted match, so then we need some metric to determine which of the possible matches to print out. For methods this could get complicated. For fields, I suppose we could print only the one where the name matches, but we're still back at a byte array.

I'm happy to update the official documentation about matching ECMA metadata signatures, but I'm not really convinced adding anything else to the exception message is going to be worth the effort.