dotnet / dotnet/linker

Trimming warning IL2111 when using dictionary GetOrAdd pattern

Open
#2,790 5 comments 2 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
392
Forks
128
Avg merge
2d 10h
Merged PRs (30d)
2

Description

When using a `ConcurrentDictionary` and using the `GetOrAdd` method to populate the dictionary, it is not possible to remove all ILLink warnings. With the below code, I'm still getting:

```
Program.cs(79,9): Trim analysis warning IL2111: DotNetDispatcher.GetCachedMethodInfo(IDotNetObjectReference, String): Method 'DotNetDispatcher.g__ScanTypeForCallableMethods|2_0(Type)' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method.
```

### Repro

* `dotnet publish -c Release` the following app:

```xml


Exe
net7.0
enable
enable
true
win-x64
link

```

```C#
using System.Collections.Concurrent;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;

MyInterop interop = new();
DotNetDispatcher.Invoke(DotNetObjectReference.Create(interop), "Method1");
DotNetDispatcher.Invoke(DotNetObjectReference.Create(interop), "Method2");
DotNetDispatcher.Invoke(DotNetObjectReference.Create(interop), "Method1");

public class MyInterop
{
[JSInvokable]
public void Method1()
{
Console.WriteLine("Method1");
}

[JSInvokable]
public void Method2()
{
Console.WriteLine("Method2");
}
}

public static class DotNetObjectReference
{
public static DotNetObjectReference Create<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TValue>(TValue value) where TValue : class
{
if (value is null)
{
throw new ArgumentNullException(nameof(value));
}

return new DotNetObjectReference(value);
}
}

public sealed class DotNetObjectReference<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TValue> :
IDotNetObjectReference where TValue : class
{
internal DotNetObjectReference(TValue value)
{
Value = value;
}

public TValue Value { get; }

object IDotNetObjectReference.Value => Value;

[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
Type IDotNetObjectReference.Type => typeof(TValue);
}

public interface IDotNetObjectReference
{
object Value { get; }

[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
Type Type { get; }
}

[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
public sealed class JSInvokableAttribute : Attribute { }

public class DotNetDispatcher
{
private static readonly ConcurrentDictionary> _cachedMethodsByType = new();

public static void Invoke(IDotNetObjectReference objectReference, string methodIdentifier)
{
MethodInfo methodInfo = GetCachedMethodInfo(objectReference, methodIdentifier);

methodInfo.Invoke(objectReference.Value, null);
}

private static MethodInfo GetCachedMethodInfo(IDotNetObjectReference objectReference, string methodIdentifier)
{
var type = objectReference.Type;
var assemblyMethods = _cachedMethodsByType.GetOrAdd(type, ScanTypeForCallableMethods);
if (assemblyMethods.TryGetValue(methodIdentifier, out var result))
{
return result;
}
else
{
throw new ArgumentException($"The type '{type.Name}' does not contain a public invokable method with [{nameof(JSInvokableAttribute)}(\"{methodIdentifier}\")].");
}

static Dictionary ScanTypeForCallableMethods([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type type)
{
var result = new Dictionary(StringComparer.Ordinal);

foreach (var method in type.GetMethods(BindingFlags.Instance | BindingFlags.Public))
{
if (method.ContainsGenericParameters || !method.IsDefined(typeof(JSInvokableAttribute), inherit: false))
{
continue;
}

var identifier = method.Name!;

if (result.ContainsKey(identifier))
{
throw new InvalidOperationException($"The type {type.Name} contains more than one " +
$"[JSInvokable] method with identifier '{identifier}'. All [JSInvokable] methods within the same " +
"type must have different identifiers. You can pass a custom identifier as a parameter to " +
$"the [JSInvokable] attribute.");
}

result.Add(identifier, method);
}

return result;
}
}
}
```

### Notes

As you can see, I've flown `DynamicallyAccessedMembers` on the Type all the way down to the call to `GetMethods`. However, the trimmer is still warning that the nested method `ScanTypeForCallableMethods` "is accessed via reflection", which it is not. It is being called when the cache isn't populated, but not through reflection.

I'm not sure exactly how this should be handled. The naïve approach would be to specially know about `GetOrAdd` methods. Maybe some hueristic can be made about methods that are of the form `(T1 one, Func func)`, and if the referenced `func` has DynamicallyAccessedMembers applied, then the `T1 one` being passed to the function needs to match - just as if I was calling `func` and passing in `one` as a parameter.

cc @vitek-karas @MichalStrehovsky @sbomer

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.