113

I have this class

public class Overloaded
{
    public void ComplexOverloadResolution(params string[] something)
    {
        Console.WriteLine("Normal Winner");
    }

    public void ComplexOverloadResolution<M>(M something)
    {
        Console.WriteLine("Confused");
    }
}

If I call it like this:

        var blah = new Overloaded();
        blah.ComplexOverloadResolution("Which wins?");

It writes Normal Winner to the console.

But, if I add another method:

    public void ComplexOverloadResolution(string something, object somethingElse = null)
    {
        Console.WriteLine("Added Later");
    }

I get the following error:

The call is ambiguous between the following methods or properties: > 'Overloaded.ComplexOverloadResolution(params string[])' and 'Overloaded.ComplexOverloadResolution<string>(string)'

I can understand that adding a method might introduce a call ambiguity, but it's an ambiguity between the two methods that already existed (params string[]) and <string>(string)! Clearly neither of the two methods involved in the ambiguity is the newly added method, because the first is a params, and the second is a generic.

Is this a bug? What part of the spec says that this should be the case?

apaderno
  • 28,547
  • 16
  • 75
  • 90
McKay
  • 12,334
  • 7
  • 53
  • 76
  • 2
    I don't think `'Overloaded.ComplexOverloadResolution(string)'` refers to the `(string)` method; I think it refers to the `(string, object)` method with no object supplied. – phoog Dec 29 '11 at 23:11
  • 1
    @phoog Oh, that data got cut by StackOverflow because it's a tag, but the error message has the template designator. I'm adding it back in. – McKay Dec 29 '11 at 23:26
  • you've caught me out! I cited the relevant sections of the spec in my answer, but I haven't spent the last half hour reading and understanding them! – phoog Dec 29 '11 at 23:36
  • @phoog, looking through those parts of the spec, I see nothing about introducing ambiguity into methods other than itself and another method, not two other methods. – McKay Dec 29 '11 at 23:41
  • It occurred to me that this is just [rock-paper-scissors](http://en.wikipedia.org/wiki/Rock_paper_scissors): any set of two distinct values has a winner, but the complete set of three values does not. – phoog Apr 16 '12 at 14:25
  • @phoog, I believe that the C# spec has such rock-paper-scissors cases (@EricLippert called them intransitive betterness relations), but in this case the compiler thinks it has encountered such a case, but in fact it has not. Hence the bug. – McKay Apr 16 '12 at 20:58
  • ah, right, I'd forgotten that key part of the story :-) – phoog Apr 16 '12 at 21:01

5 Answers5

108

Is this a bug?

Yes.

Congratulations, you have found a bug in overload resolution. The bug reproduces in C# 4 and 5; it does not reproduce in the "Roslyn" version of the semantic analyzer. I've informed the C# 5 test team, and hopefully we can get this investigated and resolved before the final release. (As always, no promises.)

A correct analysis follows. The candidates are:

0: C(params string[]) in its normal form
1: C(params string[]) in its expanded form
2: C<string>(string) 
3: C(string, object) 

Candidate zero is obviously inapplicable because string is not convertible to string[]. That leaves three.

Of the three, we must determine a unique best method. We do so by making pairwise comparisons of the three remaining candidates. There are three such pairs. All of them have identical parameter lists once we strip off the omitted optional parameters, which means that we have to go to the advanced tiebreaking round described in section 7.5.3.2 of the specification.

Which is better, 1 or 2? The relevant tiebreaker is that a generic method is always worse than a non-generic method. 2 is worse than 1. So 2 cannot be the winner.

Which is better, 1 or 3? The relevant tiebreaker is: a method applicable only in its expanded form is always worse than a method applicable in its normal form. Therefore 1 is worse than 3. So 1 cannot be the winner.

Which is better, 2 or 3? The relevant tiebreaker is that a generic method is always worse than a non-generic method. 2 is worse than 3. So 2 cannot be the winner.

To be chosen from a set of multiple applicable candidates a candidate must be (1) unbeaten, (2) beat at least one other candidate, and (3) be the unique candidate that has the first two properties. Candidate three is beaten by no other candidate, and beats at least one other candidate; it is the only candidate with this property. Therefore candidate three is the unique best candidate. It should win.

Not only is the C# 4 compiler getting it wrong, as you correctly note it is reporting a bizarre error message. That the compiler is getting the overload resolution analysis wrong is a little bit surprising. That it is getting the error message wrong is completely unsurprising; the "ambiguous method" error heuristic basically picks any two methods from the candidate set if a best method cannot be determined. It is not very good at finding the "real" ambiguity, if in fact there is one.

One might reasonably ask why that is. It is quite tricky to find two methods that are "unambigously ambiguous" because the "betterness" relation is intransitive. It is possible to come up with situations where candidate 1 is better than 2, 2 is better than 3, and 3 is better than 1. In such situations we cannot do better than picking two of them as "the ambiguous ones".

I would like to improve this heuristic for Roslyn but it is a low priority.

(Exercise to the reader: "Devise a linear-time algorithm to identify the unique best member of a set of n elements where the betterness relation is intransitive" was one of the questions I was asked the day I interviewed for this team. It's not a very hard algorithm; give it a shot.)

One of the reasons why we pushed back on adding optional arguments to C# for so long was the number of complex ambiguous situations it introduces into the overload resolution algorithm; apparently we did not get it right.

If you would like to enter a Connect issue to track it, feel free. If you just want it brought to our attention, consider it done. I'll follow up with testing next year.

Thanks for bringing this to my attention. Apologies for the error.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • 1
    Thanks for the response. you said "1 is worse than 2", but it picks method 1 if I just have method 1 and 2? – McKay Dec 29 '11 at 23:50
  • @McKay: Whoops, you are right, I stated that backwards. I'll fix the text. – Eric Lippert Dec 29 '11 at 23:55
  • Ah yes, that's clearer. Thanks. This isn't actually a real issue we were having, but I was making a C# puzzle for my coworkers, and found this odd issue. The error isn't really a problem. – McKay Dec 30 '11 at 00:04
  • 1
    It feels awkward reading the phrase "the rest of the year" considering there isn't even half a week of it left :) – BoltClock Dec 30 '11 at 11:34
  • 2
    @BoltClock indeed, the statement "leaving for the rest of the year" implies one day off. – phoog Dec 30 '11 at 18:10
  • Re: the exercise to the reader - I get it if the betterness relation only returns "better" or "worse". But if it can return "equal" I don't believe you can do better than `O(n^2)`. – default.kramer Dec 31 '11 at 22:21
  • @default.kramer: You are of course entitled to your false belief. I assure you that there is a linear time algorithm that determines the unique best candidate if there is one. Hint: the german fact is that the candidate must beat at least one other candidate and be the *unique* unbeaten candidate. Does that help? – Eric Lippert Dec 31 '11 at 23:49
  • 1
    I think so. I read _"3) be the unique candidate that has the first two properties"_ as _"be the only candidate that (is unbeaten and beats at least one other candidate)"_. But your most recent comment makes me think _"(be the only candidate that is unbeaten) and beats at least one other candidate"_. English could really use grouping symbols. If the latter is true, then I get it again. – default.kramer Jan 01 '12 at 00:27
5

What part of the spec says that this should be the case?

Section 7.5.3 (overload resolution), along with sections 7.4 (member lookup) and 7.5.2 (type inference).

Note especially section 7.5.3.2 (better function member), which says in part "optional parameters with no corresponding arguments are removed from the parameter list," and "If M(p) is a non-generic method amd M(q) is a generic method, then M(p) is better than M(q)."

However, I do not understand these parts of the spec thoroughly enough to know which parts of the spec control this behavior, let alone to judge whether it is compliant.

phoog
  • 42,068
  • 6
  • 79
  • 117
  • But that doesn't explain why adding a member would cause an ambiguity between two methods that already existed. – McKay Dec 29 '11 at 23:36
  • @McKay fair enough (see edit). We'll just have to wait for Eric Lippert to tell us whether this is correct behavior :-> – phoog Dec 29 '11 at 23:51
  • 1
    Those are the right parts of the spec all right. Trouble is that they say that this *shouldn't* be the case! – Eric Lippert Dec 30 '11 at 03:06
3

you can avoid this ambiguity by changing the name of the first parameter in some methods and specifying the parameter which you want to assign

like this :

public class Overloaded
{
    public void ComplexOverloadResolution(params string[] somethings)
    {
        Console.WriteLine("Normal Winner");
    }

    public void ComplexOverloadResolution<M>(M something)
    {
        Console.WriteLine("Confused");
    }

    public void ComplexOverloadResolution(string something, object somethingElse = null)
    {
        Console.WriteLine("Added Later");
    }
}

class Program
{
    static void Main(string[] args)
    {
        Overloaded a = new Overloaded();
        a.ComplexOverloadResolution(something:"asd");
    }
}
MhdSyrwan
  • 1,613
  • 3
  • 19
  • 26
  • Oh, I know this code is bad, and there are several ways of working around it, the question was "why does the compiler behave in this manner?" – McKay Jan 04 '12 at 22:22
1

If you remove the params from your first method, this wouldn't happen. You first and third method have both valid calls ComplexOverloadResolution(string), but if your first method is public void ComplexOverloadResolution(string[] something) there will be no ambiguity.

Supplying value for a parameter object somethingElse = null makes it optional parameter and thus it doesn't have to be specified when calling that overload.

Edit: The compiler is doing some crazy stuff here. If you move your third method in code after your first, it will report correctly. So it seems it's taking the first two overloads and report them, without checking for the correct one.

'ConsoleApplication1.Program.ComplexOverloadResolution(params string[])' and 'ConsoleApplication1.Program.ComplexOverloadResolution(string, object)'

Edit2: New finding. Removing any method from the above three will produce no ambiguaty between the two. So it seems the conflict only appears if there are three methods present, regardless of order.

Tomislav Markovski
  • 12,331
  • 7
  • 50
  • 72
1
  1. If you write

    var blah = new Overloaded();
    blah.ComplexOverloadResolution("Which wins?");
    

    or just write

    var blah = new Overloaded();
    blah.ComplexOverloadResolution();
    

    it will ends up into the same method, in method

    public void ComplexOverloadResolution(params string[] something
    

    This is cause params keyword that makes from it it best match also for case when no parameter specified

  2. If you try to add yuor new method like this

    public void ComplexOverloadResolution(string something)
    {
        Console.WriteLine("Added Later");
    }
    

    It will perfectly compile and call this method as it is a perfect match for your call with a string parameter. Much stronger then params string[] something.

  3. You declare second method like you did

    public void ComplexOverloadResolution(string something, object something=null);
    

    Compiler, jumps in total confusion between the first method and this, just added one. Cause it doesn't know which function he should all now on your call

    var blah = new Overloaded();
    blah.ComplexOverloadResolution("Which wins?");
    

    Infact, if you remove string parameter from the call, like a following code, everything compiles correctly and works like before

    var blah = new Overloaded();
    blah.ComplexOverloadResolution(); // will be ComplexOverloadResolution(params string[] something) function called here, like a best match.
    
BoltClock
  • 700,868
  • 160
  • 1,392
  • 1,356
Tigran
  • 61,654
  • 8
  • 86
  • 123
  • First, you write two calling cases that are the same. So of course it will go into the same method, or did you mean to write something different? – McKay Dec 29 '11 at 23:38
  • But again, if I'm understanding the rest of your answer correctly, you're not reading what the compiler says is the confusion, namely between the first and second methods, not the new third one I just added. – McKay Dec 29 '11 at 23:39
  • ah, thanks. But that still leaves the problem I mention in my second comment to your post. – McKay Dec 29 '11 at 23:43
  • To be more clear, you state "Compiler, jumps in total confusion between the first method and this, just added one." But it isn't. It's jumping in confusion to the other two methods. The params method, and the generic method. – McKay Dec 29 '11 at 23:44
  • @McKay: whell, it's jump in confusion having a given `state` of 3 functions, not single or couple of them, to be precise. Infact, it's enough to comment *any* of them, in order to resolve a problem. The best match among available functions is that one with `params`, the second one is that one with `generics` parameter, when we add the third one it creates a confusion in a `set` of the functions. I think, it's, most likely, not clear error message produced by compiler. – Tigran Dec 29 '11 at 23:52