87

I'm getting the following warning:

Access to foreach variable in closure. May have different behaviour when compiled with different versions of compiler.

This is what it looks like in my editor:

abovementioned error message in a hover popup

I know how fix this warning, but I want know why would I get this warning?

Is this about the "CLR" version? Is it related to "IL"?

Jeroen
  • 60,696
  • 40
  • 206
  • 339
  • 4
    http://stackoverflow.com/questions/8898925/is-there-a-reason-for-cs-reuse-of-the-variable-in-a-foreach – ta.speot.is Feb 16 '13 at 07:22
  • 1
    TL;DR answer: add .ToList() or .ToArray() at the end of your query expression and it will get rid of the warning – JoelFan Aug 25 '15 at 15:18

3 Answers3

136

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 foreach1:

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.

1 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.

Community
  • 1
  • 1
ta.speot.is
  • 26,914
  • 8
  • 68
  • 96
  • 7
    I've actually tried the foreach loop on different compilers getting different results using the same target (.Net 3.5). I used VS2010 (which in turn uses the compiler associated with .net 4.0 I believe) and VS2012 (.net 4.5 compiler I believe). In principle this means that if you are using VS2013 and editing a project targeting .Net 3.5 and building it on a build server that has a slightly older framework installed you could see different results of your program on your machine vs. the deployed build. – Ykok Mar 13 '14 at 13:27
  • Good answer, but not sure how "foreach" is relevant. Wouldn't this happen with manual enumeration, or even a simple for (int i= 0; i < collection.Size; i++) loop? It seems to be a problem with closures going out of scope, or more accurately, a problem with people understanding how closures behave when they go out of the scope they were defined inside. – Brad Jul 14 '14 at 12:53
  • The `foreach` stuff here comes from the content of the question. You're right that it can happen in various, more general, ways. – ta.speot.is Jul 14 '14 at 12:57
  • 1
    Why does R# still warn me, doesn't it read target framework, which I've set to 4.5. – Johnny_D Nov 19 '14 at 10:11
  • I'm not completely informed on the exact behaviour, but it varies with the compiler not the target framework. Also the same source file can be compiled by different compilers by opening up the project in older versions of VS and newer versions of VS. – ta.speot.is Nov 19 '14 at 11:14
  • 1
    *"So presumably you have to target .NET 4.5 or later "* This statement is not true. The version of .NET you target has no effect on this, the behavior is also changed in .NET 2.0, 3.5, and 4 if you are using C# 5 (VS 2012 or newer) to compile. That is why you only get this warning on .NET 4.0 or earlier, if you target 4.5 you don't get the warning because you can't compile 4.5 on a C#4 or older compiler. – Scott Chamberlain Feb 23 '15 at 21:44
  • So this is basically the same as the access to modified closure warning? It had me worried, I thought there might be something stranger going on – JonnyRaa May 05 '15 at 14:04
  • @ScottChamberlain I modified the answer and striked that part, so no more people get mislead. Absolutely not true. – Teejay Jul 31 '17 at 16:07
12

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.

David
  • 1,754
  • 23
  • 35
  • I read a lot of really long explanations that didn't solve this problem for me, then one short one that did. thanks! – Charles Clayton Jul 22 '15 at 20:37
  • I read the accepted answer and just thought "how is it a closure if it's not binding the variables?" but now I understand it's about when evaluation happens, thanks! – Jerome Aug 28 '15 at 12:13
  • Yes, this is obvious universal solution. Slow, memory-intensive, but I think it's really 100% working for all cases. – Al Kepp Oct 26 '16 at 17:32
9

A block-scoped variable should resolve the warning.

foreach (var entry in entries)
{
   var en = entry; 
   var result = DoSomeAction(o => o.Action(en));
}
reza.cse08
  • 5,938
  • 48
  • 39
Dmitry Gogol
  • 119
  • 1
  • 3