Invalid error using Null type as an iterable in a for-in

[MichaelRFairhurst]

Null x;
for(var i in x) {...}

CFE accepts this, analyzer rejects it. My understanding is that it should be statically valid code.

Post-nnbd, this won't be, but we need to be careful that this might(?) need to work @leafpetersen to confirm.

Never x;
for(var i in x) {...}

[lrhn]

I'm not sure this is valid code now.

The specifcation was changed recently to require the iterated object to be an Iterable. The spec now says:

\begin{normativeDartCode}
$T$ $\id_1$ = $e$;
\VAR{} $\id_2$ = $id_1$.iterator;
\WHILE{} ($\id_2$.moveNext()) \{
\ \ $D$ \id{} = $\id_2$.current;
\ \ \{ $S$ \}
\}
\end{normativeDartCode}

\noindent
If the static type of $e$ is a top type
(\ref{superBoundedTypes})
then $T$ is \code{Iterable<dynamic>},
otherwise $T$ is the static type of $e$.
It is a compile-time error if $T$ is not assignable to \code{Iterable<dynamic>}.

In short, the for-in loop is equivalent to code starting with:

Null id_1 = null;
var id_2 = id_1.iterator;

This is not type correct, so the program is not valid. The analyzer matches the specification (the specification might have been changed to match the analyzer, but we did it on purpose).

For Never, I'm not sure whether we'll make it an error to have unreachable code, the example above is definitely invalid because it fails to initialize a non-nullable variable (Never is not nullable) before it's used. If you had:

Never x = throw "Banana";
for (var y in x) ...

then I guess the same problem would occur when you try to do id_1.identifier where id_1 has type Never. (Unless we decide to allow member access on Never and just give it type Never again).

[MichaelRFairhurst]

I don't think the runtime behavior leads to clear static rules, and that both should be specified separately rather than one inferred from the other, in cases like this.

For instance, this:

It is a compile-time error if $T$ is not assignable to \code{Iterable<dynamic>}.

makes me think that there is no compile time error when $T$ is Null.

[eernstg]

The rule that It is a compile-time error if $T$ is not assignable to \code{Iterable<dynamic>} is an extra constraint, and we still have the general rule here specifying that errors in desugared code are taken into account (even though the error message should of course not refer to code that the developer has never seen).

[srawlins]

analyzer and CFE seem to both accept this still, with Never:

void f(Never x) {
  for (var i in x) {
    print(i);
  }
}

[lrhn]

Not sure that it should be an error for Never since Never is a subtype of Iterable and allows .iterator to be invoked on it.

In sound null safe code, it won't ever get there with a value implementing Null. In unsound code, it the value might be null, and that will just cause a run-time error.

Statically typing as Null is different because Null does not allow .iterator like Never does.

[srawlins]

Ok leaving analyzer as is, as this is solved for null safety; analyzer does the right thing.