Skip to content

Commit

Permalink
Improve ArrowType.hashCode and FunctionType.hashCode to make hash col…
Browse files Browse the repository at this point in the history
…lisions less likely.

For ArrowType:
The previous version simply summed all the parameter hashcodes, but since + is commutative this means that function(a,b){} and function(b,a){} would have the same hashCode.

Also it included the 'returnTypeInferred' field in the hashCode though it was not part of equals().  This has been fixed.

For FunctionType:
All functions with the same ArrowType would get the same hashcode.  To resolve this we instead just hash more data.

Doing this revealed a bug in the equals() method for FunctionType where it was assymmetric and allowed constructors to be equal to ordinary functions in some circumstances.  This has been also been fixed.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=141193824
  • Loading branch information
lukesandberg authored and blickly committed Dec 7, 2016
1 parent 24d4f1a commit 666650c
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 21 deletions.
7 changes: 1 addition & 6 deletions src/com/google/javascript/rhino/jstype/ArrowType.java
Expand Up @@ -224,16 +224,11 @@ public int hashCode() {
if (returnType != null) {
hashCode += returnType.hashCode();
}
if (returnTypeInferred) {
hashCode += 1;
}
if (parameters != null) {
Node param = parameters.getFirstChild();
while (param != null) {
JSType paramType = param.getJSType();
if (paramType != null) {
hashCode += paramType.hashCode();
}
hashCode = hashCode * 31 + (paramType == null ? 0 : paramType.hashCode());
param = param.getNext();
}
}
Expand Down
41 changes: 26 additions & 15 deletions src/com/google/javascript/rhino/jstype/FunctionType.java
Expand Up @@ -956,29 +956,40 @@ public ObjectType getTopMostDefiningType(String propertyName) {
*/
boolean checkFunctionEquivalenceHelper(
FunctionType that, EquivalenceMethod eqMethod, EqCache eqCache) {
if (isConstructor()) {
if (that.isConstructor()) {
return this == that;
}
return false;
if (this == that) {
return true;
}
if (isInterface()) {
if (that.isInterface()) {
return getReferenceName().equals(that.getReferenceName());
}
if (kind != that.kind) {
return false;
}
if (that.isInterface()) {
return false;
switch (kind) {
case CONSTRUCTOR:
return false; // constructors use identity semantics, which we already checked for above.
case INTERFACE:
return getReferenceName().equals(that.getReferenceName());
case ORDINARY:
return typeOfThis.checkEquivalenceHelper(that.typeOfThis, eqMethod, eqCache)
&& call.checkArrowEquivalenceHelper(that.call, eqMethod, eqCache);
default:
throw new AssertionError();
}

return typeOfThis.checkEquivalenceHelper(that.typeOfThis, eqMethod, eqCache) &&
call.checkArrowEquivalenceHelper(that.call, eqMethod, eqCache);
}

@Override
public int hashCode() {
return isInterface() ? getReferenceName().hashCode() : call.hashCode();
int hc = kind.hashCode();
switch (kind) {
case CONSTRUCTOR:
return 31 * hc + System.identityHashCode(this); // constructors use identity semantics
case INTERFACE:
return 31 * hc + getReferenceName().hashCode();
case ORDINARY:
hc = 31 * hc + typeOfThis.hashCode();
hc = 31 * hc + call.hashCode();
return hc;
default:
throw new AssertionError();
}
}

public boolean hasEqualCallType(FunctionType otherType) {
Expand Down

0 comments on commit 666650c

Please sign in to comment.