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

[Bug] Method extension overloading with generics fails to account for generic constraints #1628

Closed
Expro opened this issue Jun 13, 2018 · 16 comments

Comments

@Expro
Copy link

Expro commented Jun 13, 2018

Not sure if this fits here, or in Roslyn repo.

Consider following code:

public static T Extension<T>(this T) where T : ISomeInterface<TypeOne>
{
   throw new NotImplementedException();
}

public static T Extension<T>(this T) where T : ISomeInterface<TypeTwo>
{
   throw new NotImplementedException();
}

Where TypeOne and TypeTwo inheritance tree meets only on root (object).

This will result in error: Type '...' already defines a member called 'Extension' with the same parameter types.

Expected result: both extension methods are being recognized as differend overloads.

@Expro Expro changed the title [Bug] Method extension overloading with generics fails to acount for generic constraints [Bug] Method extension overloading with generics fails to account for generic constraints Jun 13, 2018
@HaloFour
Copy link
Contributor

The CLR doesn't allow for overloading on generic type constraints. They are not a part of the method signature, they're just metadata.

@benaadams
Copy link
Member

Could make a funny name for clr when name collisions are resolved via generic constraints

public static T Extension<T>(this T) where T : class
{
   throw new NotImplementedException();
}

public static T Extension<T>(this T) where T : struct
{
   throw new NotImplementedException();
}

@gafter
Copy link
Member

gafter commented Jun 13, 2018

This works in C# 7.3 without any issues.

Closing as fixed.

@benaadams
Copy link
Member

@gafter ❤️

@HaloFour
Copy link
Contributor

@gafter

The complaint seems to be about being able to define the extension methods in the same static class as overloads, which is a compiler error.

@gafter
Copy link
Member

gafter commented Jun 14, 2018

@HaloFour Perhaps so, but in C# 7.3 there is a workaround.

@benaadams
Copy link
Member

benaadams commented Jun 14, 2018

And (the improved overloading) used as a suggested perf workaround... dotnet/corefxlab#2358 (comment)

@ufcpp
Copy link

ufcpp commented Jun 14, 2018

using System;

static class Ex
{
    public struct Struct { }
    public struct Class { }

    public static T Extension<T>(this T x, Class _ = default) where T : class
    {
        throw new NotImplementedException();
    }

    public static T Extension<T>(this T x, Struct _ = default) where T : struct
    {
        throw new NotImplementedException();
    }
}

class Program
{
    static void Main()
    {
        1.Extension();
        "".Extension();
    }
}

@Expro
Copy link
Author

Expro commented Jun 14, 2018

Splitting extension versions into different classes is in fact acceptable workaround.

@dzmitry-lahoda
Copy link

Not pleasant and not a self-evident workaround. So I will have to create a range of extensions suffixed 1,2,3. And have to move it into top-level class as extension require.

using System;

Console.WriteLine();

record A();
record B();
record D();

record C() {
    public T To<T>() where T: A => throw new Exception();
    public T To<T>() where T: B => throw new Exception();
}

static class CE {
    static T To<T>(this C self) where T: D =>  throw new Exception();
}

@HaloFour
Copy link
Contributor

This is not a bug, the runtime does not consider generic type constraints as a part of the method signature and such does not permit overloading based on them.

@bernd5
Copy link
Contributor

bernd5 commented Apr 22, 2022

As a workaround you can write:

using System;

Console.WriteLine();

record A();
record B();
record D();

struct Dummy<T>{}

record C() {
    public T To<T>(Dummy<A> dummy = default) where T: A => throw new Exception();
    public T To<T>(Dummy<B> dummy = default) where T: B => throw new Exception();
    public T To<T>(Dummy<D> dummy = default) where T: D => throw new Exception();
}

static class CE {
    static T To<T>(this C self) where T: D =>  throw new Exception();
}

@dzmitry-lahoda
Copy link

No need for a dummy. May end-use that for partial updates/merges (if value not set, take from _). Nice:

record A();
record B();
record D();

record C() {
    public T To<T>(A _ = default) where T: A => throw new Exception();
    public T To<T>(B _ = default) where T: B => throw new Exception();
}

static class CE {
    static T To<T>(this C self) where T: D =>  throw new Exception();
}

@bernd5
Copy link
Contributor

bernd5 commented Apr 24, 2022

I used an empty struct to reduce the overhead (the compiler may not eliminate the not used parameter...).
But for a record it is fine I guess...

@dzmitry-lahoda
Copy link

With default hack we get:

  • error CS0854: An expression tree may not contain a call or invocation that uses optional arguments in LINQ expressions
  • cannot use x.To for sure in delegate based LINQ as it is not good with optional parameters

@CyrusNajmabadi
Copy link
Member

@dzmitry-lahoda you're commenting on a closed issue.

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

8 participants