Is "Access to modified closure" resolved by comprehension syntax?

Question

ReSharper 6.0 gives me the "Access to modified closure" warning for the dr identifier in the first code snippet.

private IEnumerable<string> GetTheDataTableStrings(DataTable dt) {
    foreach (DataRow dr in dt.Rows) {
        yield return GetStringFuncOutput(() => dr.ToString());
    }
}

I think I have a basic understanding of what this warning is trying to protect me from: dr changes several times before GetTheDataTableStrings's output is interrogated, and so the caller might not get the output/behavior I expect.

But R# doesn't give me any warning for the second code snippet.

private IEnumerable<string> GetTheDataTableStrings(DataTable dt) {
    return from DataRow dr in dt.Rows select GetStringFuncOutput(dr.ToString);
}

Is it safe for me to discard this warning/concern when using the comprehension syntax?

Other code:

string GetStringFuncOutput(Func<string> stringFunc) {
    return stringFunc();
}

Answer 1

First off, you are correct to be concerned about the first version. Each delegate created by that lambda is closed over the same variable and therefore as that variable changes, the meaning of the query changes.

Second, FYI we are highly likely to fix this in the next version of C#; this is a major pain point for developers.

(UPDATE: This answer was written in 2011. We did in fact take the fix described below in C# 5.)

In the next version each time you run through the "foreach" loop we will generate a new loop variable rather than closing over the same variable every time. This is a "breaking" change but in the vast majority of cases the "break" will be fixing rather than causing bugs.

The "for" loop will not be changed.

See http://ericlippert.com/2009/11/12/closing-over-the-loop-variable-considered-harmful-part-one/ for details.

Third, there is no problem with the query comprehension version because there is no closed-over variable that is being modified. The query comprehension form is the same as if you'd said:

return dt.Rows.Select(dr=>GetStringFuncOutput(dr.ToString));

The lambda is not closed over any outer variable, so there is no variable to be modified accidentally.

Answer 2

The issue that Resharper is warning about has been resolved in both C# 5.0 and VB.Net 11.0. The following are extracts from the language specifications. Note that the specifications can be found in the following paths by default on a machine with Visual Studio 2012 installed.

• C:\Program Files (x86)\Microsoft Visual Studio 11.0\VB\Specifications\1033\Visual Basic Language Specification.docx
• C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC#\Specifications\1033\CSharp Language Specification.docx

C# Language Specification Version 5.0

8.8.4 The foreach statement

The placement of v inside the while loop is important for how it is captured by any anonymous function occurring in the embedded-statement.

For example:

int[] values = { 7, 9, 13 };
Action f = null;
foreach (var value in values)
{
    if (f == null) f = () => Console.WriteLine("First value: " + value);
}
f();

If v was declared outside of the while loop, it would be shared among all iterations, and its value after the for loop would be the final value, 13, which is what the invocation of f would print. Instead, because each iteration has its own variable v, the one captured by f in the first iteration will continue to hold the value 7, which is what will be printed. (Note: earlier versions of C# declared v outside of the while loop.)

The Microsoft Visual Basic Language Specification Version 11.0

10.9.3 For Each...Next Statements (Annotation)

There is a slight change in behavior between version 10.0 and 11.0 of the language. Prior to 11.0, a fresh iteration variable was not created for each iteration of the loop. This difference is observable only if the iteration variable is captured by a lambda or a LINQ expression which is then invoked after the loop.

Dim lambdas As New List(Of Action)
For Each x In {1,2,3}
   lambdas.Add(Sub() Console.WriteLine(x)
Next
lambdas(0).Invoke()
lambdas(1).Invoke()
lambdas(2).Invoke()

Up to Visual Basic 10.0, this produced a warning at compile-time and printed "3" three times. That was because there was only a single variable "x" shared by all iterations of the loop, and all three lambdas captured the same "x", and by the time the lambdas were executed it then held the number 3. As of Visual Basic 11.0, it prints "1, 2, 3". That is because each lambda captures a different variable "x".