Implement hashCode on objects stored in a set or used as map keys.

dart/sdk/lib/_internal/compiler/implementation/ssa/value_range_analyzer.dart

Index: dart/sdk/lib/_internal/compiler/implementation/ssa/value_range_analyzer.dart
diff --git a/dart/sdk/lib/_internal/compiler/implementation/ssa/value_range_analyzer.dart b/dart/sdk/lib/_internal/compiler/implementation/ssa/value_range_analyzer.dart
index e067089bddd42b2f8f5579d733069b16fd334606..42560e0f6dc6840c5e7307ae084fc1cadd550a04 100644
--- a/dart/sdk/lib/_internal/compiler/implementation/ssa/value_range_analyzer.dart
+++ b/dart/sdk/lib/_internal/compiler/implementation/ssa/value_range_analyzer.dart
@@ -148,6 +148,8 @@ class IntValue extends Value {
     return this.value == other.value;
   }
 
+  int get hashCode => throw new UnsupportedError('IntValue.hashCode');

[ngeoffray, 2013/06/24 19:43:10]

Changes in this file tend to show that it's not so nice to have a warning that
you haven't implemented hashCode..... In this file, operator== is like any other
relational operator. The intent is not to put them in data structure that rely
on hashCode. I agree it's defensive programming then, but maybe just add it to
the root of this class hierarchy, to avoid seeing these duplicated
UnsupportedError.

[ahe, 2013/06/24 20:18:54]

You'd still get a warning.  The rule is simple because it is the only rule that
works in general: when you override operator==, you have to override hashCode. 
It is actually quite hard to implement operator== and hashCode, and if you don't
keep it simple you quickly get in trouble.  I look forward to discussing
operator==/hashCode with rest of you tomorrow, I have a bunch of concrete
suggestions and some general advice.  I should also try to to write it up for
dartlang.org.

[sra1, 2013/06/25 03:33:38]

I don't like the changes in this file.  There is no simple rule for this problem
that is both accurate and comfortable for the working programmer.  This kind of
invariant should be checked by a static analysis tool that has some
intelligence.

Tools I have worked with can take a pattern like the following and construct an
example if the exception can be reached.

checkHashCode(a, b) {
  if (a == b) {
    if (a.hashCode != b.hashCode) {
      throw ValidationError('hashCode different for equal objects', a, b);
    }
  }
}

A hashCode implementation, inherited or not, that throws an exception or returns
a constant will always be trivially validated.

My high level point is that we don't have to solve all problems with simple
rules.  The platform will be better if we use the hammer for nails and wait
until we have a scalpel before trying to solve more delicate problems.

[ahe, 2013/06/25 05:24:58]

I find it peculiar that you would use this particular file as an example of a
valid use: all the classes here are broken as they don't override hashCode.  The
only reason that dart2js isn't broken as well is that these objects are not put
in a set or used as keys in a hashMap.

It is possible that you can come up with a smarter tool, but I highly doubt it
would be worth the effort. It is not the case that throwing an exception is a
valid hashCode implementation, it just prevents us from accidentally using these
objects as keys. In the end, the result is going to be the same: in virtually
all real situations, you have to implement hashCode when you override
operator==.

On 2013/06/25 03:33:38, sra1 wrote:

+
   String toString() => 'IntValue $value';
   bool get isNegative => value < 0;
   bool get isPositive => value >= 0;
@@ -214,6 +216,8 @@ class InstructionValue extends Value {
     return this.instruction == other.instruction;
   }
 
+  int get hashCode => throw new UnsupportedError('InstructionValue.hashCode');
+
   Value operator +(Value other) {
     if (other.isZero) return this;
     if (other is IntValue) {
@@ -283,6 +287,8 @@ class AddValue extends BinaryOperationValue {
       || (left == other.right && right == other.left);
   }
 
+  int get hashCode => throw new UnsupportedError('AddValue.hashCode');
+
   Value operator -() => -left - right;
 
   Value operator +(Value other) {
@@ -328,6 +334,8 @@ class SubtractValue extends BinaryOperationValue {
     return left == other.left && right == other.right;
   }
 
+  int get hashCode => throw new UnsupportedError('SubtractValue.hashCode');
+
   Value operator -() => right - left;
 
   Value operator +(Value other) {
@@ -374,6 +382,8 @@ class NegateValue extends Value {
     return value == other.value;
   }
 
+  int get hashCode => throw new UnsupportedError('Negate.hashCode');
+
   Value operator +(other) {
     if (other.isZero) return this;
     if (other == value) return info.intZero;
@@ -506,6 +516,8 @@ class Range {
     return other.lower == lower && other.upper == upper;
   }
 
+  int get hashCode => throw new UnsupportedError('Range.hashCode');
+
   bool operator <(Range other) {
     return upper != other.lower && upper.min(other.lower) == upper;
   }