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

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

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of ssa;
 
 
 class ValueRangeInfo {
   final ConstantSystem constantSystem;
 
@@ skipping 130 matching lines @@
   Value max(other) {
     if (other is !IntValue) return other.max(this);
     return this.value < other.value ? other : this;
   }
 
   bool operator ==(other) {
     if (other is !IntValue) return false;
     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;
   bool get isZero => value == 0;
 }
 
 /**
  * The [MaxIntValue] represents the maximum value an integer can have,
  * which is currently +infinity.
  */
@@ skipping 46 matching lines @@
  */
 class InstructionValue extends Value {
   final HInstruction instruction;
   InstructionValue(this.instruction, info) : super(info);
 
   bool operator ==(other) {
     if (other is !InstructionValue) return false;
     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) {
       if (other.isNegative) {
         return info.newSubtractValue(this, -other);
       }
       return info.newAddValue(this, other);
     }
     if (other is InstructionValue) {
       return info.newAddValue(this, other);
@@ skipping 49 matching lines @@
 
 class AddValue extends BinaryOperationValue {
   AddValue(left, right, info) : super(left, right, info);
 
   bool operator ==(other) {
     if (other is !AddValue) return false;
     return (left == other.left && right == other.right)
       || (left == other.right && right == other.left);
   }
 
+  int get hashCode => throw new UnsupportedError('AddValue.hashCode');
+
   Value operator -() => -left - right;
 
   Value operator +(Value other) {
     if (other.isZero) return this;
     Value value = left + other;
     if (value != const UnknownValue() && value is! BinaryOperationValue) {
       return value + right;
     }
     // If the result is not simple enough, we try the same approach
     // with [right].
@@ skipping 25 matching lines @@
 }
 
 class SubtractValue extends BinaryOperationValue {
   SubtractValue(left, right, info) : super(left, right, info);
 
   bool operator ==(other) {
     if (other is !SubtractValue) return false;
     return left == other.left && right == other.right;
   }
 
+  int get hashCode => throw new UnsupportedError('SubtractValue.hashCode');
+
   Value operator -() => right - left;
 
   Value operator +(Value other) {
     if (other.isZero) return this;
     Value value = left + other;
     if (value != const UnknownValue() && value is! BinaryOperationValue) {
       return value - right;
     }
     // If the result is not simple enough, we try the same approach
     // with [right].
@@ skipping 26 matching lines @@
 
 class NegateValue extends Value {
   final Value value;
   NegateValue(this.value, info) : super(info);
 
   bool operator ==(other) {
     if (other is !NegateValue) return false;
     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;
     if (other is NegateValue) return this - other.value;
     if (other is IntValue) {
       if (other.isNegative) {
         return info.newSubtractValue(this, -other);
       }
       return info.newSubtractValue(other, value);
     }
@@ skipping 112 matching lines @@
     } else {
       return info.newUnboundRange();
     }
   }
 
   bool operator ==(other) {
     if (other is! Range) return false;
     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;
   }
 
   bool operator >(Range other) {
     return lower != other.upper && lower.max(other.upper) == lower;
   }
 
   bool operator <=(Range other) {
     return upper.min(other.lower) == upper;
@@ skipping 467 matching lines @@
     if (instruction is HPhi && !instruction.block.isLoopHeader()) {
       HInstruction result = unwrap(instruction.inputs[0]);
       for (int i = 1; i < instruction.inputs.length; i++) {
         if (result != unwrap(instruction.inputs[i])) return instruction;
       }
       return result;
     }
     return instruction;
   }
 }