Skip to content

Commit

Permalink
Only build GlobalNamespace once in AggressiveInlineAliases
Browse files Browse the repository at this point in the history 
This includes some prerequisite changes to correctly update the GlobalNamespace
after changing the AST:
 - change how subclassing references are stored, to correctly add new
   references
 - scanFromNode should not rescan parent nodes except in getprops
 - a few places in AggressiveInlineAliases need to report AST changes

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=224577204
  • Loading branch information
@lauraharker @EatingW
lauraharker authored and EatingW committed Dec 8, 2018
1 parent 8d1ad54 commit 37414c0
Show file tree
Hide file tree
Showing 4 changed files with 274 additions and 100 deletions.
92 changes: 65 additions & 27 deletions src/com/google/javascript/jscomp/AggressiveInlineAliases.java
View file
Expand Up @@ -16,8 +16,10 @@


package com.google.javascript.jscomp; package com.google.javascript.jscomp;


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


import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.common.base.Predicates; import com.google.common.base.Predicates;
import com.google.javascript.jscomp.GlobalNamespace.AstChange; import com.google.javascript.jscomp.GlobalNamespace.AstChange;
Expand All @@ -33,6 +35,7 @@
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
import javax.annotation.Nullable;


/** /**
* Inlines type aliases if they are explicitly or effectively const. Also inlines inherited static * Inlines type aliases if they are explicitly or effectively const. Also inlines inherited static
Expand All @@ -55,17 +58,25 @@ class AggressiveInlineAliases implements CompilerPass {


private final AbstractCompiler compiler; private final AbstractCompiler compiler;
private boolean codeChanged; private boolean codeChanged;
private GlobalNamespace namespace;


AggressiveInlineAliases(AbstractCompiler compiler) { AggressiveInlineAliases(AbstractCompiler compiler) {
this.compiler = compiler; this.compiler = compiler;
this.codeChanged = true; this.codeChanged = true;
} }


@VisibleForTesting
GlobalNamespace getLastUsedGlobalNamespace() {
return namespace;
}

@Override @Override
public void process(Node externs, Node root) { public void process(Node externs, Node root) {
while (this.codeChanged) { // Building the `GlobalNamespace` dominates the cost of this pass, so it is built once and
this.codeChanged = false; // updated as changes are made so it can be reused for the next iteration.
GlobalNamespace namespace = new GlobalNamespace(compiler, root); this.namespace = new GlobalNamespace(compiler, root);
while (codeChanged) {
codeChanged = false;
inlineAliases(namespace); inlineAliases(namespace);
} }
} }
Expand Down Expand Up @@ -115,7 +126,7 @@ private void inlineAliases(GlobalNamespace namespace) {
if (!name.inExterns() if (!name.inExterns()
&& name.getGlobalSets() == 1 && name.getGlobalSets() == 1
&& name.getLocalSets() == 0 && name.getLocalSets() == 0
&& name.getAliasingGets() > 0) { && (name.getAliasingGets() > 0 || name.getSubclassingGets() > 0)) {
// {@code name} meets condition (b). Find all of its local aliases // {@code name} meets condition (b). Find all of its local aliases
// and try to inline them. // and try to inline them.
List<Ref> refs = new ArrayList<>(name.getRefs()); List<Ref> refs = new ArrayList<>(name.getRefs());
Expand All @@ -131,16 +142,9 @@ private void inlineAliases(GlobalNamespace namespace) {
&& hoistScope.isGlobal() && hoistScope.isGlobal()
&& ref.getTwin() == null) { // ignore aliases in chained assignments && ref.getTwin() == null) { // ignore aliases in chained assignments
inlineGlobalAliasIfPossible(name, ref, namespace); inlineGlobalAliasIfPossible(name, ref, namespace);
} } else if (name.isClass() && ref.type == Type.SUBCLASSING_GET && name.props != null) {
}
}

if (!name.inExterns() && name.isClass()) {
List<Name> subclasses = name.subclasses;
if (subclasses != null && name.props != null) {
for (Name subclass : subclasses) {
for (Name prop : name.props) { for (Name prop : name.props) {
rewriteAllSubclassInheritedAccesses(name, subclass, prop, namespace); rewriteAllSubclassInheritedAccesses(name, ref, prop, namespace);
} }
} }
} }
Expand Down Expand Up @@ -202,12 +206,17 @@ private void maybeAddPropertiesToWorklist(Name name, Deque<Name> workList) {
* possible, since they may use this or super(). * possible, since they may use this or super().
* *
* @param superclassNameObj The Name of the superclass * @param superclassNameObj The Name of the superclass
* @param subclassNameObj The Name of the subclass * @param superclassRef The SUBCLASSING_REF
* @param prop The property on the superclass to rewrite, if any descendant accesses it. * @param prop The property on the superclass to rewrite, if any descendant accesses it.
* @param namespace The GlobalNamespace containing superclassNameObj * @param namespace The GlobalNamespace containing superclassNameObj
*/ */
private boolean rewriteAllSubclassInheritedAccesses( private boolean rewriteAllSubclassInheritedAccesses(
Name superclassNameObj, Name subclassNameObj, Name prop, GlobalNamespace namespace) { Name superclassNameObj, Ref superclassRef, Name prop, GlobalNamespace namespace) {
Node subclass = getSubclassForEs6Superclass(superclassRef.getNode());
if (subclass == null || !subclass.isQualifiedName()) {
return false;
}
String subclassName = subclass.getQualifiedName();
Ref propDeclRef = prop.getDeclaration(); Ref propDeclRef = prop.getDeclaration();
if (propDeclRef == null if (propDeclRef == null
|| propDeclRef.node == null || propDeclRef.node == null
Expand All @@ -219,7 +228,7 @@ private boolean rewriteAllSubclassInheritedAccesses(
return false; return false;
} }


String subclassQualifiedPropName = subclassNameObj.getFullName() + "." + prop.getBaseName(); String subclassQualifiedPropName = subclassName + "." + prop.getBaseName();
Name subclassPropNameObj = namespace.getOwnSlot(subclassQualifiedPropName); Name subclassPropNameObj = namespace.getOwnSlot(subclassQualifiedPropName);
// Don't rewrite if the subclass ever shadows the parent static property. // Don't rewrite if the subclass ever shadows the parent static property.
// This may also back off on cases where the subclass first accesses the parent property, then // This may also back off on cases where the subclass first accesses the parent property, then
Expand All @@ -230,9 +239,12 @@ private boolean rewriteAllSubclassInheritedAccesses(
} }


// Recurse to find potential sub-subclass accesses of the superclass property. // Recurse to find potential sub-subclass accesses of the superclass property.
if (subclassNameObj.subclasses != null) { Name subclassNameObj = namespace.getOwnSlot(subclassName);
for (Name name : subclassNameObj.subclasses) { if (subclassNameObj != null && subclassNameObj.subclassingGets > 0) {
rewriteAllSubclassInheritedAccesses(superclassNameObj, name, prop, namespace); for (Ref ref : subclassNameObj.getRefs()) {
if (ref.type == Type.SUBCLASSING_GET) {
rewriteAllSubclassInheritedAccesses(superclassNameObj, ref, prop, namespace);
}
} }
} }


Expand Down Expand Up @@ -336,12 +348,13 @@ private void inlineAliasIfPossible(Name name, Ref alias, GlobalNamespace namespa
} }


// just set the original alias to null. // just set the original alias to null.
replaceAliasAssignment(alias, aliasLhsNode); if (tryReplacingAliasingAssignment(alias, aliasLhsNode)) {
name.removeRef(alias);
}


// Inlining the variable may have introduced new references // Inlining the variable may have introduced new references
// to descendants of {@code name}. So those need to be collected now. // to descendants of {@code name}. So those need to be collected now.
namespace.scanNewNodes(newNodes); namespace.scanNewNodes(newNodes);
name.removeRef(alias);
return; return;
} }


Expand All @@ -350,9 +363,7 @@ private void inlineAliasIfPossible(Name name, Ref alias, GlobalNamespace namespa
// generators introduces some constructor aliases that weren't getting inlined. // generators introduces some constructor aliases that weren't getting inlined.
// If we find another (safer) way to avoid aliasing in method decomposition, consider // If we find another (safer) way to avoid aliasing in method decomposition, consider
// removing this. // removing this.
if (partiallyInlineAlias(alias, namespace, aliasRefs, aliasLhsNode)) { if (!partiallyInlineAlias(alias, namespace, aliasRefs, aliasLhsNode)) {
name.removeRef(alias);
} else {
// If we can't inline all alias references, make sure there are no unsafe property // If we can't inline all alias references, make sure there are no unsafe property
// accesses. // accesses.
if (referencesCollapsibleProperty(aliasRefs, name, namespace)) { if (referencesCollapsibleProperty(aliasRefs, name, namespace)) {
Expand Down Expand Up @@ -423,7 +434,7 @@ private boolean partiallyInlineAlias(


// We removed all references to the alias, so remove the original aliasing assignment. // We removed all references to the alias, so remove the original aliasing assignment.
if (!foundNonReplaceableAlias) { if (!foundNonReplaceableAlias) {
replaceAliasAssignment(alias, aliasLhsNode); tryReplacingAliasingAssignment(alias, aliasLhsNode);
} }


if (codeChanged) { if (codeChanged) {
Expand All @@ -438,19 +449,20 @@ private boolean partiallyInlineAlias(
* Replaces the rhs of an aliasing assignment with null, unless the assignment result is used in a * Replaces the rhs of an aliasing assignment with null, unless the assignment result is used in a
* complex expression. * complex expression.
*/ */
private void replaceAliasAssignment(Ref alias, Node aliasLhsNode) { private boolean tryReplacingAliasingAssignment(Ref alias, Node aliasLhsNode) {
// either VAR/CONST/LET or ASSIGN. // either VAR/CONST/LET or ASSIGN.
Node assignment = aliasLhsNode.getParent(); Node assignment = aliasLhsNode.getParent();
if (!NodeUtil.isNameDeclaration(assignment) && NodeUtil.isExpressionResultUsed(assignment)) { if (!NodeUtil.isNameDeclaration(assignment) && NodeUtil.isExpressionResultUsed(assignment)) {
// e.g. don't change "if (alias = someVariable)" to "if (alias = null)" // e.g. don't change "if (alias = someVariable)" to "if (alias = null)"
// TODO(lharker): instead replace the entire assignment with the RHS - "alias = x" becomes "x" // TODO(lharker): instead replace the entire assignment with the RHS - "alias = x" becomes "x"
return; return false;
} }
Node aliasParent = alias.node.getParent(); Node aliasParent = alias.node.getParent();
aliasParent.replaceChild(alias.node, IR.nullNode()); aliasParent.replaceChild(alias.node, IR.nullNode());
alias.name.removeRef(alias); alias.name.removeRef(alias);
codeChanged = true; codeChanged = true;
compiler.reportChangeToEnclosingScope(aliasParent); compiler.reportChangeToEnclosingScope(aliasParent);
return true;
} }


/** /**
Expand Down Expand Up @@ -561,6 +573,7 @@ private void inlineGlobalAliasIfPossible(Name name, Ref alias, GlobalNamespace n
aliasParent.replaceWith(alias.node.detach()); aliasParent.replaceWith(alias.node.detach());
aliasingName.removeRef(aliasDeclaration); aliasingName.removeRef(aliasDeclaration);
aliasingName.removeRef(aliasDeclaration.getTwin()); aliasingName.removeRef(aliasDeclaration.getTwin());
newNodes.add(new AstChange(alias.module, alias.scope, alias.node));
compiler.reportChangeToEnclosingScope(aliasGrandparent); compiler.reportChangeToEnclosingScope(aliasGrandparent);
} else { } else {
// just set the original alias to null. // just set the original alias to null.
Expand Down Expand Up @@ -589,6 +602,7 @@ private void rewriteAliasReferences(Name aliasingName, Ref aliasingRef, Set<AstC
case ALIASING_GET: case ALIASING_GET:
case PROTOTYPE_GET: case PROTOTYPE_GET:
case CALL_GET: case CALL_GET:
case SUBCLASSING_GET:
if (ref.getTwin() != null) { if (ref.getTwin() != null) {
// The reference is the left-hand side of a nested assignment. This means we store two // The reference is the left-hand side of a nested assignment. This means we store two
// separate 'twin' Refs with the same node of types ALIASING_GET and SET_FROM_GLOBAL. // separate 'twin' Refs with the same node of types ALIASING_GET and SET_FROM_GLOBAL.
Expand Down Expand Up @@ -688,4 +702,28 @@ private void rewriteAliasProp(Node value, int depth, Set<AstChange> newNodes, Na
codeChanged = true; codeChanged = true;
} }
} }

/**
* Tries to find an lvalue for the subclass given the superclass node in an `class ... extends `
* clause
*
* <p>Only handles cases where we have either a class declaration or a class expression in an
* assignment or name declaration. Otherwise returns null.
*/
@Nullable
private static Node getSubclassForEs6Superclass(Node superclass) {
Node classNode = superclass.getParent();
checkArgument(classNode.isClass(), classNode);
if (NodeUtil.isNameDeclaration(classNode.getGrandparent())) {
// const Clazz = class extends Super {
return classNode.getParent();
} else if (superclass.getGrandparent().isAssign()) {
// ns.foo.Clazz = class extends Super {
return classNode.getPrevious();
} else if (NodeUtil.isClassDeclaration(classNode)) {
// class Clazz extends Super {
return classNode.getFirstChild();
}
return null;
}
} }

0 comments on commit 37414c0

Please sign in to comment.