Skip to content

Commit

Permalink
Make the Control Flow Graph of a class never traverse inside the clas…
Browse files Browse the repository at this point in the history
…s and instead create a directed edge with the code below it.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158529934
  • Loading branch information
lillymliu authored and Tyler Breisacher committed Jun 12, 2017
1 parent 54df0ee commit 72035e5
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 10 deletions.
15 changes: 8 additions & 7 deletions src/com/google/javascript/jscomp/ControlFlowAnalysis.java
Expand Up @@ -77,7 +77,7 @@ public int compare(
private int astPositionCounter; private int astPositionCounter;
private int priorityCounter; private int priorityCounter;


private final boolean shouldTraverseFunctionsAndClasses; private final boolean shouldTraverseFunctions;
private final boolean edgeAnnotations; private final boolean edgeAnnotations;


// We need to store where we started, in case we aren't doing a flow analysis // We need to store where we started, in case we aren't doing a flow analysis
Expand Down Expand Up @@ -130,10 +130,10 @@ public int compare(
* @param shouldTraverseFunctions Whether functions should be traversed * @param shouldTraverseFunctions Whether functions should be traversed
* @param edgeAnnotations Whether to allow edge annotations. * @param edgeAnnotations Whether to allow edge annotations.
*/ */
ControlFlowAnalysis(AbstractCompiler compiler, ControlFlowAnalysis(
boolean shouldTraverseFunctionsAndClasses, boolean edgeAnnotations) { AbstractCompiler compiler, boolean shouldTraverseFunctions, boolean edgeAnnotations) {
this.compiler = compiler; this.compiler = compiler;
this.shouldTraverseFunctionsAndClasses = shouldTraverseFunctionsAndClasses; this.shouldTraverseFunctions = shouldTraverseFunctions;
this.edgeAnnotations = edgeAnnotations; this.edgeAnnotations = edgeAnnotations;
} }


Expand All @@ -159,7 +159,7 @@ public void process(Node externs, Node root) {
DiGraphNode<Node, Branch> entry = cfg.getEntry(); DiGraphNode<Node, Branch> entry = cfg.getEntry();
prioritizeFromEntryNode(entry); prioritizeFromEntryNode(entry);


if (shouldTraverseFunctionsAndClasses) { if (shouldTraverseFunctions) {
// If we're traversing inner functions, we need to rank the // If we're traversing inner functions, we need to rank the
// priority of them too. // priority of them too.
for (DiGraphNode<Node, Branch> candidate : cfg.getDirectedGraphNodes()) { for (DiGraphNode<Node, Branch> candidate : cfg.getDirectedGraphNodes()) {
Expand Down Expand Up @@ -213,9 +213,10 @@ public boolean shouldTraverse(


switch (n.getToken()) { switch (n.getToken()) {
case CLASS: case CLASS:
return shouldTraverseFunctionsAndClasses; createEdge(n, Branch.UNCOND, n.getNext());
return false;
case FUNCTION: case FUNCTION:
if (shouldTraverseFunctionsAndClasses || n == cfg.getEntry().getValue()) { if (shouldTraverseFunctions || n == cfg.getEntry().getValue()) {
exceptionHandler.push(n); exceptionHandler.push(n);
return true; return true;
} }
Expand Down
11 changes: 8 additions & 3 deletions test/com/google/javascript/jscomp/CheckUnreachableCodeTest.java
Expand Up @@ -311,9 +311,14 @@ public void testClass() {
assertUnreachable("var C = class{func(){if (true){return;} x = 1;}}"); assertUnreachable("var C = class{func(){if (true){return;} x = 1;}}");
} }


// TODO(tbreisacher): This should not produce a warning. public void testUnderClass() {
public void testClass_incorrect() { testSame("class C {} alert(1);");
assertUnreachable("class C {} alert(1);"); testSame("class D {} class C extends D {} alert(1)");
testSame("class D{} alert(1); class C extends D {}");
}

public void testFunction() {
testSame("function f() {} alert(1);");
} }


public void testSubclass() { public void testSubclass() {
Expand Down
30 changes: 30 additions & 0 deletions test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java
Expand Up @@ -827,6 +827,36 @@ public void testSimpleFunction() {
testCfg(src, expected); testCfg(src, expected);
} }


public void testSimpleClass() {
String src = "class C{} f();";
String expected =
"digraph AST {\n"
+ " node [color=lightblue2, style=filled];\n"
+ " node0 [label=\"SCRIPT\"];\n"
+ " node1 [label=\"CLASS\"];\n"
+ " node0 -> node1 [weight=1];\n"
+ " node2 [label=\"NAME\"];\n"
+ " node1 -> node2 [weight=1];\n"
+ " node3 [label=\"EMPTY\"];\n"
+ " node1 -> node3 [weight=1];\n"
+ " node4 [label=\"CLASS_MEMBERS\"];\n"
+ " node1 -> node4 [weight=1];\n"
+ " node5 [label=\"EXPR_RESULT\"];\n"
+ " node1 -> node5 "
+ "[label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n"
+ " node0 -> node5 [weight=1];\n"
+ " node6 [label=\"CALL\"];\n"
+ " node5 -> node6 [weight=1];\n"
+ " node7 [label=\"NAME\"];\n"
+ " node6 -> node7 [weight=1];\n"
+ " node5 -> RETURN "
+ "[label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n"
+ " node0 -> node1 "
+ "[label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n"
+ "}\n";
testCfg(src, expected);
}

public void testSimpleCatch() { public void testSimpleCatch() {
String src = "try{ throw x; x(); x['stuff']; x.x; x} catch (e) { e() }"; String src = "try{ throw x; x(); x['stuff']; x.x; x} catch (e) { e() }";
String expected = "digraph AST {\n" String expected = "digraph AST {\n"
Expand Down

0 comments on commit 72035e5

Please sign in to comment.