Analyzer/CFE discrepancy on for-in loops

[johnniwinther]

The analyzer and CFE don't agree:

class Foo {}

void method1(List<Foo> list) {
	for (Foo Foo in list) { // Error in CFE, no error in analyzer
	   Foo.toString();
	}
}

@eernstg @lrhn What is the correct response?

[eernstg]

I know you don't like syntactic definitions. ;-)

However, according to the specification of for-in statements, the loop above should be treated as the following:

List<foo> id1 = list;
var id2 = id1.iterator;
while (id2.moveNext()) {
  Foo Foo = id2.current; // Error, `Foo` is not a type (that's the first `Foo`).
  {
    Foo.toString(); // OK, `Foo` is a local variable with an invalid type.
  }
}

If we want to introduce a different specification of for-in statements that does not rely on a syntactic transformation then we could consider how the properties can be preserved (it doesn't sound like a really good reason to break a lot of code that we "just don't like syntactic sugar").

The desugaring above does not correspond to a situation where the iteration variable Foo is in scope in the parenthesis after for (in particular, it cannot be used in list, which is surely not a healthy thing to allow anyway). It also doesn't correspond precisely to the situation where it is in scope in the body (similarly to a formal parameter of a function), because that would eliminate the name clash and hence the error. (However, I don't think anything valuable is lost because Foo Foo is an error as an iteration variable in a for-in statement.)

So we'd at least need to think about how to specify the scoping if we want to eliminate the sugar based specification (and the resulting specification should, preferably, not just be a more fluffy text that seems to avoid certain problems because nobody really knows what it means).

[lrhn]

Can't say what the spec says exactly, if anything specific.

If going by desugaring, then there will be a variable declaration Foo Foo = $it.current;.
Never go by a desugaring when it comes to errors, don't report errors in code the user didn't write.
Also, the desugaring might need to write an inferred type into the code, and then it can become an error even if it was just var Foo in the source. Maybe.
There is no desugaring for await for (Foo Foo in stream)..., and it should behave the same, so that's a guide.

Personally I'd prefer if it's an error.
I don't like having the same identifier occurring twice, right next to each other with no declarations or var between them, and not meaning the same thing.
(I'd also make it an error in a parameter list.)

We could move towards not having variables in scope prior to their declaration, but we haven't done that. They're generally declared throughout the entire block. Parameters are not in the block, but they have a scope where they're not allowed to have the same name twice. I'd make parameters in scope in the entire declaration they're part, and same for loop-introduced variables.

No sane programmer would be hit by this, variables should be lower case, types skulle be upper case. (But ... for (pragma pragma in pragmas) ... ?! Whoops.)

[eernstg]

Never go by a desugaring when it comes to errors, don't report errors in code the user didn't write.

Definitely! I should have mentioned that.

[johnniwinther]

cc @stereotype441