11

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.

Michael
  • 3,222
  • 2
  • 21
  • 22
  • What are you exactly trying to do ? And what do you mean by "the output of this code is 1" ? – Laurent S. Apr 08 '13 at 14:28
  • @Bartdude, x is 1 after the `+=`. – Anthony Pegram Apr 08 '13 at 14:29
  • So are you trying to increment x by 2? What's the goal? – alan Apr 08 '13 at 14:29
  • 4
    Guys, forget the goal. He's dealt with that. The meaningful goal here is that he's trying to understand why the output is what it is. – Anthony Pegram Apr 08 '13 at 14:30
  • 2
    Very nice question! To add to your observations, replacing the `x += Increment(ref x);` line with `x = Increment(ref x) + x;` [produces](http://ideone.com/3y3QGd) `x == 2`! Could this be a bug? When I write `x = x + Increment(ref x);` I still get `x == 1`. – Sergey Kalinichenko Apr 08 '13 at 14:34
  • 1
    Well, I wouldn't consider it a bug, probably a feature, but too low level for me :-/ Let's say I would consider it to be potentially problematic if doing it myself as we're accessing the variable twice i the same statement, I would ask myself what the system is doing and in which order... – Laurent S. Apr 08 '13 at 14:35
  • 2
    Modifying the same variable twice in a single statement is a really bad idea. In C++ this would be undefined. – CodesInChaos Apr 08 '13 at 14:51

4 Answers4

7

+= 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.

Jude Melancon
  • 462
  • 5
  • 11
  • That makes sense. Basically it's expanding to `x = x + Increment(x)` and the ref part is overlooked completely. – Michael Apr 08 '13 at 14:57
6

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().

Matthew Watson
  • 104,400
  • 10
  • 158
  • 276
1

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.

Rik
  • 28,507
  • 14
  • 48
  • 67
1

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 :-)

Mark Hurd
  • 10,665
  • 10
  • 68
  • 101