21

The following two C# functions differ only in swapping the left/right order of arguments to the equals operator, ==. (The type of IsInitialized is bool). Using C# 7.1 and .NET 4.7.

static void A(ISupportInitialize x)
{
    if ((x as ISupportInitializeNotification)?.IsInitialized == true)
        throw null;
}

static void B(ISupportInitialize x)
{
    if (true == (x as ISupportInitializeNotification)?.IsInitialized)
        throw null;
}

But the IL code for the second one seems much more complex. For example, B is:

  • 36 bytes longer (IL code);
  • calls additional functions including newobj and initobj;
  • declares four locals versus just one.

IL for function 'A'…

[0] bool flag
        nop
        ldarg.0
        isinst [System]ISupportInitializeNotification
        dup
        brtrue.s L_000e
        pop
        ldc.i4.0
        br.s L_0013
L_000e: callvirt instance bool [System]ISupportInitializeNotification::get_IsInitialized()
L_0013: stloc.0
        ldloc.0
        brfalse.s L_0019
        ldnull
        throw
L_0019: ret

IL for function 'B'…

[0] bool flag,
[1] bool flag2,
[2] valuetype [mscorlib]Nullable`1<bool> nullable,
[3] valuetype [mscorlib]Nullable`1<bool> nullable2
        nop
        ldc.i4.1
        stloc.1
        ldarg.0
        isinst [System]ISupportInitializeNotification
        dup
        brtrue.s L_0018
        pop
        ldloca.s nullable2
        initobj [mscorlib]Nullable`1<bool>
        ldloc.3
        br.s L_0022
L_0018: callvirt instance bool [System]ISupportInitializeNotification::get_IsInitialized()
        newobj instance void [mscorlib]Nullable`1<bool>::.ctor(!0)
L_0022: stloc.2
        ldloc.1
        ldloca.s nullable
        call instance !0 [mscorlib]Nullable`1<bool>::GetValueOrDefault()
        beq.s L_0030
        ldc.i4.0
        br.s L_0037
L_0030: ldloca.s nullable
        call instance bool [mscorlib]Nullable`1<bool>::get_HasValue()
L_0037: stloc.0
        ldloc.0
        brfalse.s L_003d
        ldnull
        throw
L_003d: ret

 

Questions

  1. Is there any functional, semantic, or other substantial runtime difference between A and B? (We're only interested in correctness here, not performance)
  2. If they are not functionally equivalent, what are the runtime conditions that can expose an observable difference?
  3. If they are functional equivalents, what is B doing (that always ends up with the same result as A), and what triggered its spasm? Does B have branches that can never execute?
  4. If the difference is explained by the difference between what appears on the left side of ==, (here, a property referencing expression versus a literal value), can you indicate a section of the C# spec that describes the details.
  5. Is there a reliable rule-of-thumb that can be used to predict the bloated IL at coding-time, and thus avoid creating it?

      BONUS. How does the respective final JITted x86 or AMD64 code for each stack up?

[edit]

Additional notes based on feedback in the comments. First, a third variant was proposed, but it gives identical IL as A (for both Debug and Release builds). Sylistically, however, the C# for the new one does seem sleeker than A:

static void C(ISupportInitialize x)
{
    if ((x as ISupportInitializeNotification)?.IsInitialized ?? false)
        throw null;
}

Here also is the Release IL for each function. Note that the asymmetry A/C vs. B is still evident with the Release IL, so the original question still stands.

Release IL for functions 'A', 'C'…

        ldarg.0
        isinst [System]ISupportInitializeNotification
        dup
        brtrue.s L_000d
        pop
        ldc.i4.0
        br.s L_0012
L_000d: callvirt instance bool [System]ISupportInitializeNotification::get_IsInitialized()
        brfalse.s L_0016
        ldnull
        throw
L_0016: ret

Release IL for function 'B'…

[0] valuetype [mscorlib]Nullable`1<bool> nullable,
[1] valuetype [mscorlib]Nullable`1<bool> nullable2
        ldc.i4.1
        ldarg.0
        isinst [System]ISupportInitializeNotification
        dup
        brtrue.s L_0016
        pop
        ldloca.s nullable2
        initobj [mscorlib]Nullable`1<bool>
        ldloc.1
        br.s L_0020
L_0016: callvirt instance bool [System]ISupportInitializeNotification::get_IsInitialized()
        newobj instance void [mscorlib]Nullable`1<bool>::.ctor(!0)
L_0020: stloc.0
        ldloca.s nullable
        call instance !0 [mscorlib]Nullable`1<bool>::GetValueOrDefault()
        beq.s L_002d
        ldc.i4.0
        br.s L_0034
L_002d: ldloca.s nullable
        call instance bool [mscorlib]Nullable`1<bool>::get_HasValue()
L_0034: brfalse.s L_0038
        ldnull
        throw
L_0038: ret

Lastly, a version using new C# 7 syntax was mentioned which seems to produce the cleanest IL of all:

static void D(ISupportInitialize x)
{
    if (x is ISupportInitializeNotification y && y.IsInitialized)
        throw null;
}

Release IL for function 'D'…

[0] class [System]ISupportInitializeNotification y
        ldarg.0
        isinst [System]ISupportInitializeNotification
        dup
        stloc.0
        brfalse.s L_0014
        ldloc.0
        callvirt instance bool [System]ISupportInitializeNotification::get_IsInitialized()
        brfalse.s L_0014
        ldnull
        throw
L_0014: ret

[edit 2023]

Another new syntax, "property pattern matching", is available starting in C# 8. The following gives the same "best" IL as example D:

static void E(ISupportInitialize x)
{
    if (x is ISupportInitializeNotification { IsInitialized: true })
        throw null;
}
Glenn Slayden
  • 17,543
  • 3
  • 114
  • 108
  • 5
    In the first case the compiler is able to short-circuit the entire check when it sees that the first call on the *left* side is null, which means the left argument is null no matter what else it contains. Comparing it to anything other than null is guaranteed to be false. – Panagiotis Kanavos Aug 18 '17 at 08:37
  • 2
    In the second case though the left part is true so no short-circuiting is involved. The compiler *has* to calculate the result of the entire expression. If you check the calls, it is creating a nullable object due to the use of `?.`, and trying to call IsInitialized if the nullable has a value – Panagiotis Kanavos Aug 18 '17 at 08:41
  • Can you get rid of `?` operator in your examples and check again, will it gives the same results? – ASpirin Aug 18 '17 at 09:52
  • @ASprin surely that code wouldn't be probative for this case. Omitting `?` changes everything, since then you also no longer need `== true`, and you'd end up with `if ((x as ISupportInitializeNotification).IsInitialized)…`, and furthermore at that point you might as well just do `if (((ISupportInitializeNotification)x).IsInitialized)…` – Glenn Slayden Aug 18 '17 at 16:02
  • @PanagiotisKanavos That's a good *description* of the IL the compiler happens to end up producing here. For example, I now notice that **B's** problems begin right away: It gets trapped by its first instruction `ldc.i4.1` since that commits it to eventually doing a non-nullable compare. But you stop short of explaining why the same short-circuit isn't detected on the right side… I do realize `C#` guarantees left-to-right evaluation order perhaps in force here due to the possibilty of side-effecting `IsInitialized`, but surely failure of left side `true` could be precluded...? Optimization bug? – Glenn Slayden Aug 18 '17 at 16:53
  • Any reason why you'd want to place the `true` literal on the left side of the evaluation? Other than a stylistic desire to make the code look C++-like, that is? – code4life Aug 20 '17 at 02:45
  • Not for me, but I *was* wondering if anyone was going to bring that up. As you probably know, there's actually a legitimate reason to always put literals on the left in `C` / `C++`: it eliminates the possibility of unintentional assignment (via `=`) when equality (`==`) is intended, a nasty bug which can be hard to track down since `if (a = b)…` looks so similar to `if (a == b)…` (It's a moot issue in `C#`). So the reason instead is that I generally like to have a vague notion of the IL I'll be getting as I write `C#` code, so surprises I don't understand require some investigation of "why." – Glenn Slayden Aug 20 '17 at 04:26
  • 1
    This is code generated with optimizations turned off. You should expect it to be not optimized. – Eric Lippert Aug 20 '17 at 05:50
  • 1
    You could use the idiomatic combination of the null-propagating and the null-coalescing operator: `if ((x as ISupportInitializeNotification)?.IsInitialized ?? false)`. Even better the new is-expressions with patterns: `if (x is ISupportInitializeNotification y && y.IsInitialized)`. They both result in equal or shorter IL than A. – Johnbot Aug 20 '17 at 09:28
  • @EricLippert Thanks, I edited the post to include `Release` IL for all examples, including the two additional suggested by @Johnbot. The assymetry noted in the original question persists. – Glenn Slayden Aug 20 '17 at 19:52
  • I would say you have found a case where the C# compiler is not producing the best output possible. Almost a bug. – Jeppe Stig Nielsen Aug 21 '17 at 23:25
  • What type does `IsInitialized` return? Is it `bool`, `Nullable` or something else? What version of VC#? For now, VTC for no [mcve]. I really don't see anything like this in the spec and in a test with `bool`, there's no difference. – ivan_pozdeev Aug 22 '17 at 03:14
  • @ivan_pozdeev `ISupportInitialize` and `ISupportInitializeNotification` are both defined in the .NET namespace `System.ComponentModel`, provided by the **CLR** in `System.dll`. I omitted this information since I assumed it would be well-known and thus would only create clutter here. I have now modified the post to indicate this at the top. I believe the minimal example requirement is satisfied by the contrast between *A* and *B*, which are indeed complete, minimal, and verifiable. Please provide additional remarks if you aren't able to retract your VTC. Thanks. – Glenn Slayden Aug 22 '17 at 04:14
  • @ivan_pozdeev Re-checking your note, I've also now added the `VC#` and `.NET` versions I'm using. – Glenn Slayden Aug 22 '17 at 04:17
  • Just small comment on recent edit - property pattern matching was available since C# 8 [some docs](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/patterns), C# 10 only improved this feature to support succinct nested properties matching. – Guru Stron Mar 11 '23 at 03:05

2 Answers2

1

Looks like the 1st operand is converted to the 2nd's type for the purpose of comparison.

The excess operations in case B involve constructing a Nullable<bool>(true). While in case A, to compare something to a true/false, there's a single IL instruction (brfalse.s) that does it.

I couldn't find the specific reference in the C# 5.0 spec. 7.10 Relational and type-testing operators refers to 7.3.4 Binary operator overload resolution that in turn refers to 7.5.3 Overload resolution, but the latter one is very vague.

Glenn Slayden
  • 17,543
  • 3
  • 114
  • 108
ivan_pozdeev
  • 33,874
  • 19
  • 107
  • 152
  • 3
    Very vague? That's the most detailed part of the C# spec. Perhaps you meant *arcane*, since it's nearly impossible to make sense of by an average human reader, but that's another matter altogether. – Jeroen Mostert Aug 21 '17 at 20:51
1

So I was curious about the answer and took a look at the c# 6 specification (no clue where the c# 7 spec is hosted). Full disclaimer: I do not guarantee that my answer is correct, because I did not write the c# spec/compiler and my understanding of the internals is limited.

Yet I think that the answer lies in the resultion of the overloadable == operator. The best applicable overload for == is determined by using the rules for better function members.

From the spec:

Given an argument list A with a set of argument expressions {E1, E2, ..., En} and two applicable function members Mp and Mq with parameter types {P1, P2, ..., Pn} and {Q1, Q2, ..., Qn}, Mp is defined to be a better function member than Mq if

for each argument, the implicit conversion from Ex to Qx is not better than the implicit conversion from Ex to Px, and for at least one argument, the conversion from Ex to Px is better than the conversion from Ex to Qx.

What caught my eye is the argument list {E1, E2, .., En}. If you compare a Nullable<bool> to a bool the argument list should be something like {Nullable<bool> a, bool b}and for that argument list the Nullable<bool>.Equals(object o) method seems to be the best function, because it only takes one implicit conversion from bool to object.

However if you revert the order of the argument list to {bool a, Nullable<bool> b} the Nullable<bool>.Equals(object o) method no longer is the best function, because now you would have to convert from Nullable<bool> to bool in the first argument and then from bool to object in the second argument. That's why for case A a different overload is selected which seems to result in cleaner IL code.

Again this is an explanation that satisfies my own curiosity and seems to be in line with the c# spec. But I have yet to figure out how to debug the compiler to see what's actually going on.

Peter
  • 1,361
  • 15
  • 19