Issue with calling T[] Span<T>.ToArray()

hamarb123 commented 1 year ago

The following code produces an error that indicates there's an issue with either this library or Mono.Cecil (or I've severly misunderstood the error):

IL.DeclareLocals(true, new LocalVar(typeof(Span<int>)));
IL.Emit.Sizeof<int>();
IL.Emit.Ldc_I4(10);
IL.Emit.Mul();
IL.Emit.Conv_U();
IL.Emit.Localloc();
IL.Emit.Ldc_I4(10);
IL.Emit.Newobj(MethodRef.Constructor(typeof(Span<int>), typeof(void).MakePointerType(), typeof(int)));
IL.Emit.Stloc_0();
IL.Emit.Ldloca(0);
IL.Emit.Call(MethodRef.Method(typeof(Span<int>), "ToArray", TypeRef.TypeGenericParameters[0].MakeArrayType(), 0, Array.Empty<TypeRef>()));
int[] arr;
IL.Pop(out arr);

(This should generate something very similar to)

Span<int> span = stackalloc int[10];
int[] arr = span.ToArray();

Generates the error: Severity	Code	Description	Project	File	Line	Suppression State
Error		Fody/InlineIL: Unexpected error occured while processing method System.Void CSTest.Program::Main(System.String[]) at instruction IL_00d2: call System.Void InlineIL.IL/Emit::Call(InlineIL.MethodRef): System.NullReferenceException: Object reference not set to an instance of an object. at Mono.Cecil.ImportGenericContext.TypeParameter(String type, Int32 position) in C:\projects\fody\cecil\Mono.Cecil\Import.cs:line 94 at Mono.Cecil.DefaultMetadataImporter.ImportTypeSpecification(TypeReference type, ImportGenericContext context) in C:\projects\fody\cecil\Mono.Cecil\Import.cs:line 645 at Mono.Cecil.DefaultMetadataImporter.ImportType(TypeReference type, ImportGenericContext context) in C:\projects\fody\cecil\Mono.Cecil\Import.cs:line 492 at Mono.Cecil.DefaultMetadataImporter.ImportReference(TypeReference type, IGenericParameterProvider context) in C:\projects\fody\cecil\Mono.Cecil\Import.cs:line 732 at InlineIL.Fody.Model.TypeRefBuilder.GenericParameterTypeRefResolver.TryResolve(ModuleDefinition module, IGenericParameterProvider context) at InlineIL.Fody.Model.TypeRefBuilder.TypeSpecTypeRefResolver.TryResolve(ModuleDefinition module, IGenericParameterProvider context) at InlineIL.Fody.Model.MethodRefBuilder.<>cDisplayClass7_0.b3(MethodDefinition m) at System.Linq.Enumerable.<>cDisplayClass6_0`1.b0(TSource x) at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext() at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection) at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source) at InlineIL.Fody.Model.MethodRefBuilder.FindMethod(TypeReference typeRef, String methodName, Nullable`1 genericArity, TypeRefBuilder returnType, IReadOnlyList`1 paramTypes) at InlineIL.Fody.Model.MethodRefBuilder.MethodByNameAndSignature(ModuleDefinition module, TypeReference typeRef, String methodName, Nullable`1 genericArity, TypeRefBuilder returnType, IReadOnlyList`1 paramTypes) at InlineIL.Fody.Processing.ArgumentConsumer.gConsumeArgMethodRefBuilder\|15_0(Instruction instruction) at InlineIL.Fody.Processing.MethodWeaver.g__CreateInstructionToEmit\|25_0(<>cDisplayClass25_0& ) at InlineIL.Fody.Processing.MethodWeaver.ProcessIlEmitMethodCall(Instruction emitCallInstruction, Instruction& nextInstruction) at InlineIL.Fody.Processing.MethodWeaver.ProcessMethodCalls(CallInstructionHandler callHandler)	CSTest	A:\Code\Tests\TestInlineILFody\CSTest\Program.cs	19

It's this line that seems to cause the issue:

IL.Emit.Call(MethodRef.Method(typeof(Span<int>), "ToArray", TypeRef.TypeGenericParameters[0].MakeArrayType(), 0, Array.Empty<TypeRef>()));

Stack trace in a more readable form

Fody/InlineIL: Unexpected error occured while processing method System.Void CSTest.Program::Main(System.String[]) at instruction IL_00d2: call System.Void InlineIL.IL/Emit::Call(InlineIL.MethodRef): System.NullReferenceException: Object reference not set to an instance of an object.
   at Mono.Cecil.ImportGenericContext.TypeParameter(String type, Int32 position) in C:\projects\fody\cecil\Mono.Cecil\Import.cs:line 94
   at Mono.Cecil.DefaultMetadataImporter.ImportTypeSpecification(TypeReference type, ImportGenericContext context) in C:\projects\fody\cecil\Mono.Cecil\Import.cs:line 645
   at Mono.Cecil.DefaultMetadataImporter.ImportType(TypeReference type, ImportGenericContext context) in C:\projects\fody\cecil\Mono.Cecil\Import.cs:line 492
   at Mono.Cecil.DefaultMetadataImporter.ImportReference(TypeReference type, IGenericParameterProvider context) in C:\projects\fody\cecil\Mono.Cecil\Import.cs:line 732
   at InlineIL.Fody.Model.TypeRefBuilder.GenericParameterTypeRefResolver.TryResolve(ModuleDefinition module, IGenericParameterProvider context)
   at InlineIL.Fody.Model.TypeRefBuilder.TypeSpecTypeRefResolver.TryResolve(ModuleDefinition module, IGenericParameterProvider context)
   at InlineIL.Fody.Model.MethodRefBuilder.<>c__DisplayClass7_0.b__3(MethodDefinition m)
   at System.Linq.Enumerable.<>c__DisplayClass6_0`1.b__0(TSource x)
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at InlineIL.Fody.Model.MethodRefBuilder.FindMethod(TypeReference typeRef, String methodName, Nullable`1 genericArity, TypeRefBuilder returnType, IReadOnlyList`1 paramTypes)
   at InlineIL.Fody.Model.MethodRefBuilder.MethodByNameAndSignature(ModuleDefinition module, TypeReference typeRef, String methodName, Nullable`1 genericArity, TypeRefBuilder returnType, IReadOnlyList`1 paramTypes)
   at InlineIL.Fody.Processing.ArgumentConsumer.g__ConsumeArgMethodRefBuilder|15_0(Instruction instruction)
   at InlineIL.Fody.Processing.MethodWeaver.g__CreateInstructionToEmit|25_0(<>c__DisplayClass25_0& )
   at InlineIL.Fody.Processing.MethodWeaver.ProcessIlEmitMethodCall(Instruction emitCallInstruction, Instruction& nextInstruction)
   at InlineIL.Fody.Processing.MethodWeaver.ProcessMethodCalls(CallInstructionHandler callHandler)

ltrzesniewski commented 1 year ago

Thanks for the report! It's most definitely a bug, but I need to investigate this a bit more to understand what's going on.

In the meantime, you can work around the issue by not filtering on the return type:

IL.Emit.Call(MethodRef.Method(typeof(Span<int>), nameof(Span<int>.ToArray)));

hamarb123 commented 1 year ago

Thank you for the work around, let me know how the investigating goes

ltrzesniewski commented 1 year ago

I understood and fixed the problem, but your example also made another unit test fail (oops), so I'll need to fix that as well before I release the next version. 😅

hamarb123 commented 1 year ago

Thank you for fixing the issue! Let me know when you've released it so I can update to it.

I was wondering if it would be possible for you to also add an overload to MethodRef.Method that accepts a bool isInstance so we can have full control over which method it selects.

Would it also be possible to have a mechanism that tells the weaver to not check if the method exists at all so you can target methods which may not exist? This could just be a part of how the overload with bool isInstance works since it wouldn't need to resolve, but it could also just be a method on a MethodRef that returns a new MethodRef that the weaver interprets as 'just emit it without searching for it'.

Edit: should I make a new issue for the above feature requests?

ltrzesniewski commented 1 year ago

I was wondering if it would be possible for you to also add an overload to MethodRef.Method that accepts a bool isInstance so we can have full control over which method it selects.

I suppose I could, but:

That would add yet another overload to an already rather long list, with a long list of parameters
static is just one of many flags a method can have, should it be considered more interesting than the others?

Why would you need to have this degree of control? You'll get an error if a selector is ambiguous or no method is found.

Would it also be possible to have a mechanism that tells the weaver to not check if the method exists at all so you can target methods which may not exist?

Theoretically yes, but it could be complicated in practice and could require a lot of work. Currently, the metadata comes from the resolved type. Without this metadata, the full method signature blob would need to be manually reconstructed, and that's not exactly straightforward: see ECMA-335 II.23.2.

May I ask why you'd like to have such a feature?

hamarb123 commented 1 year ago

May I ask why you'd like to have such a feature?

I've thought about it more, and I probably don't really need this currently, I was thinking in the case of having some funky method I wanted to call.

ltrzesniewski commented 1 year ago

The fix is released in v1.7.4.

ltrzesniewski / InlineIL.Fody

Issue with calling T[] Span<T>.ToArray() #29