60

I think this question will bring me instant fame here on Stack Overflow.

Suppose you have the following type:

// represents a decimal number with at most two decimal places after the period
struct NumberFixedPoint2
{
    decimal number;

    // an integer has no fractional part; can convert to this type
    public static implicit operator NumberFixedPoint2(int integer)
    {
        return new NumberFixedPoint2 { number = integer };
    }

    // this type is a decimal number; can convert to System.Decimal
    public static implicit operator decimal(NumberFixedPoint2 nfp2)
    {
        return nfp2.number;
    }

    /* will add more nice members later */
}

It has been written such that only safe conversions that don't lose precision are allowed. However, when I try this code:

    static void Main()
    {
        decimal bad = 2.718281828m;
        NumberFixedPoint2 badNfp2 = (NumberFixedPoint2)bad;
        Console.WriteLine(badNfp2);
    }

I am surprised this compiles and, when run, writes out 2. The conversion from int (of value 2) to NumberFixedPoint2 is important here. (An overload of WriteLine that takes in a System.Decimal is preferred, in case anyone wonders.)

Why on Earth is the conversion from decimal to NumberFixedPoint2 allowed? (By the way, in the above code, if NumberFixedPoint2 is changed from a struct to a class, nothing changes.)

Do you know if the C# Language Specification says that an implicit conversion from int to a custom type "implies" the existence of a "direct" explicit conversion from decimal to that custom type?

It becomes much worse. Try this code instead:

    static void Main()
    {
        decimal? moreBad = 7.3890560989m;
        NumberFixedPoint2? moreBadNfp2 = (NumberFixedPoint2?)moreBad;
        Console.WriteLine(moreBadNfp2.Value);
    }

As you see, we have (lifted) Nullable<> conversions here. But oh yes, that does compile.

When compiled in x86 "platform", this code writes out an unpredictable numeric value. Which one varies from time to time. As an example, on one occasion I got 2289956. Now, that's one serious bug!

When compiled for the x64 platform, the above code crashes the application with a System.InvalidProgramException with message Common Language Runtime detected an invalid program. According to the documentation of the InvalidProgramException class:

Generally this indicates a bug in the compiler that generated the program.

Does anyone (like Eric Lippert, or someone who has worked with lifted conversions in the C# compiler) know the cause of these bugs? Like, what is a sufficient condition that we don't run into them in our code? Because the type NumberFixedPoint2 is actually something that we have in real code (managing other people's money and stuff).

Jeppe Stig Nielsen
  • 60,409
  • 11
  • 110
  • 181
  • 8
    [File a bug report.](http://connect.microsoft.com/VisualStudio) – dtb Aug 20 '13 at 19:01
  • Just curious... how did you even come across this? – tnw Aug 20 '13 at 19:02
  • 6
    I think it would be worth separating this question out into two. I've a horrible feeling that the first part is actually *correct*... because you're using *an* explicit conversion, all *other* explicit conversions are included too. – Jon Skeet Aug 20 '13 at 19:05
  • @dtb I believe there already is one related to this issue. – Reed Copsey Aug 20 '13 at 19:07
  • ... and the first part isn't a bug, as far as I can tell. Though it is surprising. – Jon Skeet Aug 20 '13 at 19:16
  • I wasnt able to repro the errors and invalid program exception. Seems like the compiler sees you are trying to explictly convert to your datatype, sees that there is implicit conversion between an int and your data type, and does the explicit conversion from decimal to int. Doesnt really seem like a bug to me. IL_0012: call int32 [mscorlib]System.Decimal::op_Explicit(valuetype [mscorlib]System.Decimal) IL_0017: call valuetype UserGenerator.NumberFixedPoint2 UserGenerator.NumberFixedPoint2::op_Implicit(int32) – user1985513 Aug 20 '13 at 20:06
  • @user1985513 Weren't you able to reproduce the last part? What version of the .NET Framework an C# compiler did you use? What results did you get instead? Did it give `7`? – Jeppe Stig Nielsen Aug 20 '13 at 20:19
  • 10
    The cause of the bug is mistakes, likely my fault, so sorry about that. Lifted user-defined conversions involving decimal are one of the trickiest parts of the conversion logic because all the conversions on decimal are actually implemented as user-defined conversions but treated by the compiler as built-in conversions. The original C# compilers had a number of architectural issues involving "premature lowering" of lifted expressions that made this part of the semantic analysis very bug-prone. – Eric Lippert Aug 20 '13 at 20:46
  • 5
    When we re-wrote that logic in Roslyn we took the opportunity to eliminate premature lowerings whenever possible; instead, what I did was I did a very naïve and obviously-correct binding, and then built an optimizing lowering pass that ran on the naïve analysis. We thereby eliminated a number of long-standing bugs in lifted user-defined conversion analysis and code generation. If this subject interests you, I wrote a long series of blog articles describing the optimization pass; see http://ericlippert.com/2012/12/20/nullable-micro-optimizations-part-one/ – Eric Lippert Aug 20 '13 at 20:48
  • @EricLippert Thanks for the info. I wonder when Roslyn will ever "ship" as the default compiler. Maybe you can contribute some information to [some of my other recent questions](http://stackoverflow.com/users/1336654/jeppe-stig-nielsen?tab=questions&sort=newest) as well? Some people were looking for you in those threads as well. – Jeppe Stig Nielsen Aug 20 '13 at 21:02
  • @JeppeStigNielsen: I am still under NDA regarding shipping schedules and vehicles so regrettably I cannot say what the plan of record is for a Roslyn-backed default compiler. – Eric Lippert Aug 20 '13 at 21:05

2 Answers2

44

I'm just replying to the first part of the question to start with. (I suggest that the second part should be a separate question; it's more likely to be a bug.)

There's only an explicit conversion from decimal to int, but that conversion is being implicitly called in your code. The conversion happens in this IL:

IL_0010:  stloc.0
IL_0011:  ldloc.0
IL_0012:  call       int32 [mscorlib]System.Decimal::op_Explicit(valuetype [mscorlib]System.Decimal)
IL_0017:  call       valuetype NumberFixedPoint2 NumberFixedPoint2::op_Implicit(int32)

I believe this is the correct behaviour according to the spec, even though it's surprising1. Let's work our way through section 6.4.5 of the C# 4 spec (User-Defined Explicit Conversions). I'm not going to copy out all the text, as it would be tedious - just what the relevant results are in our case. Likewise I'm not going to use subscripts, as they don't work well with code font here :)

  • Determine the types S0 and T0: S0 is decimal, and T0 is NumberFixedPoint2.
  • Find the set of types, D, from which used-defined conversion operators will be considered: just { decimal, NumberFixedPoint2 }
  • Find the set of applicable user-defined and lifted conversion operators, U. decimal encompasses int (section 6.4.3) because there's a standard implicit conversion from int to decimal. So the explicit conversion operator is in U, and is indeed the only member of U
  • Find the most specific source type, Sx, of the operators in U
    • The operator doesn't convert from S (decimal) so the first bullet is out
    • The operator doesn't convert from a type that encompasses S (decimal encompasses int, not the other way round) so the second bullet is out
    • That just leaves the third bullet, which talks about the "most encompassing type" - well, we've only got one type, so that's okay: Sx is int.
  • Find the most specific target type, Tx, of the operators in U
    • The operator convers straight to NumberFixedPoint2 so Tx is NumberFixedPoint2.
  • Find the most specific conversion operator:
    • U contains exactly one operator, which does indeed convert from Sx to Tx, so that's the most specific operator
  • Finally, apply the conversion:
    • If S is not Sx, then a standard explicit conversion from S to Sx is performed. (So that's decimal to int.)
    • The most specific user-defined conversion operator is invoked (your operator)
    • T is Tx so there's no need for the conversion in the third bullet

The line in bold is the bit which confirms that a standard explicit conversion really is feasible, when only an explicit conversion from a different type is actually specified.


1 Well I found it surprising, at least. I'm not aware of seeing this before.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • That's still rather messy, especially considering how easily a user can stumble into unexpected behavior... – Shotgun Ninja Aug 20 '13 at 19:22
  • Ah, thanks, so the spec leads to this behavior. Very precise exposition you give. I guess the implicit conversion is harmful (easily used in un-intended ways), so maybe it's better to not have it. – Jeppe Stig Nielsen Aug 20 '13 at 19:30
  • @JeppeStigNielsen: You get the same behaviour if you make it explicit. – Jon Skeet Aug 20 '13 at 19:46
  • After your answer, the SO site lists the thread [Why do I need an intermediate conversion to go from struct to decimal, but not struct to int?](http://stackoverflow.com/questions/2806810/) as "Related", and it describes another surprising consequence of this *encompassing* rule, so go and upvote that thread as well if you find this interesting. – Jeppe Stig Nielsen Aug 20 '13 at 19:47
  • Good, I will remove it entirely, and people will have to create values of my struct with ordinary static methods (not `operator` conversions). – Jeppe Stig Nielsen Aug 20 '13 at 19:50
  • 8
    @JeppeStigNielsen: Jon's analysis is, of course, correct. A shorter way to characterize the behavior of the compiler is: a *cast* operator is permitted to silently insert a *standard* explicit or implicit conversion on "either side" of a user-defined explicit or implicit conversion. If you have a user-defined conversion from `Sailboat` to `Mammal` then you can have a conversion from `Watercraft` inserted on the "in" side and/or a conversion to `Giraffe` inserted on the "out" side of the user-defined conversion. – Eric Lippert Aug 20 '13 at 20:42
  • 5
    @JeppeStigNielsen: You might find my article on the subject interesting. http://blogs.msdn.com/b/ericlippert/archive/2007/04/16/chained-user-defined-explicit-conversions-in-c.aspx – Eric Lippert Aug 20 '13 at 20:42
  • Hey Jon Skeet, you are excellent in this part of the C# spec, could you please check and maybe improve [this answer on a user-defined conversion](http://stackoverflow.com/a/18389393/1336654) that I just wrote? – Jeppe Stig Nielsen Aug 22 '13 at 19:40
  • @JeppeStigNielsen: Not right now, but will hopefully do so later. – Jon Skeet Aug 22 '13 at 19:47
24

Your second portion (using nullable types) appears to be very similar to this known bug in the current compiler. From the response on the Connect issue:

While we do not currently have plans to address this issue in the next release of Visual Studio, we do plan to investigate a fix in Roslyn

As such, this bug will hopefully get corrected in a future release of Visual Studio and the compilers.

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373