Add awaiter implementation #133

extraes · 2024-06-17T23:55:48Z

Adds Pass61ImplementAwaiters
Looks for types implementing INotifyCompletion and generates new methods that allow the interop types to implement that interface, calling the Il2CppSystem.Action -> System.Action implicit conversion before calling the original method.

This makes originally-awaitable types (e.g. UniTasks, if the game has them) awaitable again.

I previously did this in a much less automated way with Cecil in another project, but the runtime didn't like what I was doing, so I figured I'd make it less janky and add it to Il2CppInterop, and sure enough the runtime no longer rejects the type.

ds5678 · 2024-12-22T05:38:37Z

I investigated this pull request and what it's attempting to solve. I discovered that the type system improvements I mentioned above will not automatically fix the issue.

As I understand it, the issue is that even if proper interface support is implemented with system interface support, only the Il2Cpp INotifyCompletion interface will be implemented since the generated OnCompleted method uses Il2CppSystem.Action instead of System.Action. Your pull request generates a helper method to avoid this.

For the rewrite, I could see a generic implementation where all Il2Cpp Action and Func delegates in method signatures are converted to their System counterparts, but I can also see your narrow implementation persisting into the rewrite if the generic implementation proves difficult or has some undesirable outcomes.

I am willing to review this pull request once it has been ported to AsmResolver.

Reference: https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/INotifyCompletion.cs

Paste from ML plugin, will be changed to remove unnecessary code/comments & improve generic handling

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

ds5678 · 2024-12-22T07:50:38Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+    public static void DoPass(RewriteGlobalContext context)
+    {
+        var corlib = context.GetAssemblyByName("mscorlib");
+        var actionUntyped = corlib.GetTypeByName("System.Action");


Do we seriously not use separate namespace and name when doing this throughout the whole project? That seems so inefficient.

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

- Use CorLibTypeFactory.Void for void reference - Added a couple more early-outs before LINQ - Use .Single - Use MemberCloner (and define a nested type for that) - Check method signature & forward to methods that may have been wonkily unhollowed

ds5678 · 2024-12-25T18:53:40Z

Whenever you're ready for another review, let me know.

extraes · 2024-12-26T20:45:10Z

Ah yeah, if you can review again that'd be great

ds5678

I gave it another pass. 👍

ds5678 · 2024-12-27T20:42:52Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+        var corlib = context.CorLib;
+        var actionUntyped = corlib.GetTypeByName("System.Action");
+
+        var actionConversion = actionUntyped.NewType.Methods.FirstOrDefault(m => m.Name == "op_Implicit") ?? throw new MissingMethodException("Untyped action conversion");


Methods.Single

ds5678 · 2024-12-27T20:51:59Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+            foreach (var typeContext in assemblyContext.Types)
+            {
+                // Used later for MemberCloner, just putting up here as an early exit in case .Module is ever null
+                if (typeContext.NewType.Module is null)


This should be impossible. I don't mind the null check, but I think a Debug.Assert would be better.

This check prevents the compiler from warning later down the line. I can replace it with a Debug.Assert if you'd like, but I'd then have to either ignore the warnings or ! the nullability away.

ds5678 · 2024-12-27T20:56:13Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+                if (typeContext.OriginalType.IsInterface || typeContext.OriginalType.Interfaces.Count == 0)
+                    continue;
+
+                var interfaceImplementation = typeContext.OriginalType.Interfaces.FirstOrDefault(interfaceImpl => interfaceImpl.Interface?.Name == nameof(INotifyCompletion));


I would prefer you check the namespace too.

ds5678 · 2024-12-27T20:59:54Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+                if (allOnCompleted.Length == 0)
+                {
+                    // Likely defined as INotifyCompletion.OnCompleted & the name is unhollowed as something like "System_Runtime_CompilerServices_INotifyCompletion_OnCompleted"
+                    allOnCompleted = typeContext.NewType.Methods.Where(m => ((string?)m.Name)?.EndsWith(nameof(INotifyCompletion.OnCompleted)) ?? false).ToArray();


I think you can check the method contexts for the exact name.

ds5678 · 2024-12-27T21:09:21Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+
+                // Conversion spits out an Il2CppSystem.Action, so look for methods that take that (and only that) in & return void, so the stack is balanced
+                // And use IsAssignableTo because otherwise equality checks would fail due to the TypeSignatures being different references
+                var interopOnCompleted = allOnCompleted.FirstOrDefault(m => m.Parameters.Count == 1 && m.Signature is not null && m.Signature.ReturnType == voidRef && SignatureComparer.Default.Equals(m.Signature.ParameterTypes[0], actionConversion.Signature?.ReturnType));


Use signature comparer instead of equality for the void check.

You also need to check that it's an instance method.

ds5678 · 2024-12-27T21:16:42Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+                typeContext.NewType.Interfaces.Add(new(notifyCompletionRef.Value));
+
+                var instructions = body.Instructions;
+                instructions.Add(CilOpCodes.Nop);


This is unnecessary.

The nops are there because I was copying the IL I got from a normally-compiled OnComplete. I'll remove them.

ds5678 · 2024-12-27T21:17:11Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+                    instructions.Add(CilOpCodes.Call, interopOnCompleted);
+                }
+
+                instructions.Add(CilOpCodes.Nop);


This is unnecessary.

ds5678 · 2024-12-27T21:20:55Z

Il2CppInterop.Generator/Passes/Pass61ImplementAwaiters.cs

+                }
+
+                // Conversion spits out an Il2CppSystem.Action, so look for methods that take that (and only that) in & return void, so the stack is balanced
+                // And use IsAssignableTo because otherwise equality checks would fail due to the TypeSignatures being different references


It seems like you no longer use IsAssignableTo.

Ah yeah that was a comment from before I saw SignatureComparer. I'll change it to reflect the current state.

Add awaiter implementation

d89f9ec

ds5678 added generation Related to assembly generation enhancement New feature or request labels Jul 15, 2024

This comment was marked as outdated.

Sign in to view

extraes added 3 commits December 21, 2024 22:12

Merge branch 'BepInEx:master' into awaiter-implementation

dfa39ba

Modder commits "worst code ever", asked to leave library

d06f960

Paste from ML plugin, will be changed to remove unnecessary code/comments & improve generic handling

Clean up code & remove vestiges of ML/Cecil

5737180

ds5678 mentioned this pull request Dec 22, 2024

Convert Il2Cpp Delegates to System Delegates in Method Signatures #195

Open

Fix import ordering & remove an unused using

1a1f541

ds5678 requested changes Dec 22, 2024

View reviewed changes

extraes added 4 commits December 24, 2024 23:10

Add "CorLib" reference for mscorlib to global context

4710b3f

Changes for PR review

f574321

- Use CorLibTypeFactory.Void for void reference - Added a couple more early-outs before LINQ - Use .Single - Use MemberCloner (and define a nested type for that) - Check method signature & forward to methods that may have been wonkily unhollowed

Remove commented vestigial code

7272bdf

Remove nullability shut-ups from CreateMemberReference

3f509b2

ds5678 requested changes Dec 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add awaiter implementation #133

Add awaiter implementation #133

extraes commented Jun 17, 2024

This comment was marked as outdated.

ds5678 commented Dec 22, 2024

ds5678 Dec 22, 2024

ds5678 commented Dec 25, 2024

extraes commented Dec 26, 2024

ds5678 left a comment

ds5678 Dec 27, 2024

ds5678 Dec 27, 2024

extraes Dec 29, 2024

ds5678 Dec 27, 2024

ds5678 Dec 27, 2024

ds5678 Dec 27, 2024

ds5678 Dec 27, 2024

extraes Dec 29, 2024

ds5678 Dec 27, 2024

ds5678 Dec 27, 2024

extraes Dec 29, 2024

Add awaiter implementation #133

Are you sure you want to change the base?

Add awaiter implementation #133

Conversation

extraes commented Jun 17, 2024

This comment was marked as outdated.

ds5678 commented Dec 22, 2024

Choose a reason for hiding this comment

ds5678 commented Dec 25, 2024

extraes commented Dec 26, 2024

ds5678 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment