Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: add a context for RHS of equality operations. #3653

Open
stereotype441 opened this issue Mar 12, 2024 · 1 comment
Open

Proposal: add a context for RHS of equality operations. #3653

stereotype441 opened this issue Mar 12, 2024 · 1 comment
Labels
feature Proposed language feature that solves one or more problems

Comments

@stereotype441
Copy link
Member

The analyzer and front end currently use the following rules to perform type inference for equality expressions (expressions of the form e1 op e2, where op is either == or !=):

  • First, e1 is type inferred in context _, producing m1 with static type T1.
  • Let K be static type of the single argument accepted by T1.operator==.
  • Then, e2 is type inferred in context _, producing m2 with static type T2.
  • Define m as follows:
    • If T2 <: K?, let m be m1 op m2.
    • Otherwise, if a coercion C exists that coerces type T2 to T2', and T2' <: K?, then let m be m1 op C(m2).
    • Otherwise, it is a compile-time error.
  • Then, the result of type inferring e1 op e2 is m, with static type bool.

It seems odd to me that the type K? is used for coercions but not to supply a context when type inferring e2. This matters if a user decides to declare an operator== with a covariant argument type. For example:

class ComparableList<T> {
  final List<T> _values;
  ComparableList(this._values);

  bool operator==(covariant ComparableList<T> other) {
    if (_values.length != other._values.length) return false;
    for (var i = 0; i < _values.length; i++) {
      if (_values[i] != other._values[i]) return false;
    }
    return true;
  }
}

f(ComparableList<double> doubles) => doubles == ComparableList([0]);

main() {}

This code is rejected by both the analyzer and front end, with the error message:

The argument type 'ComparableList<int>' can't be assigned to the parameter type 'ComparableList<double>?'.

However, if C.operator== is replaced with any other user definable operator, then the code is accepted:

class ComparableList<T> {
  final List<T> _values;
  ComparableList(this._values);

  bool operator+(covariant ComparableList<T> other) {
    if (_values.length != other._values.length) return false;
    for (var i = 0; i < _values.length; i++) {
      if (_values[i] != other._values[i]) return false;
    }
    return true;
  }
}

f(ComparableList<double> doubles) => doubles + ComparableList([0]);

main() {}

This seems unnecessarily inconsistent. I think we should change the third bullet in the type inference rules to be:

  • Then, e2 is type inferred in context K?, producing m2 with static type T2.

This would make type inference for operator == more consistent with other operators.

@lrhn
Copy link
Member

lrhn commented Mar 13, 2024

Agree.
We do allow overriding operator==, so we should also use the overridden type in the minuscule amount of cases where someone does that. (But… like why? And like don't!)

The other alternative is to remove coercion as well, but this is a statically decided behavior, so it's cost-free in most cases (because K? will be Object? in almost every case), and in the hypothetical case where we know that calling is going to throw if we don't coerce, we might as well do the consistent thing.

(We still can't change the return type to the declared return type, even if we can now have extension types as subtypes of bool. Unless Erik's proposal of allowing extension type return types for fx async functions can be extended to operator==.)

@lrhn lrhn added the feature Proposed language feature that solves one or more problems label Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

2 participants