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

Less boilerplate for operator == #36004

Open
Hixie opened this issue Feb 22, 2019 · 8 comments
Open

Less boilerplate for operator == #36004

Hixie opened this issue Feb 22, 2019 · 8 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).

Comments

@Hixie
Copy link
Contributor

Hixie commented Feb 22, 2019

Our current best-practice for operator == is:

  @override
  bool operator ==(Object other) {
    if (other.runtimeType != runtimeType)
      return false;
    final Foo typedOther = other;
    return typedOther.bar == bar
        && typedOther.baz == baz
        && typedOther.quux == quux;
  }

It's unfortunate that we have to introduce a new variable here. It would be nice if this worked:

  @override
  bool operator ==(Object other) {
    if (other.runtimeType != runtimeType)
      return false;
    assert(other is Foo); // compiler promotes other to Foo
    return other.bar == bar
        && other.baz == baz
        && other.quux == quux;
  }

...or maybe with new syntax:

  @override
  bool operator ==(Object other) {
    if (other.runtimeType != runtimeType)
      return false;
    promote other as Foo; // equivalent of: if (other is! Foo) throw TypeError();
    return other.bar == bar
        && other.baz == baz
        && other.quux == quux;
  }

...or even:

  @override
  bool operator ==(Object other) {
    if (other.runtimeType != runtimeType) // compiler knows this being false means other is Foo
      return false;
    return other.bar == bar
        && other.baz == baz
        && other.quux == quux;
  }

See also:

@eernstg
Copy link
Member

eernstg commented Feb 22, 2019

I understand that the focus is on equality here, but promotion seems to be an important topic as well.

For that, note that #25565 (comment) lists a number of other issues targeting promotion improvements, and #25565 is in general a good hub for keeping discussions on that topic together.

In the language team we have discussed using an expression statement like other as Foo; to promote other to Foo, and that idea is certainly relevant to the discussion in #25565 (and to proposals like this one).

The last example using other.runtimeType != runtimeType is a smart trick, and it looks really attractive that we might get rid of the type test altogether! ;-)

However, the test other.runtimeType != runtimeType cannot soundly show anything about the type of other, because any class (both that of this and that of other) could override runtimeType to return whatever it wants. We could just ignore soundness and trust the result (and generate dynamic checks to ensure that the heap remains sound at run time), but that would be inconsistent with all recent developments in the definition of Dart. We could make it safe by introducing a Type.runtimeTypeOf(Object) static method, which would be guaranteed to return the actual run-time type of its argument. But even with that, it would still be a somewhat odd and ad-hoc promotion, because subtypes of the run-time type of this would be rejected, even though they would succeed at an as Foo or is Foo test.

@kevmoo kevmoo added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Feb 27, 2019
@Hixie
Copy link
Contributor Author

Hixie commented Mar 5, 2019

We could also just ban overrides of runtimeType. Combine this with proposals elsewhere to add post-fix "fake methods" like .as, and define .runtimeType as one of these fake methods. Or combine it with the proposal to introduce sealed methods, and seal it. Or just special case runtimeType.

@natebosch
Copy link
Member

Can we update the first post from "Our current best-practice" to "Flutter's current best-practice"? Using runtimeType for equality checks is not a universal best practice.

I do think it would be worthwhile to explore disallowing overrides of runtimeType.

@matanlurey
Copy link
Contributor

Going further, Flutter SDK's best practice. Most Flutter users do not use this pattern.

@leecommamichael
Copy link

leecommamichael commented Nov 25, 2019

Would it not be valid to write the following? Short-circuiting at the type-check seems to protect against illegal accesses.

@override
  bool operator ==(other) => 
        other is ThisClass 
        && other.bar == bar
        && other.baz == baz
        && other.quux == quux;

As a Swift developer, I love that the Swift compiler synthesizes Hashable and Equatable for simple structs. Seeing that in Dart would be great (and while I'm at it, the first-party Codable support).

@Hixie
Copy link
Contributor Author

Hixie commented Nov 25, 2019

Using is in operator == is usually wrong as it makes the == operator asymmetric.

@leecommamichael
Copy link

leecommamichael commented Nov 25, 2019

Using is in operator == is usually wrong as it makes the == operator asymmetric.

Was my example asymmetric? I think this is the kind of code most people will want to write, now I'm doubting it. :)

@Hixie
Copy link
Contributor Author

Hixie commented Nov 25, 2019

consider

class ThatClass extends ThisClass {
  ThatClass(this.bob, Bar bar, Baz baz, Quux quux) : super(bar, baz, quux);
  final Bob bob;
  operator ==(other) =>
    other is ThatClass
    && other.bob == bob
    && super==(other);
}
// ...
print(ThatClass(a, b, c, d) == ThisClass(b, c, d)); // false, good
print(ThisClass(b, c, d) == ThatClass(a, b, c, d)); // true, oops

micimize added a commit to micimize/linter that referenced this issue Jan 8, 2020
> Using `is` in `operator ==` is usually wrong as it makes the `==` operator asymmetric.
dart-lang/sdk#36004 (comment)
pq pushed a commit to dart-lang/linter that referenced this issue Aug 24, 2021
> Using `is` in `operator ==` is usually wrong as it makes the `==` operator asymmetric.
dart-lang/sdk#36004 (comment)
copybara-service bot pushed a commit that referenced this issue Aug 23, 2023
> Using `is` in `operator ==` is usually wrong as it makes the `==` operator asymmetric.
#36004 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).
Projects
None yet
Development

No branches or pull requests

6 participants