From 72035e581d2deb7bbe517e8907897016e2c4f378 Mon Sep 17 00:00:00 2001 From: lillymliu Date: Fri, 9 Jun 2017 10:01:05 -0700 Subject: [PATCH] Make the Control Flow Graph of a class never traverse inside the class and instead create a directed edge with the code below it. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=158529934 --- .../jscomp/ControlFlowAnalysis.java | 15 +++++----- .../jscomp/CheckUnreachableCodeTest.java | 11 +++++-- .../jscomp/ControlFlowAnalysisTest.java | 30 +++++++++++++++++++ 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/com/google/javascript/jscomp/ControlFlowAnalysis.java b/src/com/google/javascript/jscomp/ControlFlowAnalysis.java index 04a250f2bbc..7adc1628458 100644 --- a/src/com/google/javascript/jscomp/ControlFlowAnalysis.java +++ b/src/com/google/javascript/jscomp/ControlFlowAnalysis.java @@ -77,7 +77,7 @@ public int compare( private int astPositionCounter; private int priorityCounter; - private final boolean shouldTraverseFunctionsAndClasses; + private final boolean shouldTraverseFunctions; private final boolean edgeAnnotations; // We need to store where we started, in case we aren't doing a flow analysis @@ -130,10 +130,10 @@ public int compare( * @param shouldTraverseFunctions Whether functions should be traversed * @param edgeAnnotations Whether to allow edge annotations. */ - ControlFlowAnalysis(AbstractCompiler compiler, - boolean shouldTraverseFunctionsAndClasses, boolean edgeAnnotations) { + ControlFlowAnalysis( + AbstractCompiler compiler, boolean shouldTraverseFunctions, boolean edgeAnnotations) { this.compiler = compiler; - this.shouldTraverseFunctionsAndClasses = shouldTraverseFunctionsAndClasses; + this.shouldTraverseFunctions = shouldTraverseFunctions; this.edgeAnnotations = edgeAnnotations; } @@ -159,7 +159,7 @@ public void process(Node externs, Node root) { DiGraphNode entry = cfg.getEntry(); prioritizeFromEntryNode(entry); - if (shouldTraverseFunctionsAndClasses) { + if (shouldTraverseFunctions) { // If we're traversing inner functions, we need to rank the // priority of them too. for (DiGraphNode candidate : cfg.getDirectedGraphNodes()) { @@ -213,9 +213,10 @@ public boolean shouldTraverse( switch (n.getToken()) { case CLASS: - return shouldTraverseFunctionsAndClasses; + createEdge(n, Branch.UNCOND, n.getNext()); + return false; case FUNCTION: - if (shouldTraverseFunctionsAndClasses || n == cfg.getEntry().getValue()) { + if (shouldTraverseFunctions || n == cfg.getEntry().getValue()) { exceptionHandler.push(n); return true; } diff --git a/test/com/google/javascript/jscomp/CheckUnreachableCodeTest.java b/test/com/google/javascript/jscomp/CheckUnreachableCodeTest.java index 98259849a7a..70561572264 100644 --- a/test/com/google/javascript/jscomp/CheckUnreachableCodeTest.java +++ b/test/com/google/javascript/jscomp/CheckUnreachableCodeTest.java @@ -311,9 +311,14 @@ public void testClass() { assertUnreachable("var C = class{func(){if (true){return;} x = 1;}}"); } - // TODO(tbreisacher): This should not produce a warning. - public void testClass_incorrect() { - assertUnreachable("class C {} alert(1);"); + public void testUnderClass() { + testSame("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() { diff --git a/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java b/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java index e620089e99c..a0bcc249723 100644 --- a/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java +++ b/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java @@ -827,6 +827,36 @@ public void testSimpleFunction() { 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() { String src = "try{ throw x; x(); x['stuff']; x.x; x} catch (e) { e() }"; String expected = "digraph AST {\n"