dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.91k stars 4.63k forks source link

Some delegate invoke thunks fail to compile with `where T : allows ref struct` delegates #105029

Closed SingleAccretion closed 1 month ago

SingleAccretion commented 1 month ago

In a recent downstream merge, I noticed these when compiling S.R.Tests:

  ILC: Method '[S.P.CoreLib]System.Func`2<System.ReadOnlySpan`1<char>,int32>.InvokeObjectArrayThunk(ReadOnlySpan`1<char>)' will always throw because: Invalid IL or CLR metadata
  ILC: Method '[S.P.CoreLib]System.Func`2<System.ReadOnlySpan`1<char>,int32>.InvokeOpenInstanceThunk(ReadOnlySpan`1<char>)' will always throw because: Invalid IL or CLR metadata
dotnet-policy-service[bot] commented 1 month ago

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas See info in area-owners.md if you want to be subscribed.

MichalStrehovsky commented 1 month ago

These should not be getting the thunks. Unfortunately, we make this decision based on looking at the type definition, not the specific instantiated type, so we don't detect this new substitution possibility where we should:

https://github.com/dotnet/runtime/blob/66ae90f3b7ec4f13fffcd71913eca0b45777e58c/src/coreclr/tools/Common/TypeSystem/IL/DelegateInfo.cs#L137-L139

I wonder if it would be easiest to just suppress this message and let it generate the throwing body.

huoyaoyuan commented 1 month ago

These should not be getting the thunks.

Do you mean the thunks should be unreachable?

In coreclr they are reachable in second level of indirection like delegate2 = delegat1.Invoke, see https://github.com/dotnet/runtime/pull/104731#discussion_r1675580636.

jkotas commented 1 month ago

In coreclr they are reachable in second level of indirection like delegate2 = delegat1.Invoke, see https://github.com/dotnet/runtime/pull/104731#discussion_r1675580636.

There are number of different delegate thunks/stubs. #104731 is about delegate Invoke method. This issue is about the nativeaot-specific thunks that support System.Linq.Expressions.

agocke commented 1 month ago

What's the priority on fixing this? Will this block a 9.0 scenario or is it just a spurious warning?