How to prevent arithmetic overflow from corrupting the internal state of a class?

Question

I have a class that has two int fields x and y, and a method Increment that increments both of these fields by dx and dy respectively. I would like to prevent the state of my class to become corrupted by silent arithmetic overflows (which would result to x or y or both becoming negative), so I am explicitly incrementing the fields in a checked block:

class MyClass
{
    private int x;
    private int y;

    public void Increment(int dx, int dy)
    {
        checked { x += dx; y += dy; }
    }
}

This should ensure that in case of arithmetic overflow the caller will receive an OverflowException, and the state of my class will remain intact. But then I realized that an arithmetic overflow could occur in the increment of y, after the x has already successfully incremented, resulting to a different type of state corruption, which is not less disruptive than the first. So I changed the implementation of the Increment method like this:

public void Increment2(int dx, int dy)
{
    int x2, y2;
    checked { x2 = x + dx; y2 = y + dy; }
    x = x2; y = y2;
}

This seems like a logical solution to the problem, but now I am concerned that the compiler could "optimize" my carefully crafted implementation, and reorder the instructions in a way that would allow the x assignment to occur before the y + dy addition, resulting again to state corruption. I would like to ask if, according to the C# specification, this undesirable scenario is possible.

I am also considering removing the checked keyword, and instead compiling my project with the "Check for arithmetic overflow" option enabled (<CheckForOverflowUnderflow>true</CheckForOverflowUnderflow>). Could this make any difference, regarding a possible reordering of the instructions inside the Increment method?

---

Update: a more succinct arithmetic-overflow-safe implementation is possible by using tuple deconstruction. Is this version any different (less safe) than the verbose implementation?

public void Increment3(int dx, int dy)
{
    (x, y) = checked((x + dx, y + dy));
}

Clarification: The MyClass is intended to be used in a single-threaded application. Thread-safety is not a concern (I know that it's not thread-safe, but it doesn't matter).

Answer 1

Make your class immutable. When you want to change something, return a new instance.

class MyClass
{
    private int x;
    private int y;

    public MyClass Increment(int dx, int dy)
    {
        checked
        {
            return new MyClass { x = this.x + dx, y = this.y + dy }; 
        }
    }
}

And in your calling code, you'd replace

myClass.Increment( a, b );

with

myClass = myClass.Increment( a, b );

This ensures your class is always internally consistent.

If you don't want to return a new instance, you can get the same benefit by using an internal readonly struct.

public readonly struct Coords
{
    public int X { get; init; }
    public int Y { get; init; }
}

class MyClass
{
    private Coords _coords;

    public void Increment(int dx, int dy)
    {
        checked
        { 
            var newValue = new Coords { X = _coords.X + dx, Y = _coords.Y + dy };
        }
        _coords = newValue;
    }
}

Answer 2

TL;DR; This is perfectly safe to do within a single thread.

The CLI, which implements your code in native machine language, is simply not allowed to reorder instructions in such a fashion as to have a visible side-effect, at least as far as observations from a single thread are concerned. It is forbidden by the specification.

Let's take a look at ECMA-335, the specification for the CLR and CLI, (my bold)

I.12.6.4 Optimization

Conforming implementations of the CLI are free to execute programs using any technology that guarantees, within a single thread of execution, that side-effects and exceptions generated by a thread are visible in the order specified by the CIL.
... snip ...
There are no ordering guarantees relative to exceptions injected into a thread by another thread (such exceptions are sometimes called “asynchronous exceptions” (e.g., System.Threading.ThreadAbortException).

[Rationale: An optimizing compiler is free to reorder side-effects and synchronous exceptions to the extent that this reordering does not change any observable program behavior. end rationale]

[Note: An implementation of the CLI is permitted to use an optimizing compiler, for example, to convert CIL to native machine code provided the compiler maintains (within each single thread of execution) the same order of side-effects and synchronous exceptions.
This is a stronger condition than ISO C++ (which permits reordering between a pair of sequence points) or ISO Scheme (which permits reordering of arguments to functions). end note]

So exceptions must occur in the order specified in the IL code compiled by C#, therefore if an overflow occurs in a checked context, the exception must be thrown before any observation of the next instructions. (In an unchecked context, there is no such guarantee, because there is no exception, however the difference is not observable on a single thread.)

Note that this does not mean that the two additions cannot occur before the stores into local variables or before the two overflow checks, because the locals cannot be observed once an exception is thrown. In a fully optimized build, the locals will probably be stored in CPU registers, and wiped in the event of an exception.

The CPU is also free to reorder internally, so long as the same guarantees apply.

---

There is one exception to all of this, barring the mentioned multi-threading allowance:

Optimizers are granted additional latitude for relaxed exceptions in methods. A method is E-relaxed for a kind of exception if the innermost custom attribute CompilationRelaxationsAttribute pertaining to exceptions of kind E is present and specifies to relax exceptions of kind E.

However the current Microsoft implementation does not provide such a relaxation option anyway.

---

As to using the tuple-deconstructing syntax, unfortunately the spec for C# 7 has not been released, but this page on Github indicates that it should also be side-effect free.