Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Compiler warnings with unobvious assignment #2587

Closed
Kir-Antipov opened this issue Jun 7, 2019 · 2 comments
Closed

[Proposal] Compiler warnings with unobvious assignment #2587

Kir-Antipov opened this issue Jun 7, 2019 · 2 comments

Comments

@Kir-Antipov
Copy link

C# is a beautiful and flexible language that will not allow a beginner to "shoot off his leg"... or it will?


Issue

Let's imagine that we have the following class:

class Foo
{
    public int Value;
    public Foo Next;
}

Then we want to do something like this:

var a = new Foo();
var b = new Foo { Value = 1, Next = new Foo { Value = 2 } };
a.Next = a = b;
Console.WriteLine(a.Next == a);

What we'll see in the console's output?
Most of you're now thinking something like this:

It's kinda simple!
First we assign "b" to "a", so "a" and "b" are now the same
Then we assign "a" (or "b", cause they're the same now) to the "Next" field of object "a" (or "b")
So a.Next == a is true!

And I have some bad news for you, cause it's false :D
If you're surprised, then let me surprise you even more:

a = b;
a.Next = a; // or b
Console.WriteLine(a.Next == a); // true

Explanation

When we have construction like this:

a = b;
a.x = b;

As a result, we'll get this IL-code:

;store b into a
ldloc b
stloc a 

;store b into a.x
ldloc a
ldloc b 
stfld class Foo Foo::X

But if we want to simplify this, and we'll use the following construction (that should mean the same as the example above):

a.x = a = b; 

IL will look like this:

ldloc a ;load "a" to the top of the stack
ldloc b ;load "b" to the top of the stack
dup     ;load "b" to the top of the stack
stloc a ;store "b" to "a"
stfld class Foo Foo::X ;store "b" to an object on top of the stack that is no longer "a"!

As we see, the assignment happens to the object we didn't expected (cause we know that assignment goes from right to left).

var aClone = a;
a.x = a = b; 
Console.WriteLine(a.x == a); // false
Console.WriteLine(a.x == b); // false
Console.WriteLine(aClone.x == a); // true
Console.WriteLine(aClone.x == b); // true

Proposal

I found this behaviour kinda unobvious. Especially for those who aren't familiar with the "subtleties" of C#

So I want to suggest introducing compiler warnings for those cases where the left side of an assignment contains a variable, the object inside of which changes somewhere in the right side of the expression, since it wouldn't be very cool to replace a = b; a.x = b; with a similar a.x = a = b; and spend a few sleepless nights in search of a bug :D

@YairHalberstadt
Copy link
Contributor

You can write an analyser for this yourself.

If you want it be more widely available, you can suggest this idea to roslyn-analysers.

@maxkoshevoi
Copy link

@YairHalberstadt thank you for advice. Issue moved to dotnet/roslyn-analyzers#2527

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants