Implicit conversion to System.Double with a nullable struct via compiler generated locals: why is this failing?

Question

Given the following, why does the InvalidCastException get thrown? I can't see why it should be outside of a bug (this is in x86; x64 crashes with a 0xC0000005 in clrjit.dll).

class Program
{
    static void Main(string[] args)
    {
        MyDouble? my = new MyDouble(1.0);
        Boolean compare = my == 0.0;
    }

    struct MyDouble
    {
        Double? _value;

        public MyDouble(Double value)
        {
            _value = value;
        }

        public static implicit operator Double(MyDouble value)
        {
            if (value._value.HasValue)
            {
                return value._value.Value;
            }

            throw new InvalidCastException("MyDouble value cannot convert to System.Double: no value present.");
        }
    }
}

Here is the CIL generated for Main():

.method private hidebysig static void Main(string[] args) cil managed
{
    .entrypoint
    .maxstack 3
    .locals init (
        [0] valuetype [mscorlib]System.Nullable`1<valuetype Program/MyDouble> my,
        [1] bool compare,
        [2] valuetype [mscorlib]System.Nullable`1<valuetype Program/MyDouble> CS$0$0000,
        [3] valuetype [mscorlib]System.Nullable`1<float64> CS$0$0001)
    L_0000: nop 
    L_0001: ldloca.s my
    L_0003: ldc.r8 1
    L_000c: newobj instance void Program/MyDouble::.ctor(float64)
    L_0011: call instance void [mscorlib]System.Nullable`1<valuetype Program/MyDouble>::.ctor(!0)
    L_0016: nop 
    L_0017: ldloc.0 
    L_0018: stloc.2 
    L_0019: ldloca.s CS$0$0000
    L_001b: call instance bool [mscorlib]System.Nullable`1<valuetype Program/MyDouble>::get_HasValue()
    L_0020: brtrue.s L_002d
    L_0022: ldloca.s CS$0$0001
    L_0024: initobj [mscorlib]System.Nullable`1<float64>
    L_002a: ldloc.3 
    L_002b: br.s L_003e
    L_002d: ldloca.s CS$0$0000
    L_002f: call instance !0 [mscorlib]System.Nullable`1<valuetype Program/MyDouble>::GetValueOrDefault()
    L_0034: call float64 Program/MyDouble::op_Implicit(valuetype Program/MyDouble)
    L_0039: newobj instance void [mscorlib]System.Nullable`1<float64>::.ctor(!0)
    L_003e: stloc.3 
    L_003f: ldloca.s CS$0$0001
    L_0041: call instance !0 [mscorlib]System.Nullable`1<float64>::GetValueOrDefault()
    L_0046: call float64 Program/MyDouble::op_Implicit(valuetype Program/MyDouble)
    L_004b: conv.r8 
    L_004c: ldc.r8 0
    L_0055: bne.un.s L_0060
    L_0057: ldloca.s CS$0$0001
    L_0059: call instance bool [mscorlib]System.Nullable`1<float64>::get_HasValue()
    L_005e: br.s L_0061
    L_0060: ldc.i4.0 
    L_0061: stloc.1 
    L_0062: ret 
}

Note lines 0x2D - 0x3E in the IL. It retrieves the MyDouble? instance, calls GetValueOrDefault on it, calls the implicit operator on that, and then wraps the result in a Double? and stores it in the compiler-generated CS$0$0001 local. In lines 0x3F to 0x55, we retrieve the CS$0$0001 value, 'unwrap' via GetValueOrDefault and then compare to 0... BUT WAIT A MINUTE! What is that extra call to MyDouble::op_Implicit doing on line 0x46?

If we debug the C# program, we indeed see 2 calls to implicit operator Double(MyDouble value), and it is the 2nd call that fails, since value is not initialized.

What is going on here?

Luciferase

Very interesting - I honestly cant figure this one out. If you copy and paste the disassembly from Reflector this doesn't happen. – Gruel

@Kragen - I did copy and paste the disassembly from Reflector... – Luciferase

Sure sounds like a bug. Probably my bad. The compiler code that deals with lifted user-defined conversion operations is extremely complicated. I'll take a look at it when I'm back in the office tomorrow. Apologies for the error. – Fax

@Eric - Cool, thanks. While you're in there, I'm sure you could add lifted member accessor operators - sounds pretty orthogonal and future break-proof to me. (I jest! I jest!) – Luciferase

Answer 1

It is clearly a C# compiler bug. Thanks for bringing it to my attention.

Incidentally, it is a bad practice to have a user defined implicit conversion operator that throws an exception; the documentation states that implicit conversions should be those that never throw. Are you sure you don't want this to be an explicit conversion?

Anyway, back to the bug.

The bug repros in C# 3 and 4 but not in C# 2. Which means that it was my fault. I probably caused the bug when I redid the user-defined lifted implicit operator code in order to make it work with expression tree lambdas. Sorry about that! That code is very tricky, and apparently I did not test it adequately.

What the code is supposed to do is:

First, overload resolution attempts to resolve the meaning of ==. The best == operator for which both arguments are valid is the lifted operator that compares two nullable doubles. Therefore it should be analyzed as:

Boolean compare = (double?)my == (double?)0.0; 

(If you write the code like this then it does the right thing in C# 3 and 4.)

The meaning of the lifted == operator is:

• evaluate both arguments
• if both are null then the result is true -- clearly this cannot happen in this case
• if one is null and the other is not then the result is false
• if both are not null then both are unwrapped to double and compared as doubles.

Now the question is "what is the right way to evaluate the left hand side?"

We have here a lifted user-defined conversion operator from MyDouble? to double?. The correct behaviour is:

• If "my" is null, then the result is a null double?.
• If "my" is not null then the result is the user-defined conversion of my.Value to double, and then the conversion of that double to double?.

Clearly something is going wrong in this process.

I'll enter a bug in our database, but any fix will probably miss the deadline for changes that make it into the next service pack. I would be looking into workarounds if I were you. Again, apologies for the error.

Fax

This makes designing and implementing a compiler sound hard. – Mathews

@Eric: Out of curiosity, at Microsoft, do you guys test your changes only yourselves? I imagine if there is a QA dept for C#, then they must be programmers trying to write all sorts of different code they could think of to break the changes? – Vaivode

@Eric: yes the throws in the implicit conversion is against the grain, but it's on purpose because we shouldn't ever be hitting it: it flags an uninitialized MyDouble struct, preventing code which doesn't convert to a MyDouble by the .ctor(Double) or the (not shown) explicit operator MyDouble(Double) from quietly failing. – Luciferase

@Joan: the search space for "all sorts of different code" that is acceptable in C# is larger than the known universe. Indeed, it is theoretically infinite. Even given Microsoft's revenue last quarter, I suspect the engineering effort to cover that space with tests is unachievable. – Luciferase

@Joan: Yes, our QA team are all experienced C#/VB programmers, and they spend all day trying to come up with programs that break the compilers and other tools. I thought we had pretty good coverage on the combination of lifted user-defined conversions and lifted built-in operators, but apparently we need more coverage there. – Fax

@Eric: Thanks, that workflow seems pretty good. It's super rare I see a bug about C# :O – Vaivode

@EricLippert: Are you aware if this was fixed in C# 5.0? It appears to repro in VS 2012 and I want to make sure I'm not just missing something. I'd understand if it just missed the bug bar. – Luciferase

Answer 2

This sure looks like a compiler bug to me. The IL suggests the compiler is generating code to convert the MyDouble? to a double with the conversion operator, then to a double?. But takes a nosedive when it then uses the conversion operator again on that double?. That's bad, wrong argument type. Nor is there a need.

This feedback article resembles this bug. Over 6 years old already, that must be a tricky part of the compiler. I do imagine it is.

Noteworthy

Agreed. The 2nd call to MyDouble::op_Implicit just is wrong. Thanks for digging up the Connect issue; you're right, it looks like that is it. I'll resumbit to see if I can get some traction on it, and since it is an issue in the lifting code generation, perhaps even some slip as to whether the lifted member accessor will be in C# 5! – Luciferase