Skip to content

Commit

Permalink
Restructures hashCode() on JSType and sub-types to prevent possib…
Browse files Browse the repository at this point in the history
…le infinite recursions.

This change also centralizes some logic and vague comments about nominal type hashcodes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202002954
  • Loading branch information
nreid260 authored and brad4d committed Jun 26, 2018
1 parent 64af14a commit 2543a8f
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 33 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/rhino/jstype/AllType.java
Expand Up @@ -110,7 +110,7 @@ JSType resolveInternal(ErrorReporter reporter) {
}

@Override
public int hashCode() {
int recursionUnsafeHashCode() {
return System.identityHashCode(this);
}

Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/rhino/jstype/ArrowType.java
Expand Up @@ -220,7 +220,7 @@ boolean checkArrowEquivalenceHelper(
}

@Override
public int hashCode() {
int recursionUnsafeHashCode() {
int hashCode = Objects.hashCode(returnType);
if (parameters != null) {
Node param = parameters.getFirstChild();
Expand Down
10 changes: 2 additions & 8 deletions src/com/google/javascript/rhino/jstype/EnumElementType.java
Expand Up @@ -39,7 +39,6 @@

package com.google.javascript.rhino.jstype;

import static com.google.common.base.Preconditions.checkState;

import com.google.javascript.rhino.ErrorReporter;
import com.google.javascript.rhino.Node;
Expand Down Expand Up @@ -142,14 +141,9 @@ public boolean isNominalType() {
return hasReferenceName();
}

/**
* If this is equal to a NamedType object, its hashCode must be equal
* to the hashCode of the NamedType object.
*/
@Override
public int hashCode() {
checkState(hasReferenceName());
return getReferenceName().hashCode();
int recursionUnsafeHashCode() {
return NamedType.nominalHashCode(this);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/rhino/jstype/FunctionType.java
Expand Up @@ -919,7 +919,7 @@ final boolean checkFunctionEquivalenceHelper(
}

@Override
public int hashCode() {
int recursionUnsafeHashCode() {
int hc = kind.hashCode();
switch (kind) {
case CONSTRUCTOR:
Expand Down
Expand Up @@ -167,14 +167,10 @@ public boolean isNominalType() {
return hasReferenceName();
}

/**
* If this is equal to a NamedType object, its hashCode must be equal
* to the hashCode of the NamedType object.
*/
@Override
public int hashCode() {
int recursionUnsafeHashCode() {
if (hasReferenceName()) {
return getReferenceName().hashCode();
return NamedType.nominalHashCode(this);
} else {
return super.hashCode();
}
Expand Down
36 changes: 33 additions & 3 deletions src/com/google/javascript/rhino/jstype/JSType.java
Expand Up @@ -76,6 +76,8 @@ public abstract class JSType implements Serializable {
private JSType resolveResult = null;
protected TemplateTypeMap templateTypeMap;

private boolean hashCodeInProgress = false;

private boolean inTemplatedCheckVisit = false;
private static final CanCastToVisitor CAN_CAST_TO_VISITOR =
new CanCastToVisitor();
Expand Down Expand Up @@ -765,12 +767,40 @@ public boolean equals(Object jsType) {
return (jsType instanceof JSType) && isEquivalentTo((JSType) jsType);
}

/**
* Calculates a hash of the object as per {@link Object#hashCode()}.
*
* <p>This method is <em>unsafe</em> for multi-threaded use. The implementation mutates instance
* state to prevent recursion and therefore expects sole access.
*/
@Override
public abstract int hashCode();
public final int hashCode() {
if (hashCodeInProgress) {
return -1; // Recursive base-case.
}

this.hashCodeInProgress = true;
int hashCode = recursionUnsafeHashCode();
this.hashCodeInProgress = false;
return hashCode;
}

/**
* This predicate is used to test whether a given type can appear in a
* numeric context, such as an operand of a multiply operator.
* Calculates {@code #hashCode()} with the assumption that it will never be called recursively.
*
* <p>To work correctly this method should only called under the following conditions:
*
* <ul>
* <li>by a subclass of {@link JSType};
* <li>within the body of an override;
* <li>when delegating to a superclass implementation.
* </ul>
*/
abstract int recursionUnsafeHashCode();

/**
* This predicate is used to test whether a given type can appear in a numeric context, such as an
* operand of a multiply operator.
*/
public boolean matchesNumberContext() {
return false;
Expand Down
16 changes: 11 additions & 5 deletions src/com/google/javascript/rhino/jstype/NamedType.java
Expand Up @@ -40,6 +40,8 @@
package com.google.javascript.rhino.jstype;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Strings.emptyToNull;

import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -84,6 +86,12 @@
public final class NamedType extends ProxyObjectType {
private static final long serialVersionUID = 1L;

static int nominalHashCode(ObjectType type) {
checkState(type.hasReferenceName());
String name = checkNotNull(emptyToNull(type.getReferenceName()));
return name.hashCode();
}

private final String reference;
private final String sourceName;
private final int lineno;
Expand Down Expand Up @@ -209,13 +217,11 @@ public boolean isNominalType() {
}

@Override
public int hashCode() {
return reference.hashCode();
int recursionUnsafeHashCode() {
return nominalHashCode(this);
}

/**
* Resolve the referenced type within the enclosing scope.
*/
/** Resolve the referenced type within the enclosing scope. */
@Override
JSType resolveInternal(ErrorReporter reporter) {

Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/rhino/jstype/NoObjectType.java
Expand Up @@ -144,7 +144,7 @@ public final boolean matchesSymbolContext() {
}

@Override
public final int hashCode() {
int recursionUnsafeHashCode() {
return System.identityHashCode(this);
}

Expand Down
Expand Up @@ -555,7 +555,7 @@ public void matchRecordTypeConstraint(ObjectType constraintObj) {
}

@Override
public int hashCode() {
int recursionUnsafeHashCode() {
if (isStructuralType()) {
return Objects.hash(className, properties);
} else {
Expand Down
Expand Up @@ -290,7 +290,7 @@ public Iterable<ObjectType> getCtorExtendedInterfaces() {
}

@Override
public int hashCode() {
int recursionUnsafeHashCode() {
return referencedType.hashCode();
}

Expand Down
4 changes: 2 additions & 2 deletions src/com/google/javascript/rhino/jstype/TemplatizedType.java
Expand Up @@ -123,8 +123,8 @@ StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
}

@Override
public int hashCode() {
int baseHash = super.hashCode();
int recursionUnsafeHashCode() {
int baseHash = super.recursionUnsafeHashCode();

// TODO(b/110224889): This case can probably be removed if `equals()` is updated.
if (isSpecializedOnlyWithUnknown) {
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/rhino/jstype/UnionType.java
Expand Up @@ -508,7 +508,7 @@ public HasPropertyKind getPropertyKind(String pname, boolean autobox) {
}

@Override
public int hashCode() {
int recursionUnsafeHashCode() {
return this.hashcode;
}

Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/rhino/jstype/UnknownType.java
Expand Up @@ -176,7 +176,7 @@ JSType resolveInternal(ErrorReporter reporter) {
}

@Override
public int hashCode() {
int recursionUnsafeHashCode() {
return System.identityHashCode(this);
}
}
3 changes: 2 additions & 1 deletion src/com/google/javascript/rhino/jstype/ValueType.java
Expand Up @@ -77,7 +77,8 @@ public HasPropertyKind getPropertyKind(String propertyName, boolean autobox) {
}

@Override
public final int hashCode() {
final int recursionUnsafeHashCode() {
// Subclasses of this type are unique within a JSTypeRegisty.
return System.identityHashCode(this);
}
}

0 comments on commit 2543a8f

Please sign in to comment.