Access to foreach variable in closure warning

Question

There are two parts to this warning. The first is...

Access to foreach variable in closure

...which is not invalid per se but it is counter-intuitive at first glance. It's also very hard to do right. (So much so that the article I link to below describes this as "harmful".)

Take your query, noting that the code you've excerpted is basically an expanded form of what the C# compiler (before C# 5) generates for foreach¹:

I [don't] understand why [the following is] not valid:

string s; while (enumerator.MoveNext()) { s = enumerator.Current; ...

Well, it is valid syntactically. And if all you're doing in your loop is using the value of s then everything is good. But closing over s will lead to counter-intuitive behaviour. Take a look at the following code:

var countingActions = new List<Action>();

var numbers = from n in Enumerable.Range(1, 5)
              select n.ToString(CultureInfo.InvariantCulture);

using (var enumerator = numbers.GetEnumerator())
{
    string s;

    while (enumerator.MoveNext())
    {
        s = enumerator.Current;

        Console.WriteLine("Creating an action where s == {0}", s);
        Action action = () => Console.WriteLine("s == {0}", s);

        countingActions.Add(action);
    }
}

If you run this code, you'll get the following console output:

Creating an action where s == 1
Creating an action where s == 2
Creating an action where s == 3
Creating an action where s == 4
Creating an action where s == 5

This is what you expect.

To see something you probably don't expect, run the following code immediately after the above code:

foreach (var action in countingActions)
    action();

You'll get the following console output:

s == 5
s == 5
s == 5
s == 5
s == 5

Why? Because we created five functions that all do the exact same thing: print the value of s (which we've closed over). In reality, they're the same function ("Print s", "Print s", "Print s"...).

At the point at which we go to use them, they do exactly what we ask: print the value of s. If you look at the last known value of s, you'll see that it's 5. So we get s == 5 printed five times to the console.

Which is exactly what we asked for, but probably not what we want.

The second part of the warning...

May have different behaviour when compiled with different versions of compiler.

...is what it is. Starting with C# 5, the compiler generates different code that "prevents" this from happening via foreach.

Thus the following code will produce different results under different versions of the compiler:

foreach (var n in numbers)
{
    Action action = () => Console.WriteLine("n == {0}", n);
    countingActions.Add(action);
}

Consequently, it will also produce the R# warning :)

My first code snippet, above, will exhibit the same behaviour in all versions of the compiler, since I'm not using foreach (rather, I've expanded it out the way pre-C# 5 compilers do).

Is this for CLR version?

I'm not quite sure what you're asking here.

Eric Lippert's post says the change happens "in C# 5". So presumably you have to target .NET 4.5 or later with a C# 5 or later compiler to get the new behaviour, and everything before that gets the old behaviour.

But to be clear, it's a function of the compiler and not the .NET Framework version.

Is there relevance with IL?

Different code produces different IL so in that sense there's consequences for the IL generated.

¹foreach is a much more common construct than the code you've posted in your comment. The issue typically arises through use of foreach, not through manual enumeration. That's why the changes to foreach in C# 5 help prevent this issue, but not completely.

The first answer is great, so I thought I'd just add one thing.

You're getting the warning because, in your example code, reflectedModel is being assigned an IEnumerable, which will only be evaluated at the time of enumeration, and enumeration itself could happen outside of the loop if you assigned reflectedModel to something with a broader scope.

If you changed

...Where(x => x.Name == property.Value)

to

...Where(x => x.Name == property.Value).ToList()

then reflectedModel would be assigned a definite list within the foreach loop, so you wouldn't receive the warning, as the enumeration would definitely happen within the loop, and not outside it.

A block-scoped variable should resolve the warning.

foreach (var entry in entries)
{
   var en = entry; 
   var result = DoSomeAction(o => o.Action(en));
}