21
votes

Given the following (heavily edited, pseudo-)code:

int count = 0;
thing.Stub(m => m.AddBlah()).WhenCalled(o => count++);
thing.Stub(m => m.RemoveBlah()).WhenCalled(o => count--);

DoStuff(thing);

Assert.AreEqual(1, count);

ReSharper provides a warning on count - "Access to modified closure". I understand why I'm getting this warning (the count variable is being modified in two different lambdas, and is likely to have undesirable semantics), but I don't understand ReSharper's advice: "Wrap local variable in array". If I let ReSharper do this, I get:

int count[] = { 0 };
thing.Stub(m => m.AddBlah()).WhenCalled(o => count[0]++);
thing.Stub(m => m.RemoveBlah()).WhenCalled(o => count[0]--);

DoStuff(thing);

Assert.AreEqual(1, count[0]);

And no warning.

Why is using an array safe?

2
Which version of ReSharper did you see this in?John Saunders

2 Answers

13
votes

I noticed this same thing in ReSharper myself, and was also left wondering why it doesn't warn when the value is wrapped in an array. The other answer here is unfortunately wrong, and seems to misunderstand how closures are implemented, so thought I would attempt explain (what I think is) the rationale behind this refactoring.

As you've seen, the result is the same whether array-wrapped or not, so the refactoring doesn't really "fix" anything and the same issues that can be encountered when accessing an ordinary modified closure exist after applying the change. However, after the change since the count array itself is not being modified (only its contents), the "Access to modified closure" warning is no longer relevant.

The change doesn't really make the problem any more obvious in (at least in my opinion), so it would seem that this suggestion is essentially telling ReSharper to ignore the issue, without having to resort to the rather messy // ReSharper disable AccessToModifiedClosure mechanism to suppress the error.

1
votes

This is because the 2 types are different. The int is a Value type and the array is a Reference type. This means that the int is on the stack and the array's pointer is on the stack.

When you update a Value type it updates that piece of stack memory. The Reference type on the other hand leaves that piece of stack memory alone and modifies what it points to.

Resharper doesn't complain about the array because the 2 different Lambda methods are creating a closure around the memory that points to where to update a value. Both Lambdas get the same address and don't change the original.