Spec is ambiguous about class hierarchy of enums; leads to observable differences in front end and analyzer behavior.

[stereotype441]

(This came up organically in a project I'm working on with @sobrienpdx. I've reduced it down to a minimal repro.)

The following code is accepted by the analyzer but rejected by the front end:

abstract class A {
  String get s;
}

abstract class B implements A {}

enum E1 implements B {
  e1;

  @override
  String get s => 'E1';
}

enum E2 implements B {
  e2;

  @override
  String get s => 'E2';
}

final allValues = [...E1.values, ...E2.values];

main() {
  for (var value in allValues) {
    print(value.s);
  }
}

The front end issues this error message:

Error: The getter 's' isn't defined for the class 'Object'.
 - 'Object' is from 'dart:core'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 's'.
    print(value.s);
                ^

After some digging, I figured out that the difference comes down to a subtle interaction between least upper bound and an ambiguity in the enhanced enums spec.

The spec doesn't fully pin down what the class hierarchy of an enum should look like. It says:

The specification here does not specify how the index and name of an enum is associated with the enum instances. In practice it’s possible to desugar an enum declaration to a class declaration, as long as the desugaring can access private names from other libraries (dart:core in particular).

The existing enums are implemented as desugaring into a class extending a private _Enum class which holds the final int index; declaration and a final String _name; declaration (used by the the EnumName.name getter), and both fields are initialized by a constructor.

In practice, the implementation of the enhanced enums will likely be something similar.

Either first declare Enum as:

abstract class Enum {
  Enum._(this.index, this._name);  
  final int index;
  final String _name;
  String _$enumToString();
  String toString() => _$enumToString();
}

or retain the current _Enum class and make that the actual superclass of enum classes. Either works, I’ll use Enum as the superclass directly in the following.

What was implemented was:

• In the analyzer, all enums extend Enum, which extends Object.
• In the front end, all enums extend _Enum, which in turn implements Enum, which extends Object.

This means that, when analyzing the code example above, the analyzer considers the class hierarchy to be:

graph BT
  B --> A --> Object
  Enum --> Object
  E1 --> B
  E1 --> Enum
  E2 --> B
  E2 --> Enum

Loading

Whereas the front end considers the class hierarchy to be:

graph BT
  B --> A --> Object
  _Enum --> Enum --> Object
  E1 --> B
  E1 --> _Enum
  E2 --> B
  E2 --> _Enum

Loading

The inferred type of allValues is List<LUB(E1, E2)>. LUB takes the class that's at the greatest unique depth in the hierarchy, so that means in the analyzer case the LUB is B, whereas in the front end case the LUB is Object.

I think we should probably change the analyzer to match the front end behavior, and adjust the spec to say that all enums extend a private _Enum class in dart:core. (It's not necessary to specify that _Enum implements Enum, because both the analyzer and the front end can see this by analyzing the core library).

[lrhn]

Yep, this is dart-lang/sdk#51730, aka #3290, again.

My long term proposal is to not count inaccessible declarations in the depth of a type when used for LUB. That means assigning a library to each LUB operation, but I think we can safely do that, since UP is only invoked where specific syntax triggers it, and that syntax's library can be used.

That's probably breaking and would have to be language versioned, and it's probably a non-trivial migration. (Well, semi-trivial, every place where an UP would give a different result, we insert an as PreviousUPResult on one or both of the brances. It's just not pretty.)

I firmly believe that will be give more consistent and possibly predictable results (inserting a private superclass in the type hierarchy is no longer a breaking change), but it will also likely cause more types to end up having the same depth, which worsens the output of Up.
(The very-long-term solution is to stop using depths of types for anything, since it's a meaningless and accidental property of a type, and have an actually useful LUB.)

A local-change-only option is to make _Enum a supertype of Enum instead of a subtype. Or unrelated, and we just happen to make every enum implement both. That'll change the results of UP, but not avoid conflicts. It's just potentially other conflicts.

[stereotype441]

We decided in today's language team meeting that we would make the two implementations consistent (following the front end's behavior) and update the spec to make this clear. I'll take this task.

[eernstg]

Closing: The discrepancy will be eliminated by the specification change in #3671 and the associated implementation.