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

Provide examples of Info.OfMethod for generic method and methods with arguments. #582

Open
virzak opened this issue Feb 9, 2024 · 9 comments

Comments

@virzak
Copy link

virzak commented Feb 9, 2024

Describe the issue

Info.MethodOf documentation is somewhat unclear regarding generic methods and parameters.

Below are some of my attempts that came up short.

Minimal Repro

namespace MyNs
{
    public class MyClass
    {
        // OK
        public static readonly MethodInfo ProduceMethod = Info.OfMethod<MyClass>(nameof(Produce)); // OK
        public static readonly MethodInfo ProduceMethod2 = Info.OfMethod("InfoOfMethodOverload", "MyNs.MyClass", "Produce"); // OK
        public int Produce() => throw new NotImplementedException();
        
        // What do I do?
        //public static readonly MethodInfo ProduceGeneric = Info.OfMethod<MyClass>(nameof(Produce)+"``1<T>");
        //public static readonly MethodInfo ProduceGeneric = Info.OfMethod("InfoOfMethodOverload", "MyNs.MyClass", nameof(Produce)+"``1<T>");
        //public static readonly MethodInfo ProduceGeneric = Info.OfMethod("InfoOfMethodOverload", "MyNs.MyClass", nameof(Produce)+"``1<mscorlib|System.Int32>");
        public int Produce<T>() => throw new NotImplementedException();

        // What do I do?
        //public static readonly MethodInfo ProduceWithParam = Info.OfMethod<MyClass>(nameof(Produce), "System.Runtime|System.Int32");
        //public static readonly MethodInfo ProduceWithParam2 = Info.OfMethod("InfoOfMethodOverload", "MyNs.MyClass", "Produce", "mscorlib|System.Int32");
        //public static readonly MethodInfo ProduceWithParam2 = Info.OfMethod("InfoOfMethodOverload", "MyNs.MyClass", "Produce", "int");
        public int Produce(int z) => throw new NotImplementedException();
    }
}
@ltrzesniewski
Copy link
Member

That double backquote in the documentation seems off, can you try with a single backquote instead?

Also, the <T> part shouldn't be needed for open generics.

@virzak
Copy link
Author

virzak commented Feb 9, 2024

@ltrzesniewski, didn't work with single backtick either.

@ltrzesniewski ltrzesniewski self-assigned this Feb 10, 2024
@tom-englert
Copy link
Member

  • Actually there seems to be no way to distinguish generic and non-generic methods with the same parameters.
  • If the generic parameter is used in a methods parameter, it is possible by specifying "T" as parameter type.

see sample here: https://github.com/Fody/InfoOf/blob/master/Tests/ParamCheckerTests/GenericMethod.cs

@tom-englert
Copy link
Member

@virzak I've updated the readme, did this answer your questions?

@ltrzesniewski ltrzesniewski removed their assignment Feb 11, 2024
@virzak
Copy link
Author

virzak commented Feb 11, 2024

@tom-englert,

Thanks, the questions are answered. A few thoughts:

  • Specifying type parameters in MethodOf would be awesome. Here is a real world example where I'd love to have a handle on two different Count methods.
  • Specifying type parameters might not be enough if they differ by constraints:
public int Produce<T>(T? t) where T : class => throw new NotImplementedException();
public int Produce<T>(T? t) where T : struct => throw new NotImplementedException();
  • Saw a typo therE by the way: "Actually ther is no specific support".

@virzak
Copy link
Author

virzak commented Feb 11, 2024

Another thought on this topic.
Does it make sense to drive generation from method attributes?

class MyClass
{
  [CreateMethodInfo(location = "MyNS.MyOtherClass.ExistingField")]
  public int Produce<T>(T? t) where T : class => throw new NotImplementedException();

  [CreateMethodInfo(location = "MyNS.MyOtherClass.NonExistingField")] // will be added
  public int Produce<T>(T? t) where T : struct => throw new NotImplementedException();
}

class MyOtherClass
{
  static MethodInfo ExistingField = null! // will be overwritten
}

@ltrzesniewski
Copy link
Member

ltrzesniewski commented Feb 11, 2024

Specifying type parameters in MethodOf would be awesome.

I suppose that's a missing feature if it's not achievable today.

Specifying type parameters might not be enough if they differ by constraints

That's actually not possible to get in C#, even if it seems so. 🙂

Here are the actual signatures of your example methods:

public int Produce<T>(T t) where T : class
public int Produce<T>(System.Nullable<T> t) where T : struct

Removing the ? from the struct one will cause a compilation error.

Does it make sense to drive generation from method attributes?

That would be quite a bit of additional work, and would only be useful for symbols in your own assembly.

If InfoOf isn't currently sufficient for your needs (and you really want those two method handles), I may suggest using my InlineIL weaver along with the ldtoken opcode, although that may prove a bit overkill. 😅

@tom-englert
Copy link
Member

In your sample code above both methods are distinguishable by their parameters, so you can get them via

InfoOf.Method<Class>("Produce", "T")
InfoOf.Method<Class>("Produce", "Nullable<T>")

or to make it even more obvious, rename T to TClass and TStruct.

Specifying type parameters in MethodOf would be awesome.

I think it's a very special edge case where you have a generic overload and the generic parameter is not used in the method parameters, so I wonder if it's worth spending efforts on this.

Also I don't see the benefit of designing such an highly optimized code - and then accessing it via reflection?

@virzak
Copy link
Author

virzak commented Feb 26, 2024

Also I don't see the benefit of designing such an highly optimized code - and then accessing it via reflection?

@tom-englert, you're probably correct. This is all about EF Core expression trees detecting that a certain method has been called. This not likely where the performance bottleneck will be, since EF Core itself pays a heavy cost upfront to generate queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants