Lifting ForEach, Breaking Change in Visual Studio 2012

This post is updated due to the STUPID error I originally made in the last code sample. Thanks to the folks that pointed it out!

Also, I neglected to thank Adil Mughal for grabbing the ReSharper warnng text for me.

There’s a breaking change you should know about in Visual Studio 2012. In case you have a short attention span, aka have a life, I’ll give you the punch line first. The behavior of closures has changed in the specific instance of a lambda expression.

Good news: You aren’t writing the affected code on purpose, or if you are I honestly want to know why

The Answer and the Explanation

While you might expect this code to print 0,1,2,3, etc, it actually prints 9,9,9,9, etc.

When variables are used in a lambda expression, the actual variable goes along for the ride. Not a copy, not the value but the actual variable. This is called a closure and is essential to you getting the behavior you expect at many other points in .NET. You can also find examples in this blog entry by Frans Bouma and a different example by Dmitri Nesteruk.

This is a significant enough issue that both Visual Basic and ReSharper issue a warning. C# without ReSharper does not issue a warningThe Visual Basic warning is

Warning

1

Using the iteration variable in a lambda expression may have unexpected results. Instead, create a local variable within the loop and assign it the value of the iteration variable.

The ReSharper warning is

Access to modified closure

The Visual Studio 2012 Change

When a lambda expression uses the looping variable of a for each loop in either Visual Basic or C#, the Visual Studio 2012 compiler creates a new variable scoped to the loop and assigns the value of the looping variable to this variable. Logically it’s the same as:

Since the new variable is scoped to the loop, there’s a new copy for every iteration of the loop. The original code compiled in Visual Studio 2012 prints 0,1,2,3… Try it!

Why this is a Really Good Thing to Change

I’ve been showing this code at user group presentations around the country and taken a few other opportunities to put this code in front of .NET developers – some of them really good .NET developers – and asking what the code would do.

The most common response is a few people in the room get it after about a minute. Sometimes, no one gets it. It’s exceedingly rare that anyone will have an immediate knee jerk response that it will print all nines. I get boatloads of “I should have seen that!” and very few “I saw it right away!” And, the very fact I am asking the question creates a context that the code probably doesn’t do what you’d expect and I made it as obvious as I possibly could. Your code is going to be a list of customers or invoices or something else. The vast majority of .NET developers do not expect the VS 2010 result, even though it is technically correct.

The async features of Visual Studio 2012 will create lambda expressions for you, and you will be much more likely to create lists of lambda expressions for simultaneous execution. Your ability to see this issue in your code gets significantly tougher in some VS 2012 scenarios. It was time to make this change.

It would have been nice if this had been an error from the beginning of lambda usage in .NET. But that didn’t happen and adding this as an error would have been a different kind of breaking change. For better or worse, the automatic fix won over adding an error or warning.

Is it a big deal?

I get asked a lot whether this is a big deal – sometimes even by people that are responsible for a code base.

It’s probably not a big deal in your code. The chances are excellent that you do not have code that uses a looping variable in a lambda expression. If you do, it’s probably a mistake – that’s why VB and ReSharper issue a warning. And it’s possible that any change in behavior will either be immensely obvious and perhaps trigger a test failure, or be so subtle that it never breaks your code at all.

But these are the wrong question to ask about a breaking change. The right question is “Can you guarantee that this does not exist anywhere in the code bases you are responsible for?” That’s the bar for a breaking change – can I guarantee I know how it affects me. The only way to do that is to run a tool that looks for the problem – check for the Visual Basic warning or run ReSharper.

I have had only one person say they were extremely confident that their unit tests would catch this change. I believe they are wrong. This is a side case that can give extremely unexpected behavior. It’s not a change I’d trust unit tests to find.

What’s Not Changed

This change happened because it can be very difficult to see the issue in a for each loop. It wasn’t done lightly and the teams decided to do the change in as narrow a scope as possible. That means the change was not made to for loops in either Visual Basic or C#. The following code returns 10,10,10, etc with both the VS 2012 and VS 2010 compilers.

The Critical Footnote

Any breaking compiler changes are a big deal. They aren’t such a big deal if we know about them, they are side cases and the new behavior is improved. The Visual Studio teams have announced that they are working to replace the compilers in a future version of Visual Studio. The current compilers are old and it seems highly unlikely that they can be rewritten without breaking changes. We can handle some breaking changes as long as the issues can be found with tools – specifically we can identify code that is clean of known behavior changes because of compiler changes. One of the reasons I want the community talking about this breaking change is that we need to establish expectations for all future breaking changes. So, talk about it. Run ReSharper. Tell your friends.!