Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
SingleAccretion opened this issue Jul 17, 2024 · 5 comments · Fixed by #105871
Closed
Labels
area-NativeAOT-coreclr in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@SingleAccretion
Copy link
Contributor

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 dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jul 17, 2024
Copy link
Contributor

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

@MichalStrehovsky
Copy link
Member

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:

// Methods that have a byref-like type in the signature cannot be invoked with the object array thunk.
// We would need to box the parameter and these can't be boxed.
// Neither can be methods that have pointers in the signature.

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

@MichalStrehovsky MichalStrehovsky added this to the 9.0.0 milestone Jul 17, 2024
@MichalStrehovsky MichalStrehovsky removed the untriaged New issue has not been triaged by the area owner label Jul 17, 2024
@huoyaoyuan

This comment was marked as off-topic.

@jkotas

This comment was marked as off-topic.

@agocke
Copy link
Member

agocke commented Jul 30, 2024

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

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Aug 2, 2024
… generics

Fixes dotnet#105029

I looked into redoing delegate thunk management to be per instance but that would be an unnecessarily big diff. Instead adding a facility to allow `ILStubMethodIL` specialize per-instance if needed and in this specialization generate a throwing method body (that should be unreachable at runtime).
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr in-pr There is an active PR which will close this issue when it is merged
Projects
Archived in project
5 participants