Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C# method generic params parameter bug?

I appears to me as though there is a bug/inconsistency in the C# compiler.

This works fine (first method gets called):

public void SomeMethod(string message, object data);
public void SomeMethod(string message, params object[] data);

// ....
SomeMethod("woohoo", item);

Yet this causes "The call is ambiguous between the following methods" error:

public void SomeMethod<T>(string message, T data);
public void SomeMethod<T>(string message, params T[] data);

// ....
SomeMethod("woohoo", (T)item);

I could just use the dump the first method entirely, but since this is a very performance sensitive library and the first method will be used about 75% of the time, I would rather not always wrap things in an array and instantiate an iterator to go over a foreach if there is only one item.

Splitting into different named methods would be messy at best IMO.

Thoughts?

EDIT:

I guess Andrew might be onto something.

Full example:

public static class StringStuffDoer
{
    public static string ToString<T>(T item1, T item2)
    {
        return item2.ToString() + item1.ToString();
    }

    public static string ToString<T>(T item, params T[] items)
    {
        StringBuilder builder = new StringBuilder();

        foreach (T currentItem in items)
        {
            builder.Append(currentItem.ToString());
        }

        return item.ToString() + builder.ToString();
    }

    public static void CallToString()
    {
        ToString("someString", null); // FAIL
        ToString("someString", "another string"); // SUCCESS
        ToString("someString", (string)null); // SUCCESS
    }
}

I still think it is odd that the cast is needed - the call is not ambiguous. It works if you replace T with string or object or any non-generic type, so why wouldn't it work for the generic? It properly finds the two possible matching methods, so I believe that by the spec, it should pick the one that doesn't use params if possible. Correct me if I'm wrong here.

(NOT SO) FINAL UPDATE:

Sorry for taking you guys on this tyraid, I've apparently been staring at this too long...too much looking at generics and params for one night. The non-generic version throws the ambiguous error as well, I just had the method signature off in my mockup test.

REAL FINAL UPDATE:

Okay, this is why the problem didn't show up in my non-generic test. I was using "object" as the type parameters. SomeMethod(object) and SomeMethod(params object[]) do NOT throw the ambiguous error, I guess "null" automatically gets cast to "object". A bit strange I'd say, but somewhat understandable, maybe.

So, oddly enough, this call DOES work:

SomeMethod<object>("someMessage", null);
like image 979
Mike M Avatar asked May 18 '10 09:05

Mike M


3 Answers

It appears to me as though there is a bug/inconsistency in the C# compiler.

There certainly are bugs and inconsistencies in the compiler. You have not found one of them. The compiler is behaving completely correctly and according to spec in all these cases.

I'm doing my best to make sense of this very confusing question. Let me try to break it down into a series of questions.

Why does this succeed and call the first method?

public void SomeMethod(string message, object data);
public void SomeMethod(string message, params object[] data);
// ....
SomeMethod("woohoo", item);

(Presumption: that item is an expression of a compile-time type other than object[].)

Overload resolution must choose between two applicable methods. The second method is only applicable in its expanded form. A method that is applicable only in its expanded form is automatically worse than a method applicable in its normal form. Therefore the remaining better method is chosen.

Why does this fail with an ambiguity error?

public void SomeMethod<T>(string message, T data);
public void SomeMethod<T>(string message, params T[] data);
// ...
SomeMethod("woohoo", (T)item);

It is impossible to say because you do not say what "T" is. Where does T come from in this example? There are two type parameters named T declared; is this code in the context of one of those methods? Since those are different types both named T it could make a difference. Or is this yet a third type called T?

Since the question does not have enough information to answer it, I'm going to ask a better question on your behalf.

Why does this fail with an ambiguity error?

public void SomeMethod<T>(string message, T data);
public void SomeMethod<T>(string message, params T[] data);
// ...
SomeMethod("woohoo", "hello");

It doesn't. It succeeds. Type inference chooses "string" for T in both methods. Both generic methods are applicable; the second is applicable in its expanded form, so it loses.

OK, then why does this fail with an ambiguity error?

public void SomeMethod<T>(string message, T data);
public void SomeMethod<T>(string message, params T[] data);
// ...
SomeMethod("woohoo", null);

It doesn't. It fails with a "cannot infer T" error. There's not enough information here to determine what T is in either case. Since type inference fails to find an candidate method, the candidate set is empty and overload resolution has nothing to choose between.

So this succeeds because... ?

public void SomeMethod<T>(string message, T data);
public void SomeMethod<T>(string message, params T[] data);
// ...
SomeMethod("woohoo", (string)null);

Type inference infers that both methods are candidates when constructed with "string". Again, the second method is worse because it is applicable only in its expanded form.

What if we take type inference out of the picture? Why is this ambiguous?

public void SomeMethod<T>(string message, T data);
public void SomeMethod<T>(string message, params T[] data);
// ...
SomeMethod<string>("woohoo", null);

We now have three applicable candidates. The first method, the second method in its normal form, and the second method in its expanded form. The expanded form is discarded because expanded is worse than normal. That leaves two methods in their normal form, one taking string and the other taking string[]. Which is better?

When faced with this choice we always choose the one with the more specific type. If you said

public void M(string s) { ... }
public void M(object s) { ... }
...
M(null);

we'd choose the string version because string is more specific than object. Every string is an object but not every object is a string.

string is not convertible to string[]. string[] is not convertible to string. Neither is more specific than the other. Therefore this is an ambiguity error; there are two "best" candidates.

Then why does this succeed and what does it do?

public void SomeMethod<T>(string message, T data);
public void SomeMethod<T>(string message, params T[] data);
// ...
SomeMethod<object>("woohoo", null);

Again we have three candidates, not two. We automatically eliminate the expanded form as before, leaving two. Of the two methods in normal form which remain, which is better?

We must determine which is more specific. Every object array is an object, but not every object is an object array. object[] is more specific than object, so we choose to call the version that takes an object[]. We pass a null parameter array, which is almost certainly not what you intended.

This is why it is an extremely poor programming practice to make overloads like you are doing. You introduce potential for your users to run into all kinds of crazy ambiguities when you do this kind of stuff. Please do not design methods like this.

A better way to design this sort of logic is: (Note that I have not actually compiled this code, this is just off the top of my head.)

static string ToString<T>(T t)
{
    return t == null ? "" : t.ToString();
}
static string ToString<T>(T t1, T t2)
{
    return ToString<T>(t1) + ToString<T>(t2);
}
static string ToString<T>(T t1, T t2, params T[] rest)
{
    string firstTwo = ToString<T>(t1, t2);
    if (rest == null) return firstTwo;
    var sb = new StringBuilder();
    sb.Append(firstTwo);
    foreach(T t in rest)
      sb.Append(ToString<T>(t));
    return sb.ToString();
}

Now every case is handled with reasonable semantics and decent efficiency. And for any given call site you can immediately predict precisely which method will be called; there are only three possibilites: one argument, two arguments or more than two arguments. Each is handled unambiguously by a particular method.

like image 193
Eric Lippert Avatar answered Oct 12 '22 11:10

Eric Lippert


Seems to work for me, does the rest of your code look like the following?

class TestThing<T>
{
    public void SomeMethod(string message, T data)
    {
        Console.WriteLine("first");
    }
    public void SomeMethod(string message, params T[] data)
    {
        Console.WriteLine("second");
    }
}


class Program
{
    static void Main(string[] args)
    {
        var item = new object();
        var test_thing = new TestThing<object>();
        test_thing.SomeMethod("woohoo", item);
        test_thing.SomeMethod("woohoo", item, item);

        Console.ReadLine();
    }
}

Compiles fine on .NET 3.5 and outputs "first" and then "second" when run. Which version are you using/targeting?

EDIT

The issue from your code above is that the compiler can't tell what type null is. It is equally valid to assume that it is a string or an array of strings, hence why it is ambiguous when just null and not ambiguous when you specifically cast it (i.e. you're telling the compiler how it should be treated).

UPDATE

"The same can be argued for SomeMethod(string, string) and SomeMethod (string, params string[]), yet it works"

Actually, no it doesn't. You get the same ambiguous method problem.

public static string TypedToString(string item1, string item2)
{
  return "";
}

public static string TypedToString(string item1, params string[] items)
{
  return "";
}

public static void CallToString()
{
  TypedToString("someString", null); // FAIL
}
like image 22
Paolo Avatar answered Oct 12 '22 11:10

Paolo


You should change the signatures to be more specific. Eg:

void Foo(object o1);
void Foo(object o1, object o2);
void Foo(object o1, object o2, object o3, params object[] rest);

Note the difference between the last 2. This solves the ambiguity problem (will work for generics too).

Update:

public void SomeMethod<T>(string message, T data);
public void SomeMethod<T>(string message, T data1, T data2, params T[] data);

Just 2 methods. No ambiguity.

like image 24
leppie Avatar answered Oct 12 '22 11:10

leppie