Closing over the loop variable considered harmful, part one

UPDATE: We are taking the breaking change. In C# 5, the loop variable of a foreach will be logically inside the loop, and therefore closures will close over a fresh copy of the variable each time. The for loop will not be changed. We return you now to our original article.


I don’t know why I haven’t blogged about this one before; this is the single most common incorrect bug report we get. That is, someone thinks they have found a bug in the compiler, but in fact the compiler is correct and their code is wrong. That’s a terrible situation for everyone; we very much wish to design a language which does not have “gotcha” features like this.

But I’m getting ahead of myself. Continue reading

Simple names are not so simple, part two

Today, the solution to the puzzle from last time. The code is correct, and compiles without issue. I was quite surprised when I first learned that; it certainly looks like it violates our rule about not using the same simple name to mean two different things in one block.

The key is to understanding why this is legal is that the query comprehensions and foreach loops are specified as syntactic sugars for another program, and it is that program which is actually analyzed for correctness. Our original program:

static void Main()
{
  int[] data = { 1, 2, 3, 1, 2, 1 };
    foreach (var m in from m in data orderby m select m)
      System.Console.Write(m);
}

is transformed into

static void Main()
{
  int[] data = { 1, 2, 3, 1, 2, 1 };
  {
    IEnumerator<int> e = 
      ((IEnumerable<int>)(data.OrderBy(m=>m)).GetEnumerator();
    try
    {
      int m; // Inside the "while" in C# 5 and above, outside in C# 1 through 4.
      while(e.MoveNext())
      {
        m = (int)(int)e.Current;
        Console.Write(m);
      }
    }
    finally
    {
      if (e != null) ((IDisposable)e).Dispose();
    }
  }
}

There are five usages of m in this transformed program; m is:

  1. declared as the formal parameter of a lambda.
  2. used in the body of the lambda; here it refers to the formal parameter.
  3. declared as a local variable
  4. written to in the loop; here it refers to the local variable
  5. read from in the loop; here it refers to the local variable

Is there any usage of a local variable before its declaration? No.

Are there any two declarations that have the same name in the same declaration space? It would appear so. The body of Main defines a local variable declaration space, and clearly the body of Main contains, indirectly, two declarations for m, one as a formal parameter and one as a local. But I said last time: local variable declaration spaces have special rules for determining overlaps. It is illegal for a local variable declaration space to directly contain a declaration such that another nested local variable declaration space contains a declaration of the same name. But an outer declaration space which indirectly contains two such declarations is not  an error. So in this case, no, there are no local variable declarations spaces which directly contain a declaration for m, such that a nested local variable declaration space also directly contains a declaration for m. Our two local variable declarations spaces which directly contain a declaration for m do not overlap anywhere.

Is there any declaration space which contains two inconsistent usages of the simple name m? Yes, again, the outer block of Main contains two inconsistent usages of m. But again, this is not relevant. The question is whether any declaration spaces directly containing a usage of m have an inconsistent usage. Again, we have two declaration spaces but they do not overlap each other, so there’s no problem here either.

The thing which makes this legal, interestingly enough, is the generation of the loop variable declaration logically within the try block. Were it to be generated outside the try block then this would be a violation of the rule about inconsistent usage of a simple name throughout a declaration space.

Simple names are not so simple, part one

C# has many rules that are designed to prevent some common sources of bugs and encourage good programming practices. So many, in fact, that it is often quite confusing to sort out exactly which rule has been violated. I thought I might spend some time talking about what the different rules are. We’ll finish up with a puzzle.

To begin with, it will be vital to understand the difference between scope and declaration space. To refresh your memory of my earlier article: the scope of an entity is the region of text in which that entity may be referred to by its unqualified name. A declaration space is a region of text in which no two things may have the same name (with an exception for methods which differ by signature.) A “local variable declaration space” is a particular kind of declaration space used for declaring local variables; local variable declaration spaces have special rules for determining when they overlap.

The next thing that you have to understand to make any sense o this is what a “simple name” is. A simple name is always either just a plain identifier, like x, or, in some cases, a plain identifier followed by a type argument list, like Frob<int, string>.

Lots of things are treated as “simple names” by the compiler: local variable declarations, lambda parameters, and so on, always have the first form of simple name in their declarations. When you say Console.WriteLine(x); the Console and x are simple names but the WriteLine is not. Confusingly, there are some textual entities which have the form of simple names, but are not treated as simple names by the compiler. We might talk about some of those situations in later fabulous adventures.

So, without further ado, here are some relevant rules which are frequently confused. It’s rules 3 and 4 that people find particularly confusing.

  1. It is illegal to refer to a local variable before its declaration. (This seems reasonable I hope.)
  2. It is illegal to have two local variables of the same name in the same local variable declaration space or nested local variable declaration spaces.
  3. Local variables are in scope throughout the entire block in which the declaration occurs. This is in contrast with C++, in which local variables are in scope in their block only at points after the declaration.
  4. For every occurrence of a simple name, whether in a declaration or as part of an expression, all uses of that simple name within the immediately enclosing local variable declaration space must refer to the same entity.

The purpose of all of these rules is to prevent the class of bugs in which the reader/maintainer of the code is tricked into believing they are referring to one entity with a simple name, but are in fact accidentally referring to another entity entirely. These rules are in particular designed to prevent nasty surprises when performing what ought to be safe refactorings.

Consider a world in which we did not have rules 3 and 4. In that world, this code would be legal:

class C
{
  int x;
  void M()
  {
    // 100 lines of code
    x = 20; // means "this.x";
    Console.WriteLine(x); // means "this.x"
    // 100 lines of code
    int x = 10;
    Console.WriteLine(x); // means "local x"
  }
}

This is hard on the person reading the code, who has a reasonable expectation that the two Console.WriteLine(x) lines do in fact both print out the contents of the same variable. But it is particularly nasty for the maintenance programmer who wishes to impose a reasonable coding standard upon this body of code. “Local variables are declared at the top of the block where they’re used” is a reasonable coding standard in a lot of shops. But changing the code to:

class C
{
  int x;
  void M()
  {
    int x;
    // 100 lines of code
    x = 20; // no longer means "this.x";
    Console.WriteLine(x); // no longer means "this.x"
    // 100 lines of code
    x = 10;
    Console.WriteLine(x); // means "local x"
  }
}

changes the meaning of the code! We wish to discourage authoring of multi-hundred-line methods, but making it harder and more error-prone to refactor them into something cleaner is not a good way to achieve that goal.

Notice that the original version of this program, rule 3 means that the program violates rule 1 — the first usage of x is treated as a reference to the local before it is declared. The fact that it violates rule 1 because of rule 3 is precisely what prevents it from being a violation of rule 4! The meaning of x is consistent throughout the block; it always means the local, and therefore is sometimes used before it is declared. If we scrapped rule 3 then this would be a violation of rule 4, because we would then have two inconsistent meanings for the simple name x within one block.

Now, these rules do not mean that you can refactor willy-nilly. We can still construct situations in which similar refactorings fail. For example:

class C
{
  int x;
  void M()
  {
    {
      // 100 lines of code
      x = 20; // means "this.x";
      Console.WriteLine(x); // means "this.x"
    }
    {
      // 100 lines of code
      int x = 10;
      Console.WriteLine(x); // means "local x"
    }
  }
}

This is perfectly legal. We have the same simple name being used two different ways in two different blocks, but the immediately enclosing block of each usage does not overlap that of any other usage. The local variable is in scope throughout its immediately enclosing block, but that block does not overlap the block above. In this case, it is safe to refactor the declaration of the local to the top of its block, but not safe to refactor the declaration to the top of the outermost block; that would change the meaning of x in the first block. Moving a declaration up is almost always a safe thing to do; moving it out is not necessarily safe.

Now that you know all that, here’s a puzzle for you, a puzzle that I got completely wrong the first time I saw it:

using System.Linq;
class Program
{
  static void Main()
  {
    int[] data = { 1, 2, 3, 1, 2, 1 };
    foreach (var m in from m in data orderby m select m)
      System.Console.Write(m);
  }
}

It certainly looks like simple name m is being used multiple times to mean different things. Is this program legal? If yes, why do the rules for not re-using simple names not apply? If no, precisely what rule has been violated?

Next time on FAIC: The solution to this puzzle.

What’s the difference between conditional compilation and the Conditional attribute?

User: Why does this program not compile correctly in the release build?

class Program 
{ 
#if DEBUG 
    static int testCounter = 0; 
#endif 
    static void Main(string[] args) 
    { 
        SomeTestMethod(testCounter++); 
    } 
    [Conditional("DEBUG")] 
    static void SomeTestMethod(int t) { } 
}

Eric: This fails to compile in the release build because testCounter cannot be found in the call to SomeTestMethod.

User: But that call site is going to be omitted anyway, so why does it matter? Clearly there’s some difference here between removing code with the conditional compilation directive versus using the conditional attribute, but what’s the difference?

Eric: You already know the answer to your question, you just don’t know it yet. Let’s get Socratic; let me turn this around and ask you how this works. How does the compiler know to remove the method call site?

User: Because the method called has the Conditional attribute on it.

Eric: You know that. But how does the compiler know that the method called has the Conditional attribute on it?

User: Because overload resolution chose that method. If this were a method from an assembly, the metadata associated with that method has the attribute. If it is a method in source code, the compiler knows that the attribute is there because the compiler can analyze the source code and figure out the meaning of the attribute.

Eric: I see. So fundamentally, overload resolution does the heavy lifting. How does overload resolution know to choose that method? Suppose hypothetically there were another method of the same name with different parameters.

User: Overload resolution works by examining the arguments to the call and comparing them to the parameter types of each candidate method and then choosing the unique best match of all the candidates.

Eric: And there you go. Therefore the arguments must be well-defined at the point of the call, even if the call is going to be removed. In fact, the call cannot be removed unless the arguments are extant! But in the release build, the type of the argument cannot be determined because its declaration has been removed.

So now you see that the real difference between these two techniques for removing unwanted code is what the compiler is doing when the removal happens. At a high level, the compiler processes a text file like this. First it “lexes” the file. That is, it breaks the string down into “tokens” — sequences of letters, numbers and symbols that are meaningful to the compiler. Then those tokens are “parsed” to make sure that the program conforms to the grammar of C#. Then the parsed state is analyzed to determine semantic information about it; what all the types are of all the expressions and so on. And finally, the compiler spits out code that implements those semantics.

The effect of a conditional compilation directive happens at lex time; anything that is inside a removed #if block is treated by the lexer as a comment. It’s like you simply deleted the whole contents of the block and replaced it with whitespace. But removal of call sites depending on conditional attributes happens at semantic analysis time; everything necessary to perform that semantic analysis must be present. 

User: Fascinating. Which parts of the C# specification define this behavior?

Eric: The specification begins with a handy “table of contents”, which is very useful for answering such questions. The table of contents states that section 2.5.1 describes “Conditional compilation symbols” and section 17.4.2 describes “The Conditional attribute”.

User: Awesome!

What’s the difference? fixed versus fixed

I got an email the other day that began:

I have a question about fixed sized buffers in C#:

unsafe struct FixedBuffer 
{ 
  public fixed int buffer[100];

Now by declaring buffer as fixed it is not movable…

And my heart sank. This is one of those deeply unfortunate times when subtle choices made in the details of language design encourage misunderstandings.

When doing pointer arithmetic in unsafe code on a managed object, you need to make sure that the garbage collector does not move the memory you’re looking at. If a collection on another thread happens while you’re doing pointer arithmetic on an object, the pointer can get all messed up. Therefore, C# classifies all variables as “fixed” or “movable”. If you want to do pointer arithmetic on a movable object, you can use the fixed keyword to say “this local variable contains data which the garbage collector should not move.” When a collection happens, the garbage collector needs to look at all the local variables for in-flight calls (because of course, stuff that is in local variables needs to stay alive); if it sees a “fixed” local variable then it makes a note to itself to not move that memory, even if that fragments the managed heap. (This is why it is important to keep stuff fixed for as little time as possible.) So typically, we use “fixed” to mean “fixed in place”.

But that’s not what “fixed” means in this context; this means “the buffer in question is fixed in size to be one hundred ints” — basically, it’s the same as generating one hundred int fields in this structure.

Obviously we often use the same keyword to mean conceptually the same thing. For example, we use the keyword internal in many ways in C#, but all of them are conceptually the same. It is only ever used to mean “accessibility to some entity is restricted to only code in this assembly”.

Sometimes we use the same keyword to mean two completely different things, and rely upon context for the user to figure out which meaning is intended. For example:

var results = from c in customers where c.City == "London" select c;

versus

class C<T> where T : IComparable<T>

It should be clear that where is being used in two completely different ways: to build a filter clause in a query, and to declare a type constraint on a generic type parameter.

We cause people to run into trouble when one keyword is used in two different ways but the difference is subtle, like our example above. The user’s email went on to ask a whole bunch of questions which were predicated on the incorrect assumption that a fixed-in-size buffer is automatically fixed in place in memory.

Now, one could say that this is just an unfortunate confluence of terms; that “fixed in size” and “fixed in place” just happen to both use the word “fixed” in two different ways, how vexing. But the connection is deeper than that: you cannot safely access the data stored in a fixed-in-size buffer unless the container of the buffer has been fixed in place. The two concepts are actually quite strongly related in this case, but not at allthe same.

On the one hand it might have been less confusing to use two keywords, say pinned and fixed. But on the other hand, both usages of fixed are only valid in unsafe code. A key assumption of all unsafe code features is that if you are willing to use unsafe code in C#, then you are already an expert programmer who fully understands memory management in the CLR. That’s why we make you write unsafe on the code; it indicates that you’re turning off the safety system and you know what you’re doing.

A considerable fraction of the keywords of C# are used in two or more ways: fixed, into, partial, out, in, new, delegate, where, using, class, struct, true, false, base, this, event, return and void all have at least two different meanings. Most of those are clear from the context, but at least the first three — fixed, into and partial — have caused enough confusion that I’ve gotten questions about the differences from perplexed users. I’ll take a look at into and partial next.

Arrays of arrays

Most C# developers understand that there’s a difference between a “rectangular” and a “ragged” two-dimensional array.

int[,] rectangle = { 
  {10, 20}, 
  {30, 40}, 
  {50, 60} }; 
int[][] ragged = { 
  new[] {10},
  new[] {20, 30},
  new[] {40, 50, 60} };

Here we have a two-dimensional array with six elements, arranged in three rows of two elements each. And we have a one-dimensional array with three elements, where each element is itself a one-dimensional array with one, two or three elements.

That’s all very straightforward. Where things get brain-destroying is when you try to make a ragged array of two-dimensional arrays.  Continue reading

Reserved and contextual keywords

Many programming languages, C# included, treat certain sequences of letters as “special”.

Some sequences are so special that they cannot be used as identifiers. Let’s call those the “reserved keywords” and the remaining special sequences we’ll call the “contextual keywords”. They are “contextual” because the character sequence might one meaning in a context where the keyword is expected and another in a context where an identifier is expected.[1. An unfortunate consequence of this definition is that using is said to be a reserved keyword even though its meaning depends on its context; whether the using begins a directive or a statement determines its meaning. And similarly for other keywords that have multiple usages, like fixed.]

The C# specification defines the following reserved keywords:

Continue reading

Locks and exceptions do not mix

A couple years ago I wrote a bit about how our codegen for the lock statement could sometimes lead to situations in which an unoptimized build had different potential deadlocks than an optimized build of the same source code. This is unfortunate, so we’ve fixed that for C# 4.0. However, all is still not rainbows, unicorns and Obama, as we’ll see.
Continue reading