Ayende @ Rahien

It's a girl

Tricky Code

Without running the code, what is the result of this masterpiece?

class Program
{
	static void Main(string[] args)
	{
		DoSomething("a","b");
	}

	public static void DoSomething<T>(IList<T> set)
	{
		Console.WriteLine(set.Count);
	}

	public static void DoSomething<T>(params T[] items)
	{
		List<T> set = new List<T>();
		foreach (T t in items)
		{
			if (t == null)
				continue;
			set.Add(t);
		}
		DoSomething(set);
	}
}

It surprised the hell out of me until I figured out what was going on, then I was very amused. This code works exactly as it should be, to produce a very different result than the expected one.

Comments

Jon Skeet
12/31/2007 11:59 AM by
Jon Skeet

Where do ISet and HashedSet come from? NHibernate?

Using HashSet from .NET 3.5, this prints "2" as I'd expect.

I'm now intrigued as to what your version does...

Jon

Chris Bilson
12/31/2007 12:11 PM by
Chris Bilson

params bugs are one of my favorite low hanging fruit things to look for when I hear someone has a stack overflow (never happens to be...no..nu-uh...never.)

It's too bad the compiler can't catch these, though.

Chris Bilson
12/31/2007 12:13 PM by
Chris Bilson

Sorry: typo above: never happens to me...

Ayende Rahien
12/31/2007 12:32 PM by
Ayende Rahien

Skeet,

I changed that to IList and List, now try it.

Jon Skeet
12/31/2007 12:44 PM by
Jon Skeet

Ick. Yes. I see.

I'd love to say that I'd have spotted the problem with this new code, but I'd be lying through my teeth.

So, what was your fix? Making the call DoSomething(set); or something else?

Ayende Rahien
12/31/2007 12:48 PM by
Ayende Rahien

Well, if you add make the variable an IList instead of a List, it will work.

You can cast it as well, to force it.

The best would be to skip overloading, I feel

Jon Skeet
12/31/2007 12:55 PM by
Jon Skeet

Agreed. That way you also avoid callers from other methods creating nested lists (or sets) when they didn't want to.

Anything that avoids having to wade through the overload resolution part of the spec, including the now-nearly-impenetrable type inference rules, is good :)

Jay R. Wren
12/31/2007 04:31 PM by
Jay R. Wren

I consider this a bug in the compiler.

I disagree that the code works exactly as it should be, unless someone can point to the C# spec and show me where this is defined.

Omer Mor
12/31/2007 07:56 PM by
Omer Mor

Wicked!!!

I wonder if there's any use for the type:

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List<System.Collections.Generic.List<System.Collections.Generic.List<

System.Collections.Generic.List>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

:-)

Omer.

Markus Zywitza
01/01/2008 04:18 PM by
Markus Zywitza

It's a shame that there is no compiler switch to stop the generic type inference in C#...

see also http://mortslikeus.blogspot.com/2007/11/wtf-overloading-and-generics.html

Jon Skeet
01/01/2008 06:47 PM by
Jon Skeet

Jay, the code is working exactly as it should be. Which bit of the following logic do you disagree with?

1) Both DoSomething methods are applicable function members due to type inference. The second method is applicable in its expanded form. (Spec 7.4.3.1)

2) Because the second method is applicable in its expanded form, the parameters are compared for "betterness" in that expanded form (7.4.3.2)

3) The parameter type sequences aren't identical, so the tie-breaking rules of 7.4.3.2 aren't applied.

4) The effective conversions are List to IList (the first method), and List to List (the second). The conversion from List to List is better (7.4.3.4).

All section numbers are from the Unified C# 3 Specification.

The generics involved make the whole thing a lot more confusing. I think most people would pick the right choice in this situation:

Invocation: Foo("hello");

Overloads:

void Foo (object o)

void Foo (string[] args)

The rules being applied are the same, it's just that Ayende's situation also has type inference involved, and the types being inferred are different for the two overloads.

Jon

Jay R. Wren
01/01/2008 09:06 PM by
Jay R. Wren

Jon,

I see no reason why List should match T[]. They both implement IList, but neither method signature uses IList.

I think I am definitely missing something.

I do see #1, that both are applicable due to type inference.

I don't see #2. What expanded form? List expands to param T[]? really? I must say that I am surprised.

4 - sure, List to List is better, but where is that match? I only see T[] and IList

I'll read up on the spec, but I thought I understood.

--

j

Jon Skeet
01/01/2008 11:36 PM by
Jon Skeet

List matches T[] because it's a single element, of type List.

In other words, it's trying to call:

DoSomething (new List[] { set });

The important bit which is tricky is that the inferred T of the called DoSomething() is actually List of the calling DoSomething.

Let's change the code to make it easier to understand, by getting rid of the actual recursion. Tiny snippet:

static void Main(string[] args)

{

    List<int> foo = new List<int>();

            DoSomething(foo);

}


public static void DoSomething<T>(IList<T> set)

{

    // Doesn't matter

}


public static void DoSomething<T>(params T[] items)

{

           // Doesn't matter

}

So the "T" inferred for the first DoSomething is int, but the "T" inferred for the second DoSomething is List. The same is true in the original code, but instead of int we're using the type parameter of the calling method, which doesn't change the rules but makes it harder to actually explain.

Does that help?

Jon

Jay R. Wren
01/08/2008 10:20 PM by
Jay R. Wren

OH!

I see it now.

Yes, that makes MUCH more sense.

Thanks Jon.

Comments have been closed on this topic.