dart2wasm incorrectly uses identity comparison in switches over Type in language version < 3.0

[mraleph]

cc @mkustermann @osa1 @johnniwinther @eernstg

The code below does not work when compiled with dart2wasm, but works on the VM.

// @dart=2.15

import "package:expect/expect.dart";

class C<T> {
  T v;
  C(this.v);
}

String foo(Object? v) {
  switch (v.runtimeType) {
    case C<int>: return 'int';
    case C<String>: return 'String';
    default:
      return 'unknown';
  }
}

main() {
  final arr = ["int", C(1), "String", C("")];
  for (var i = 0; i < arr.length; i += 2) {
    final t = arr[i];
    final v = arr[i+1];
    Expect.equals(t, foo(v));
  }
}

You end up with a code which uses identical() for comparison - which does not work unless types are globally canonicalized and runtimeType results are not canonicalized in dart2wasm.

It seems dart2wasm assumes that CFE will lower any switch which requires calling operator== into a sequence of if-s, but CFE does not due to some legacy bug.

Here is I think what happened here: I think we have been missing a compile time error for such switch in language versions prior to 3.0¹. Old specification for switch statement says:

It is a compile-time error unless each expression $e_j$ , $j\in 1..n$ is constant. It is a compile-time error if the value of the expressions $e_j$ , $j\in 1..n$ are not either:

• instances of the same class C, for all $j\in 1..n$, or
• instances of a class that implements int, for all $j\in 1..n$, or
• instances of a class that implements String, for all $j\in 1..n$.

In other words, all the expressions in the cases evaluate to constants of the exact same user defined class or are of certain known types. Note that the values of the expressions are known at compile time, and are independent of any type annotations.

It is a compile-time error if the class C does not have primitive equality (10.2.3).

Note that the last requirement is imposed on the class C itself and not on the constants. This is very relevant distinction in this case because if we check 10.2.3 for rules we find these two:

• Every instance of type Type which was originally obtained by evaluating
  a constant type literal (20.2) has primitive equality.
  ...
• The class Object has primitive equality.
• A class C has primitive equality if it does not have an implementation of the operator == that overrides the one inherited from Object, and it does not have an implementation of the getter hashCode that overrides the one inherited from Object.

These rules mean that int and String (as constants) have primitive equality but not class Type itself because it has an override of ==. Which in turn means that switch like the one above should have triggered a compile time error.

However neither analyzer nor CFE emit a compile time error. CFE just produces SwitchStatement here if language version is below 3.0²

dart2wasm then makes an assumption that this type of SwitchStatement can be lowered using identical(...), which leads to incorrectly behaving switch.

VM on the other hand simply builds operator== calls³ for each case in SwitchStatement - just like specification instructs us.

Footnotes

1. Pattern specification lifted restrictions on user defined equality to align classical switches with patterns. See discussions here or search for primitive equality in the patterns feature spec ↩

2. with language versions >=3.0 CFE correctly lowers this switch into sequence of ifs, because lowering correctly treats Type as a class with non-primitive equality. ↩

3. FWIW VM also canonicalizes Type objects globally so you would not be able to observe incorrect switch behavior on the VM even if VM used identical. ↩

[osa1]

It is a compile-time error if the class C does not have primitive equality (10.2.3).

Note that the last requirement is imposed on the class C itself and not on the constants

Is this important, given that the specification says each expression should be a constant?

It seems like VM is supporting what should be a compile-time error.

Should const-ness of the expressions be checked by CFE?

Is Dart 2.15 a version we want to support in dart2wasm?

[mraleph]

Is this important, given that the specification says each expression should be a constant?

Yes, it is important. See 10.2.3 - it has two different rules: one for constants and the second for classes. These rules mean that int or String (as constants) have primitive equality but class Type itself does not (it has an override of ==). I think both CFE and analyzer overlooked this distinction by mistake.

It seems like VM is supporting what should be a compile-time error.

Yes, both CFE and analyzer are missing a compile time error for this case.

Is Dart 2.15 a version we want to support in dart2wasm?

Yes, because it is a supported language version.

[mraleph]

To clarify: I think the bug is actually in CFE, not in dart2wasm. I don't think we can start emitting a compile time error here (not worth the trouble of going through breaking change), but CFE should at least change the lowering and not emit SwitchStatement for this.

[mraleph]

Any opinions @johnniwinther @eernstg ?

[eernstg]

Agreed, the switch in the OP should have been reported as a compile-time error because the class Type as such does not have primitive equality.

However, the class C which is mentioned is the common type of the values of cases, not the static type of the scrutinee. If we consider (just as a rhetorical device, with no implementation implications) the constant type literals to have their own type (an epsilon subtype of Type) then it would consist exclusively of objects that are specified to have primitive equality. It's very tempting to say that this type does have primitive equality.

(This is not the same thing as inheriting == from Object: For example, an implementation that doesn't canonicalize every string value (if that's even possible), the type String needs to have a specialized == such that it can compare the contents of the strings.)

I also suspect that switches on type literals have been used rather widely (perhaps very unevenly: many teams would never use it, a few would use it rather frequently). This could be taken as a strong hint that it would be helpful to change the behavior of dart2wasm such that it uses ==, just like the VM.