Skip to content

Commit

Permalink
CrossModuleCodeMotion: add instanceof guards after moving code.
Browse files Browse the repository at this point in the history
This will make it easier to change the criteria used to determine where code
will be moved.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=156580678
  • Loading branch information
brad4d authored and Tyler Breisacher committed May 20, 2017
1 parent d4ec854 commit f087bae
Showing 1 changed file with 17 additions and 24 deletions.
41 changes: 17 additions & 24 deletions src/com/google/javascript/jscomp/CrossModuleCodeMotion.java
Expand Up @@ -89,13 +89,13 @@ public void process(Node externs, Node root) {
// Traverse the tree and find the modules where a var is declared + used // Traverse the tree and find the modules where a var is declared + used
collectReferences(root); collectReferences(root);


// Move the functions + variables to a deeper module [if possible]
moveCode();

// Make is so we can ignore constructor references in instanceof. // Make is so we can ignore constructor references in instanceof.
if (parentModuleCanSeeSymbolsDeclaredInChildren) { if (parentModuleCanSeeSymbolsDeclaredInChildren) {
makeInstanceOfCodeOrderIndependent(); addInstanceofGuards();
} }

// Move the functions + variables to a deeper module [if possible]
moveCode();
} }
} }


Expand Down Expand Up @@ -129,6 +129,9 @@ private void moveCode() {


compiler.reportChangeToEnclosingScope(declParent); compiler.reportChangeToEnclosingScope(declParent);
} }
// Update variable declaration location.
info.wasMoved = true;
info.declModule = info.preferredModule;
} }
} }
} }
Expand All @@ -145,6 +148,8 @@ private class NamedInfo {
// The module where declarations appear // The module where declarations appear
private JSModule declModule = null; private JSModule declModule = null;


private boolean wasMoved = false;

// information on the spot where the item was declared // information on the spot where the item was declared
private final Deque<Declaration> declarations = private final Deque<Declaration> declarations =
new ArrayDeque<>(); new ArrayDeque<>();
Expand All @@ -166,14 +171,6 @@ void addUsedModule(JSModule m) {
} }
} }


boolean isUsedInOrDependencyOfModule(JSModule m) {
checkNotNull(m);
if (preferredModule == null) {
return false;
}
return m == preferredModule || graph.dependsOn(m, preferredModule);
}

/** /**
* Add a declaration for this name. * Add a declaration for this name.
* @return Whether this is a valid declaration. If this returns false, * @return Whether this is a valid declaration. If this returns false,
Expand Down Expand Up @@ -591,12 +588,18 @@ private static boolean canMoveValue(
* false if tested with a constructor that is undefined. This allows ignoring * false if tested with a constructor that is undefined. This allows ignoring
* instanceof with respect to cross module code motion. * instanceof with respect to cross module code motion.
*/ */
private void makeInstanceOfCodeOrderIndependent() { private void addInstanceofGuards() {
Node tmp = IR.block(); Node tmp = IR.block();
for (Map.Entry<Node, InstanceofInfo> entry : instanceofNodes.entrySet()) { for (Map.Entry<Node, InstanceofInfo> entry : instanceofNodes.entrySet()) {
Node n = entry.getKey(); Node n = entry.getKey();
InstanceofInfo info = entry.getValue(); InstanceofInfo info = entry.getValue();
if (!info.namedInfo.allowMove || !info.mustBeGuardedByTypeof()) { // No need for a guard if:
// 1. the declaration wasn't moved
// 2. OR it was moved to the start of the module containing this instanceof reference
// 3. OR it was moved to a module the instanceof reference's module depends on
if (!info.namedInfo.wasMoved
|| info.namedInfo.declModule.equals(info.module)
|| graph.dependsOn(info.module, info.namedInfo.declModule)) {
continue; continue;
} }
// Wrap "foo instanceof Bar" in // Wrap "foo instanceof Bar" in
Expand Down Expand Up @@ -630,15 +633,5 @@ private static class InstanceofInfo {
this.module = checkNotNull(module); this.module = checkNotNull(module);
this.namedInfo = checkNotNull(namedInfo); this.namedInfo = checkNotNull(namedInfo);
} }

/**
* Returns true if this instance of instanceof is in a deeper module than
* the deepest module (by reference) of the related name.
* In that case the name may be undefined when the instanceof runs and we
* have to guard it with typeof.
*/
boolean mustBeGuardedByTypeof() {
return !this.namedInfo.isUsedInOrDependencyOfModule(this.module);
}
} }
} }

0 comments on commit f087bae

Please sign in to comment.