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.

Print | posted on Monday, December 31, 2007 1:31 PM

Feedback


Gravatar

# re: Tricky Code 12/31/2007 1:59 PM Jon Skeet

Where do ISet<T> and HashedSet<T> come from? NHibernate?

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

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

Jon


Gravatar

# re: Tricky Code 12/31/2007 2:11 PM 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.


Gravatar

# re: Tricky Code 12/31/2007 2:13 PM Chris Bilson

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


Gravatar

# re: Tricky Code 12/31/2007 2:32 PM Ayende Rahien

Skeet,
I changed that to IList<T> and List<T>, now try it.


Gravatar

# re: Tricky Code 12/31/2007 2:44 PM 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<T>(set); or something else?


Gravatar

# re: Tricky Code 12/31/2007 2:48 PM Ayende Rahien

Well, if you add make the variable an IList<T> instead of a List<T>, it will work.
You can cast it as well, to force it.
The best would be to skip overloading, I feel


Gravatar

# re: Tricky Code 12/31/2007 2:55 PM 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 :)


Gravatar

# re: Tricky Code 12/31/2007 6:31 PM 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.


 re: Tricky Code 12/31/2007 9:56 PM 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<string>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

:-)

Omer.


Gravatar

# re: Tricky Code 1/1/2008 6:18 PM 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


Gravatar

# re: Tricky Code 1/1/2008 8:47 PM 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<T> to IList<T> (the first method), and List<T> to List<T> (the second). The conversion from List<T> to List<T> 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


Gravatar

# re: Tricky Code 1/1/2008 11:06 PM Jay R. Wren

Jon,

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

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<T> expands to param T[]? really? I must say that I am surprised.

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

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


Gravatar

# re: Tricky Code 1/2/2008 1:36 AM Jon Skeet

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

In other words, it's trying to call:

DoSomething (new List<T>[] { set });

The important bit which is tricky is that the inferred T of the called DoSomething() is actually List<T> 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<int>. 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


Gravatar

# re: Tricky Code 1/9/2008 12:20 AM Jay R. Wren

OH!

I see it now.

Yes, that makes MUCH more sense.

Thanks Jon.

Comments have been closed on this topic.