Skip to content

Commit

Permalink
Remove ReferenceMap.isExpressionValueAliased: this method currently d…
Browse files Browse the repository at this point in the history 
…oesn't serve any purpose as any "unknown" reference (not a call, a definition, or known use) is assumed to be an escaping alias.

Also pull out some common code into helper methods while we are here.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=175236435
  • Loading branch information
@concavelenz @dimvar
concavelenz authored and dimvar committed Nov 10, 2017
1 parent 462ebc7 commit efe342c
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 130 deletions.
127 changes: 47 additions & 80 deletions src/com/google/javascript/jscomp/OptimizeCalls.java
View file
Expand Up @@ -102,85 +102,6 @@ Iterable<Map.Entry<String, ArrayList<Node>>> getPropReferences() {
return props.entrySet();
}

/**
* @return true iff the result of the expression is consumed in a way that creates an alias.
*/
private static boolean isExpressionValueAliased(Node expr) {
Node parent = expr.getParent();
switch (parent.getToken()) {
case CAST:
return isExpressionValueAliased(parent);
case HOOK:
return (expr != parent.getFirstChild()) && isExpressionValueAliased(parent);
case AND:
case OR:
return isExpressionValueAliased(parent);
case COMMA:
return (expr == parent.getLastChild()) && isExpressionValueAliased(parent);

// function result expressions
case RETURN:
case THROW:
case YIELD:
case AWAIT:
return true;

// Tagged template literals receive the call
case TEMPLATELIT:
return parent.getParent().isTaggedTemplateLit();
case NEW:
case CALL: // CALL(..., value, ...)
return (expr != parent.getFirstChild());

// Direct assignments
case ASSIGN: // ... = value
case NAME: // NAME = value
case ARRAYLIT:// [ ..., value, ... ]
return true;
// object literals give the value a new name
case COMPUTED_PROP:
return expr == parent.getLastChild() && isExpressionValueAliased(parent);
case STRING_KEY:
return isExpressionValueAliased(parent);

case OBJECTLIT:
// objects can be reflected upon
// TODO(johnlenz): add support for objects assigned to prototypes
return true;

case GETPROP:
case GETELEM:
// Calls escape the "this" value. a.foo() aliases "a" as "this" but general
// property references do not.
Node grandparent = parent.getParent();
if (grandparent != null && grandparent.isCall()) {
return true;
}
return false;

default:
break;
}
return false;
}

static boolean isAliasingReference(Node n) {
if (isCallOrNewTarget(n)) {
return false;
} else if (NodeUtil.isLValue(n)) {
Node value = NodeUtil.getRValueOfLValue(n);
if (value == null) {
// A "var x;" declaration is not an alias.
return false;
} else {
// We consider recursive calls to be aliased.
return NodeUtil.isNamedFunctionExpression(value);
}
} else {
return isExpressionValueAliased(n);
}
}

/**
* Given a set of references, returns the set of known definitions. Specifically,
* those of the form:
Expand Down Expand Up @@ -434,12 +355,58 @@ public void exitScope(NodeTraversal t) {
}
}


static ReferenceMap buildPropAndGlobalNameReferenceMap(
AbstractCompiler compiler, Node externs, Node root) {
final ReferenceMap references = new ReferenceMap();
NodeTraversal.traverseRootsEs6(compiler, new ReferenceMapBuildingCallback(
compiler, references), externs, root);
return references;
}

/**
* @return Whether the provide name may be a candidate for
* call optimizations.
*/
static boolean mayBeOptimizableName(AbstractCompiler compiler, String name) {
if (compiler.getCodingConvention().isExported(name)) {
return false;
}

// Avoid modifying a few special case functions. Specifically, $jscomp.inherits to
// recognize 'inherits' calls. (b/27244988)
if (name.equals(NodeUtil.JSC_PROPERTY_NAME_FN)
|| name.equals(NodeUtil.EXTERN_OBJECT_PROPERTY_STRING)
|| name.equals("inherits")
|| name.equals("$jscomp$inherits")
|| name.equals("goog$inherits")) {
return false;
}
return true;
}

/**
* @return Whether the reference is a known non-aliasing reference.
*/
static boolean isAllowedReference(Node n) {
Node parent = n.getParent();
switch (parent.getToken()) {
case INSTANCEOF:
case TYPEOF:
case GETELEM:
case GETPROP:
// Calls escape the "this" value. a.foo() aliases "a" as "this" but general
// property references do not.
Node grandparent = parent.getParent();
if (n == parent.getFirstChild() && grandparent != null && grandparent.isCall()) {
return false;
}
return true;
default:
if (NodeUtil.isNameDeclaration(parent) && !n.hasChildren()) {
// allow "let x;"
return true;
}
}
return false;
}
}
36 changes: 7 additions & 29 deletions src/com/google/javascript/jscomp/OptimizeParameters.java
View file
Expand Up @@ -338,27 +338,13 @@ private static boolean alreadyRemoved(Node n) {
* - there is at least one definition
*/
private boolean isCandidate(String name, ArrayList<Node> refs) {
if (compiler.getCodingConvention().isExported(name)) {
return false;
}

if (name.equals(NodeUtil.JSC_PROPERTY_NAME_FN)
|| name.equals(NodeUtil.EXTERN_OBJECT_PROPERTY_STRING)) {
return false;
}

// We don't want to re-write $jscomp.inherits to not stop recognizing
// 'inherits' calls. (b/27244988)
if (name.equals("inherits")
|| name.equals("$jscomp$inherits")
|| name.equals("goog$inherits")) {
if (!OptimizeCalls.mayBeOptimizableName(compiler, name)) {
return false;
}

boolean seenCandidateDefiniton = false;
boolean seenCandidateUse = false;
for (Node n : refs) {
Node parent = n.getParent();
// TODO(johnlenz): Determine what to do about ".constructor" references.
// Currently classes that are super classes or have superclasses aren't optimized
//
Expand All @@ -369,23 +355,15 @@ private boolean isCandidate(String name, ArrayList<Node> refs) {
if (ReferenceMap.isCallOrNewTarget(n)) {
// TODO(johnlenz): filter .apply when we support it
seenCandidateUse = true;
continue;
} else if (ReferenceMap.isAliasingReference(n)) {
// The name is aliased, so we don't know anything about its uses, this is not
// a candidate
return false;
} else if (parent.isInstanceOf() || parent.isTypeOf()) {
continue;
} else if (parent.isGetProp() || parent.isGetElem()) {
continue;
} else if (NodeUtil.isNameDeclaration(parent) && !n.hasChildren()) {
// allow "let x;"
continue;
} else if (isCandidateDefinition(n)) {
seenCandidateDefiniton = true;
} else {
// TODO(johnlenz): allow extends clauses.
return false;
// If this isn't an non-aliasing reference (typeof, instanceof, etc)
// then there is nothing that can be done.
if (!OptimizeCalls.isAllowedReference(n)) {
// TODO(johnlenz): allow extends clauses.
return false;
}
}
}

Expand Down
27 changes: 6 additions & 21 deletions src/com/google/javascript/jscomp/OptimizeReturns.java
View file
Expand Up @@ -92,22 +92,14 @@ public void process(Node externs, Node root, ReferenceMap definitions) {
* definitions will be removed.
*/
private boolean isCandidate(String name, List<Node> refs) {
if (compiler.getCodingConvention().isExported(name)) {
return false;
}

// Avoid modifying a few special case functions.
if (name.equals(NodeUtil.JSC_PROPERTY_NAME_FN)
|| name.equals(NodeUtil.EXTERN_OBJECT_PROPERTY_STRING)) {
if (!OptimizeCalls.mayBeOptimizableName(compiler, name)) {
return false;
}

boolean seenCandidateDefiniton = false;
boolean seenUse = false;

for (Node n : refs) {
// Assume indirect definitions references use the result
Node parent = n.getParent();
if (ReferenceMap.isCallTarget(n)) {
Node callNode = ReferenceMap.getCallOrNewNodeForTarget(n);
if (NodeUtil.isExpressionResultUsed(callNode)) {
Expand All @@ -116,24 +108,17 @@ private boolean isCandidate(String name, List<Node> refs) {
return false;
}
seenUse = true;
} else if (ReferenceMap.isAliasingReference(n)) {
// The name is aliased, so we don't know anything about its uses, this is not
// a candidate
return false;
} else if (parent.isInstanceOf() || parent.isTypeOf()) {
continue;
} else if (parent.isGetProp() || parent.isGetElem()) {
continue;
} else if (NodeUtil.isNameDeclaration(parent) && !n.hasChildren()) {
// allow "let x;"
continue;
} else if (isCandidateDefinition(n)) {
// NOTE: While is is possible to optimize calls to functions for which we know
// only some of the definition are candidates but to keep things simple, only
// optimize if all of the definitions are known.
seenCandidateDefiniton = true;
} else {
return false;
// If this isn't an non-aliasing reference (typeof, instanceof, etc)
// then there is nothing that can be done.
if (!OptimizeCalls.isAllowedReference(n)) {
return false;
}
}
}

Expand Down

0 comments on commit efe342c

Please sign in to comment.