+972 52-548-6969
Tricky Code
time to read 1 min | 121 words
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
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
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.
Sorry: typo above: never happens to _m_e...
Skeet,
I changed that to IList<T> and List<T>, now try it.
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?
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
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 :)
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.
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.
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
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
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
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:
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
OH!
I see it now.
Yes, that makes MUCH more sense.
Thanks Jon.
Comment preview