ref Parameter and Assignment in same line

Question

I ran into a nasty bug recently and the simplified code looks like below:

int x = 0;
x += Increment(ref x);

...

private int Increment(ref int parameter) {
    parameter += 1;
    return 1;
}

The value of x after the Increment call is 1! This was an easy fix once I found out what was happening. I assigned the return value to a temporary variable and then updated x. I was wondering what explains this issue. Is it something in the spec or some aspect of C# that I'm overlooking.

Answer 1

+= reads the left argument then the right one, so it reads the variable, executes the method that increments, sums the results, and assigns to the variable. In this case, it reads 0, computes 1 with a side effect of changing the variable to 1, sums to 1, and assigns 1 for the variable. The IL confirms this, as it shows loads, a call, an add, and a store in that order.

Altering the return to 2 to see the result is 2 confirms that the method's return value is the part that "sticks."

Since someone asked, here's the full IL via LINQPad with its annotations:

IL_0000:  ldc.i4.0
IL_0001:  stloc.0     // x
IL_0002:  ldloc.0     // x
IL_0003:  ldloca.s    00 // x
IL_0005:  call        UserQuery.Increment
IL_000A:  add
IL_000B:  stloc.0     // x
IL_000C:  ldloc.0     // x
IL_000D:  call        LINQPad.Extensions.Dump

Increment:
IL_0000:  ldarg.0
IL_0001:  dup
IL_0002:  ldind.i4
IL_0003:  ldc.i4.1
IL_0004:  add
IL_0005:  stind.i4
IL_0006:  ldc.i4.2
IL_0007:  ret

Note that on line IL_000A, the stack contains the load of x (which was 0 when it was loaded) and the return value of Increment (which is 2). It then runs add and stloc.0 without further inspection of x's value.

Answer 2

This:

static void Main()
{
    int x = 0;
    x += Increment(ref x);
    Console.WriteLine(x);
}

Gets compiled to this:

.method private hidebysig static void Main() cil managed
{
    .entrypoint
    .maxstack 2
    .locals init (
        [0] int32 x)
    L_0000: nop 
    L_0001: ldc.i4.0 
    L_0002: stloc.0 
    L_0003: ldloc.0 
    L_0004: ldloca.s x
    L_0006: call int32 Demo.Program::Increment(int32&)
    L_000b: add 
    L_000c: stloc.0 
    L_000d: ldloc.0 
    L_000e: call void [mscorlib]System.Console::WriteLine(int32)
    L_0013: nop 
    L_0014: ret 
}

The compiler is using ldloca.s x to put the current value of x into a local register, and then it calls Increment() and uses add to add the return value to the register. This results in the value of x from before the call to Increment() being used.

The relevant part from the actual C# language spec is this:

An operation of the form x op= y is processed by applying binary operator overload resolution (§7.3.4) as if the operation was written x op y. Then,

If the return type of the selected operator is implicitly convertible to the type of x, the operation is evaluated as x = x op y, except that x is evaluated only once.

Which means that:

x += Increment(ref x);

Will be rewritten as:

x = x + Increment(ref x);

Since this will be evaluated from left-to-right, the old value of x will be captured and used instead of the value altered by the call to Increment().

Answer 3

The C# spec says about compound operators: (7.17.2)

the operation is evaluated as x = x op y, except that x is evaluated only once.

So x is evaluated (being 0) and then incremented by the result of the method.

Answer 4

It's implied by other answers, and I endorse the suggestion from C++ to treat this as "a bad thing to do", but the "simple" fix is:

int x = 0;
x = Increment(ref x) + x;

Because C# ensures left to right evaluation of expressions*, this does what you expected.

*Quoting section "7.3 Operators" of the C# Spec:

Operands in an expression are evaluated from left to right. For example, in F(i) + G(i++) * H(i), method F is called using the old value of i, then method G is called with the old value of i, and, finally, method H is called with the new value of i. This is separate from and unrelated to operator precedence.

Note that last sentence means this:

int i=0, j=0;
Console.WriteLine(++j * (++j + ++j) != (++i + ++i) * ++i);
i = 0; j = 0;
Console.WriteLine($"{++j * (++j + ++j)} != {(++i + ++i) * ++i}");
i = 0; j = 0;
Console.WriteLine($"{++j} * ({++j} + {++j}) != ({++i} + {++i}) * {++i}");

outputs this:

True
5 != 9
1 * (2 + 3) != (1 + 2) * 3

and that last line can be "trusted" to be the same values as used in the previous two expressions. I.E. even though the addition is performed before the multiplication, because of the brackets, the operands have already been evaluated.

Note that "refactoring" this to:

i = 0; j = 0;
Console.WriteLine(++j * TwoIncSum(ref j) !=  TwoIncSum(ref i) * ++i);
i = 0; j = 0;
Console.WriteLine($"{++j * TwoIncSum(ref j)} != { TwoIncSum(ref i) * ++i}");
i = 0; j = 0;
Console.WriteLine($"{++j} * {TwoIncSum(ref j)} != {TwoIncSum(ref i)} * {++i}");

private int TwoIncSum(ref int parameter)
{
    return ++parameter + ++parameter;
}

still works exactly the same:

True
5 != 9
1 * 5 != 3 * 3

But I still would prefer to not rely upon it :-)