Skip to content

Commit

Permalink
Rollback "Restructures hashCode() on JSType and sub-types to prev…
Browse files Browse the repository at this point in the history
…ent possible infinite recursions."

Looks to cause NPE in NamedType.nominalHashCode

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202029838
  • Loading branch information
blickly authored and brad4d committed Jun 26, 2018
1 parent a5db18c commit af93bc3
Show file tree
Hide file tree
Showing 14 changed files with 33 additions and 60 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
int recursionUnsafeHashCode() {
public int hashCode() {
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
int recursionUnsafeHashCode() {
public int hashCode() {
int hashCode = Objects.hashCode(returnType);
if (parameters != null) {
Node param = parameters.getFirstChild();
Expand Down
10 changes: 8 additions & 2 deletions src/com/google/javascript/rhino/jstype/EnumElementType.java
Expand Up @@ -39,6 +39,7 @@

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 @@ -141,9 +142,14 @@ 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
int recursionUnsafeHashCode() {
return NamedType.nominalHashCode(this);
public int hashCode() {
checkState(hasReferenceName());
return getReferenceName().hashCode();
}

@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
int recursionUnsafeHashCode() {
public int hashCode() {
int hc = kind.hashCode();
switch (kind) {
case CONSTRUCTOR:
Expand Down
Expand Up @@ -167,10 +167,14 @@ 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
int recursionUnsafeHashCode() {
public int hashCode() {
if (hasReferenceName()) {
return NamedType.nominalHashCode(this);
return getReferenceName().hashCode();
} else {
return super.hashCode();
}
Expand Down
36 changes: 3 additions & 33 deletions src/com/google/javascript/rhino/jstype/JSType.java
Expand Up @@ -76,8 +76,6 @@ 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 @@ -775,40 +773,12 @@ 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 final int hashCode() {
if (hashCodeInProgress) {
return -1; // Recursive base-case.
}

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

/**
* 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.
* 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: 5 additions & 11 deletions src/com/google/javascript/rhino/jstype/NamedType.java
Expand Up @@ -40,8 +40,6 @@
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 @@ -86,12 +84,6 @@
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 @@ -217,11 +209,13 @@ public boolean isNominalType() {
}

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

/** 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
int recursionUnsafeHashCode() {
public final int hashCode() {
return System.identityHashCode(this);
}

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

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

@Override
int recursionUnsafeHashCode() {
public int hashCode() {
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
int recursionUnsafeHashCode() {
int baseHash = super.recursionUnsafeHashCode();
public int hashCode() {
int baseHash = super.hashCode();

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

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

0 comments on commit af93bc3

Please sign in to comment.