discrepency between CFE and analyzer's operator == override

[srawlins]

See this example from @yanok (dartpad):

abstract class A {
  @override
  bool operator ==(dynamic other);
}

class B implements A {}

void test(dynamic x, dynamic y, dynamic z) {
  print(x == y || x == z);
}

void main() {
  var b1 = B(), b2 = B();
  B? b3;
  print(b1 == null);
  print(b1 == b2);
  print(b3 == b2);
  print(b3 == null);
  test(b1, b2, b3);
}

This program runs without error, but analyzer reports an invalid_implementation_override error at line 6 (class B implements A {}):

'Object.==' ('bool Function(Object)') isn't a valid concrete implementation of 'A.==' ('bool Function(dynamic)').

There is either a bug in analyzer or CFE.

cc @scheglov

[scheglov]

The analyzer implements what is written in the specification.

Specifically "F must then be a subtype of F′."
And bool Function(Object) is not a subtype of bool Function(dynamic) because dynamic is not a subtype of Object.
It accepts for example null, which is never passed to operator==, but the specification does not make an exception.
@eernstg

[eernstg]

Agreed, the operator == of B is inherited from Object and has function type bool Function(Object), and that is not a correct override of the signature in A which has function type bool Function(dynamic). This makes the declaration of B an error, because it does not have an implementation of every member in its interface (at least not one with the required signature).

The section from the language specification is pre-nnbd (see dart-lang/language#2605 for the proposed updates to include null safety). The main difference here is that covariant parameters are given the type Object? rather than Object, but that doesn't matter for this case.

The signature of == was treated in a special way during the transition from legacy to null safety. @johnniwinter, could that be the reason why the CFE does not report an error here?

[johnniwinther]

Yes. The ability to allow the legacy handling of the == argument, required a lax handling of this case. As far as I remember, because we can't (couldn't?) tell whether the dynamic parameter was user-provided or from untyped legacy code.

[eernstg]

It sounds like we can decide that the current behaviors of the CFE and the analyzer are OK. There is a slight inconsistency, but it will go away when the support for legacy code is removed. Moreover, there is no soundness issue as far as I can see:

As specified, operator == is never invoked with the argument null (even super == is subject to the special null checks before the method is called), so there's no way an invocation of an operator == (like the one in Object, inherited by B), requiring an argument of type Object, can be invoked with the argument null. So it doesn't matter that the type B seems to promise that we can pass null as the argument. Also, there is no way to tear off operator ==.

[eernstg]

@sigmundch, I believe the web compilers may treat operator == differently: They do not generate code to perform the null checks at the call site; instead, they generate code at the beginning of the body of operator == to check for null and return true/false as appropriate. Is that correctly described? Do you have any worries about keeping the current CFE behavior (where there is no compile-time error in the situation where a == signature with type bool Function(dynamic) is implemented by an implementation with type bool Function(Object))?

[sigmundch]

Your understanding is correct @eernstg - dart2js moves the null check into the operator implementation to optimize for performance/code-size. That said, this is done behind the scenes and we would like it to not be affected by the semantics from the CFE.

Until recently, we actually relied on the CFE behavior to not complain on bool Function(dynamic), but this recently triggered issues internally (which is what prompted this bug to be filed, see also b/259134430 internally). We landed https://dart-review.googlesource.com/c/sdk/+/273462 to remove our subtle dependency on this implementation discrepancy, and we believe now we can work with either the Analyzer or the CFE behavior going forward.

[eernstg]

So here's a possible conclusion: The CFE continues to have the special treatment of operator == for now. At some point in the future, when no support for legacy code is needed, the CFE is free to remove the exception (and make it an error to override == with function type bool Function(dynamic) by a declaration with function type bool Function(Object), as it is everywhere else).

Please comment if you see a problem with this.