False C# compiler warning?

Question

Ok, this is a far stretched corner case we stumbled upon, but it made me curious.

Consider the following code:

public class Foo
{
    private int foo;
    public int Reset() => foo = 0; //remember, assignment expressions
                                   //return something!
}

Will this code compile?

No, it won't if you have fail on all warnings; you'll get a member foo is assigned but never used warning.

This code is, to all purposes, the same as:

public class Foo
{
    private int foo;
    public int Reset() { foo = 0; return foo; }
}

Which compiles just fine, so what is the problem here? Note that the => syntax is not the issue, its returning the assignment expression which seems to befuddle the compiler.

I was about to say, these are not equal - look for IL but...the answer is there — T.S., Nov 01 '17 at 01:43
It's a shorter case of the compiler specification is inadequate. https://stackoverflow.com/questions/45992172/why-does-the-compiler-think-environment-exit-can-return — Joshua, Nov 01 '17 at 02:57

score 36 · Accepted Answer · answered Nov 01 '17 at 00:34

In the first example, foo is assigned, but never read from. The 0 is assigned to foo, then 0 is returned, regardless of what the value of foo is (e.g. if another thread mutated it in the meantime).

In the second example, foo is assigned, then read from. If another thread modified foo in the meantime, the modified value will be returned, not 0.

You can see this in action by comparing the compiled IL. Given the following code:

public class Foo
{
    private int foo;
    public int Reset() { return (foo = 0); }
    public int Reset2() { foo = 0; return foo; }
}

The following IL is compiled for Reset and Reset2.

.method public hidebysig 
    instance int32 Reset () cil managed 
{
    .maxstack 3
    .locals init (
        [0] int32
    )

    IL_0000: ldarg.0
    IL_0001: ldc.i4.0
    IL_0002: dup
    IL_0003: stloc.0
    IL_0004: stfld int32 Foo::foo
    IL_0009: ldloc.0
    IL_000a: ret
}

.method public hidebysig 
    instance int32 Reset2 () cil managed 
{
    .maxstack 8

    IL_0000: ldarg.0
    IL_0001: ldc.i4.0
    IL_0002: stfld int32 Foo::foo
    IL_0007: ldarg.0
    IL_0008: ldfld int32 Foo::foo
    IL_000d: ret
}

In Reset, we only store to Foo::foo (stfld).

In Reset2, you can see that we both store to it and load from it (stfld + ldfld).

ah! nice, I made the wrong assumption that the left side was returned in the assignment expression, and no, its whats to the right. Makes sense, all clear now. Thanks. — InBetween, Nov 01 '17 at 00:44
https://meta.stackoverflow.com/questions/359302/merge-tags-visual-studio-mac-and-visual-studio-for-mac?noredirect=1#comment531840_359302 — Hans Passant, Nov 13 '17 at 21:59

score 5 · Answer 2 · answered Nov 01 '17 at 10:30

To answer this in pure C# terms, the value of an assignment expression is the value assigned. We can demonstrate this thus:

public class Foo
{
    public int Bar
    {
        get { return 2; }
        set { /* do nothing */ }
    }
}

/* … */

Foo foo = new Foo();
Console.WriteLine(foo.Bar = 23);

This prints 23 to the console because the value of foo.Bar = 23 is 23 even though the value of foo.Bar is always 2.

Most of the time the only effect of this is a minor performance benefit, because most of the time the value of the property or field is going to be what was assigned to it, but we can just reuse the local value we already have rather than accessing that property of field.

The warning here is not only technically correct, but also useful: Since foo is never actually read, it's just a waste of memory to have it and of time to assign to it, and you should remove it as cruft. (Or, of course, continue developing so that a not-yet-coded-for case where it is used comes up).

False C# compiler warning?

2 Answers2