Skip to content

Commit

Permalink
Always pad the resolved list of types with unknowns and remove the "a…
Browse files Browse the repository at this point in the history 
…ddUnknowns" as an expensive way to get a list padded with unknown types.

Here we distinctly do not pad the values to unknown as some of the type construction logic uses the short list to signal that type can receive template values.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=131726218
  • Loading branch information
@concavelenz @brad4d
concavelenz authored and brad4d committed Aug 30, 2016
1 parent 263595b commit 40dd127
Showing 1 changed file with 39 additions and 39 deletions.
78 changes: 39 additions & 39 deletions src/com/google/javascript/rhino/jstype/TemplateTypeMap.java
View file
Expand Up @@ -84,17 +84,17 @@ public class TemplateTypeMap implements Serializable {
TemplateTypeMapReplacer replacer =
new TemplateTypeMapReplacer(registry, this, /* replaceMissingTypesWithUnknown */ true);

// TODO: fully populate the resolvedTemplateValues *including* UNKNOWN_TYPEs, so that the
// addUnknowns() method becomes redundant.
int nValues = this.templateValues.size();
JSType[] resolvedValues = null;
if (nValues > 0) {
resolvedValues = new JSType[nValues];
for (int i = 0; i < nValues; i++) {
int nKeys = this.templateKeys.size();
JSType[] resolvedValues = new JSType[nKeys];
for (int i = 0; i < nKeys; i++) {
if (i < nValues) {
TemplateType templateKey = this.templateKeys.get(i);
replacer.setKeyType(templateKey);
JSType templateValue = this.templateValues.get(i);
resolvedValues[i] = templateValue.visit(replacer);
} else {
resolvedValues[i] = registry.getNativeType(JSTypeNative.UNKNOWN_TYPE);
}
}
this.resolvedTemplateValues = resolvedValues;
Expand All @@ -118,6 +118,7 @@ public ImmutableList<TemplateType> getTemplateKeys() {
* Returns true if this map contains the specified template key, false
* otherwise.
*/
@SuppressWarnings("ReferenceEquality")
public boolean hasTemplateKey(TemplateType templateKey) {
// Note: match by identity, not equality
for (TemplateType entry : templateKeys) {
Expand Down Expand Up @@ -154,8 +155,9 @@ public boolean hasTemplateType(TemplateType key) {

JSType getUnresolvedOriginalTemplateType(TemplateType key) {
int index = getTemplateTypeIndex(key);
return (index == -1) ? registry.getNativeType(JSTypeNative.UNKNOWN_TYPE) :
templateValues.get(index);
return (index == -1)
? registry.getNativeType(JSTypeNative.UNKNOWN_TYPE)
: templateValues.get(index);
}

public TemplateType getTemplateTypeKeyByName(String keyName) {
Expand All @@ -174,7 +176,7 @@ public TemplateType getTemplateTypeKeyByName(String keyName) {
private int getTemplateTypeIndex(TemplateType key) {
int maxIndex = Math.min(templateKeys.size(), templateValues.size());
for (int i = maxIndex - 1; i >= 0; i--) {
if (templateKeys.get(i) == key) {
if (isSameKey(templateKeys.get(i), key)) {
return i;
}
}
Expand All @@ -186,11 +188,8 @@ private int getTemplateTypeIndex(TemplateType key) {
* JSType value is associated, returns UNKNOWN_TYPE.
*/
public JSType getResolvedTemplateType(TemplateType key) {
// Do NOT call addUnknownValues() as a shortcut for safely looking up keys that might
// correspond with an UNKNOWN_TYPE value. Doing so becomes exponentially costly with nested
// template types.
int index = getTemplateTypeIndex(key);
return (index == -1 || index >= templateValues.size())
return (index == -1)
? registry.getNativeType(JSTypeNative.UNKNOWN_TYPE)
: resolvedTemplateValues[index];
}
Expand All @@ -215,8 +214,7 @@ public boolean checkEquivalenceHelper(
public boolean checkEquivalenceHelper(TemplateTypeMap that,
EquivalenceMethod eqMethod, EqCache eqCache, SubtypingMode subtypingMode) {
boolean result = false;
if (!this.inRecursiveEquivalenceCheck &&
!that.inRecursiveEquivalenceCheck) {
if (!this.inRecursiveEquivalenceCheck && !that.inRecursiveEquivalenceCheck) {
this.inRecursiveEquivalenceCheck = true;
that.inRecursiveEquivalenceCheck = true;

Expand All @@ -229,6 +227,11 @@ public boolean checkEquivalenceHelper(TemplateTypeMap that,
return result;
}

@SuppressWarnings("ReferenceEquality")
private static boolean isSameKey(TemplateType thisKey, TemplateType thatKey) {
return thisKey == thatKey;
}

private static boolean checkEquivalenceHelper(EquivalenceMethod eqMethod,
TemplateTypeMap thisMap, TemplateTypeMap thatMap,
EqCache eqCache, SubtypingMode subtypingMode) {
Expand All @@ -247,7 +250,7 @@ private static boolean checkEquivalenceHelper(EquivalenceMethod eqMethod,
// Cross-compare every key-value pair in this TemplateTypeMap with
// those in that TemplateTypeMap. Update the Equivalence match for both
// key-value pairs involved.
if (thisKey == thatKey) {
if (isSameKey(thisKey, thatKey)) {
EquivalenceMatch newMatchType = EquivalenceMatch.VALUE_MISMATCH;
if (thisType.checkEquivalenceHelper(thatType, eqMethod, eqCache)
|| (subtypingMode == SubtypingMode.IGNORE_NULL_UNDEFINED
Expand Down Expand Up @@ -276,9 +279,21 @@ private static boolean checkEquivalenceHelper(EquivalenceMethod eqMethod,
*/
private static boolean failedEquivalenceCheck(
EquivalenceMatch eqMatch, EquivalenceMethod eqMethod) {
return eqMatch == EquivalenceMatch.VALUE_MISMATCH ||
(eqMatch == EquivalenceMatch.NO_KEY_MATCH &&
eqMethod != EquivalenceMethod.INVARIANT);
return eqMatch == EquivalenceMatch.VALUE_MISMATCH
|| (eqMatch == EquivalenceMatch.NO_KEY_MATCH && eqMethod != EquivalenceMethod.INVARIANT);
}

private ImmutableList<JSType> normalize(ImmutableList<JSType> values, int keyCount) {
int missing = keyCount - values.size();
if (missing > 0) {
ImmutableList.Builder<JSType> builder = ImmutableList.builder();
builder.addAll(values);
for (int i = 0; i < missing; i++) {
builder.add(registry.getNativeType(JSTypeNative.UNKNOWN_TYPE));
}
return builder.build();
}
return values;
}

/**
Expand All @@ -287,10 +302,12 @@ private static boolean failedEquivalenceCheck(
* specified map.
*/
TemplateTypeMap extend(TemplateTypeMap thatMap) {
thatMap = thatMap.addUnknownValues();
ImmutableList<JSType> thatNormalizedValues = normalize(
thatMap.templateValues, thatMap.templateKeys.size());

return registry.createTemplateTypeMap(
concatImmutableLists(thatMap.templateKeys, templateKeys),
concatImmutableLists(thatMap.templateValues, templateValues));
concatImmutableLists(thatNormalizedValues, templateValues));
}

/**
Expand All @@ -309,23 +326,6 @@ TemplateTypeMap addValues(ImmutableList<JSType> newValues) {
templateKeys, concatImmutableLists(templateValues, newValues));
}

/**
* Returns a new TemplateTypeMap, where all unfilled values have been filled
* with UNKNOWN_TYPE.
*/
private TemplateTypeMap addUnknownValues() {
int numUnfilledTemplateKeys = numUnfilledTemplateKeys();
if (numUnfilledTemplateKeys == 0) {
return this;
}

ImmutableList.Builder<JSType> builder = ImmutableList.builder();
for (int i = 0; i < numUnfilledTemplateKeys; i++) {
builder.add(registry.getNativeType(JSTypeNative.UNKNOWN_TYPE));
}
return addValues(builder.build());
}

/**
* Concatenates two ImmutableList instances. If either input is empty, the
* other is returned; otherwise, a new ImmutableList instance is created that
Expand All @@ -347,7 +347,7 @@ private <T> ImmutableList<T> concatImmutableLists(

boolean hasAnyTemplateTypesInternal() {
if (resolvedTemplateValues != null) {
for (JSType templateValue : addUnknownValues().resolvedTemplateValues) {
for (JSType templateValue : resolvedTemplateValues) {
if (templateValue.hasAnyTemplateTypes()) {
return true;
}
Expand Down

0 comments on commit 40dd127

Please sign in to comment.