From 75774a5302177269b6c17363a08dd1a99e15cd42 Mon Sep 17 00:00:00 2001 From: blickly Date: Wed, 21 Jun 2017 12:25:46 -0700 Subject: [PATCH] Add NodeUtil methods for permanently deleting nodes from the AST In addition to removing the node, these methods make sure to do the correct reporting to the compiler about changes/removals. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=159726802 --- .../jscomp/ConvertToTypedInterface.java | 33 +++++++------------ .../google/javascript/jscomp/NodeUtil.java | 20 +++++++++++ .../jscomp/PeepholeFoldConstants.java | 4 +-- .../jscomp/PeepholeRemoveDeadCode.java | 5 +-- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java index 741d30a82d1..eb9d3447e80 100644 --- a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java +++ b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java @@ -63,9 +63,7 @@ private void unhoistExternsToCode(Node externs, Node root) { // Clear out the contents of existing scripts. for (Node script = root.getFirstChild(); script != null; script = script.getNext()) { if (script.hasChildren()) { - NodeUtil.markFunctionsDeleted(script, compiler); - script.removeChildren(); - compiler.reportChangeToChangeScope(script); + NodeUtil.deleteChildren(script, compiler); } } @@ -136,25 +134,20 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { && !callee.matchesQualifiedName("goog.define") && !callee.matchesQualifiedName("goog.require") && !callee.matchesQualifiedName("goog.module")) { - n.detach(); - NodeUtil.markFunctionsDeleted(n, t.getCompiler()); - t.reportCodeChange(); + NodeUtil.deleteNode(n, t.getCompiler()); } return false; case ASSIGN: if (!expr.getFirstChild().isQualifiedName() || (expr.getFirstChild().isName() && !t.inGlobalScope() && !t.inModuleScope())) { - NodeUtil.removeChild(parent, n); - t.reportCodeChange(parent); - NodeUtil.markFunctionsDeleted(n, t.getCompiler()); + NodeUtil.deleteNode(n, t.getCompiler()); return false; } return true; case GETPROP: return true; default: - n.detach(); - t.reportCodeChange(); + NodeUtil.deleteNode(n, t.getCompiler()); return false; } case THROW: @@ -164,8 +157,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { case DEBUGGER: case EMPTY: if (NodeUtil.isStatementParent(parent)) { - NodeUtil.removeChild(parent, n); - t.reportCodeChange(); + NodeUtil.deleteNode(n, compiler); } return false; case LABEL: @@ -174,24 +166,23 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { case CASE: case WHILE: // First child can't have declaration. Statement itself will be removed post-order. - n.removeFirstChild(); + NodeUtil.deleteNode(n.getFirstChild(), compiler); return true; case TRY: case DO: // Second child can't have declarations. Statement itself will be removed post-order. - n.getSecondChild().detach(); + NodeUtil.deleteNode(n.getSecondChild(), compiler); return true; case FOR: - n.getSecondChild().detach(); + NodeUtil.deleteNode(n.getSecondChild(), compiler); // fall-through case FOR_OF: case FOR_IN: - n.getSecondChild().detach(); + NodeUtil.deleteNode(n.getSecondChild(), compiler); Node initializer = n.removeFirstChild(); if (initializer.isVar()) { n.getLastChild().addChildToFront(initializer); } - t.reportCodeChange(parent); return true; case CONST: case LET: @@ -409,8 +400,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { } else if (callee.matchesQualifiedName("goog.require")) { currentFile.markImportedName(expr.getLastChild().getString()); } else if (callee.matchesQualifiedName("goog.define")) { - expr.getLastChild().detach(); - t.reportCodeChange(); + NodeUtil.deleteNode(expr.getLastChild(), compiler); } else { checkState(callee.matchesQualifiedName("goog.module")); } @@ -508,8 +498,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { }); final Node functionBody = function.getLastChild(); checkState(functionBody.isNormalBlock()); - functionBody.removeChildren(); - compiler.reportChangeToEnclosingScope(functionBody); + NodeUtil.deleteChildren(functionBody, compiler); } enum RemovalType { diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index 4e375cf7c20..6c4d67893aa 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -2650,6 +2650,26 @@ static boolean isInSyntheticScript(Node n) { return n.getSourceFileName() != null && n.getSourceFileName().startsWith(" [synthetic:"); } + /** + * Permanently delete the given node from the AST, as well as report + * the related AST changes/deletions to the given compiler. + */ + public static void deleteNode(Node n, AbstractCompiler compiler) { + Node parent = n.getParent(); + NodeUtil.markFunctionsDeleted(n, compiler); + n.detach(); + compiler.reportChangeToEnclosingScope(parent); + } + + /** + * Permanently delete all the children of the given node, including reporting changes. + */ + public static void deleteChildren(Node n, AbstractCompiler compiler) { + while (n.hasChildren()) { + deleteNode(n.getFirstChild(), compiler); + } + } + /** * Safely remove children while maintaining a valid node structure. * In some cases, this is done by removing the parent from the AST as well. diff --git a/src/com/google/javascript/jscomp/PeepholeFoldConstants.java b/src/com/google/javascript/jscomp/PeepholeFoldConstants.java index 97e0e44d0c9..b3f484bee82 100644 --- a/src/com/google/javascript/jscomp/PeepholeFoldConstants.java +++ b/src/com/google/javascript/jscomp/PeepholeFoldConstants.java @@ -597,7 +597,7 @@ private Node tryFoldAndOr(Node n, Node left, Node right) { // or: false_with_sideeffects && foo() => false_with_sideeffects, foo() // This, combined with PeepholeRemoveDeadCode, helps reduce expressions // like "x() || false || z()". - n.detachChildren(); + NodeUtil.deleteChildren(n, compiler); result = IR.comma(left, right); } } @@ -607,7 +607,7 @@ private Node tryFoldAndOr(Node n, Node left, Node right) { if (result != null) { // Fold it! - n.detachChildren(); + NodeUtil.deleteChildren(n, compiler); parent.replaceChild(n, result); reportCodeChange(); diff --git a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java index 3e72189f790..b3b543c3225 100644 --- a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java +++ b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java @@ -864,10 +864,7 @@ private Node tryFoldHook(Node n) { boolean condHasSideEffects = mayHaveSideEffects(cond); // Must detach after checking for side effects, to ensure that the parents // of nodes are set correctly. - for (Node child = n.getFirstChild(); child != null; child = child.getNext()) { - NodeUtil.markFunctionsDeleted(child, compiler); - } - n.detachChildren(); + NodeUtil.deleteChildren(n, compiler); if (condHasSideEffects) { replacement = IR.comma(cond, branchToKeep).srcref(n);