From 8cdef4139e0a8cf9e71f5366898d211aca8d7251 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Mon, 9 Oct 2017 20:12:03 +0300 Subject: [PATCH] Fix JS DCE removing reachable functions --- .../jetbrains/kotlin/cli/js/dce/K2JSDce.kt | 2 +- .../kotlin/js/dce/ReachabilityTracker.kt | 24 +++++++++++++---- .../com/google/gwt/dev/js/JsAstMapper.java | 7 +++-- .../jetbrains/kotlin/js/test/BasicBoxTest.kt | 5 ++-- .../kotlin/js/test/DceTestGenerated.java | 18 +++++++++++++ .../extensionFunctionCalledFromFor.kt | 2 +- .../box/multideclaration/multiValForMap.kt | 2 +- .../box/multideclaration/multiValForRange.kt | 2 +- js/js.translator/testData/dce/arrayAccess.js | 17 ++++++++++++ .../testData/dce/localVarAndFunction.js | 27 +++++++++++++++++++ js/js.translator/testData/dce/typeOf.js | 15 +++++++++++ 11 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 js/js.translator/testData/dce/arrayAccess.js create mode 100644 js/js.translator/testData/dce/localVarAndFunction.js create mode 100644 js/js.translator/testData/dce/typeOf.js diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/js/dce/K2JSDce.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/js/dce/K2JSDce.kt index 5d0f078062e..fc5ee618f75 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/js/dce/K2JSDce.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/js/dce/K2JSDce.kt @@ -60,7 +60,7 @@ class K2JSDce : CLITool() { } val dceResult = DeadCodeElimination.run(files, includedDeclarations, logConsumer) if (dceResult.status == DeadCodeEliminationStatus.FAILED) return ExitCode.COMPILATION_ERROR - val nodes = dceResult.reachableNodes + val nodes = dceResult.reachableNodes.filterTo(mutableSetOf()) { it.reachable } val reachabilitySeverity = if (arguments.printReachabilityInfo) CompilerMessageSeverity.INFO else CompilerMessageSeverity.LOGGING messageCollector.report(reachabilitySeverity, "") diff --git a/js/js.dce/src/org/jetbrains/kotlin/js/dce/ReachabilityTracker.kt b/js/js.dce/src/org/jetbrains/kotlin/js/dce/ReachabilityTracker.kt index ba31541acfa..ea44bb14514 100644 --- a/js/js.dce/src/org/jetbrains/kotlin/js/dce/ReachabilityTracker.kt +++ b/js/js.dce/src/org/jetbrains/kotlin/js/dce/ReachabilityTracker.kt @@ -57,9 +57,21 @@ class ReachabilityTracker( analysisResult.nodeMap[x] == null && x !in analysisResult.astNodesToEliminate override fun visitNameRef(nameRef: JsNameRef) { - if (nameRef in analysisResult.astNodesToSkip) return + if (visitNameLikeNode(nameRef)) { + super.visitNameRef(nameRef) + } + } - val node = context.extractNode(nameRef) + override fun visitArrayAccess(x: JsArrayAccess) { + if (visitNameLikeNode(x)) { + super.visitArrayAccess(x) + } + } + + private fun visitNameLikeNode(x: JsExpression): Boolean { + if (x in analysisResult.astNodesToSkip) return false + + val node = context.extractNode(x) if (node != null) { if (!node.reachable) { reportAndNest("reach: referenced name $node", currentNodeWithLocation) { @@ -67,9 +79,10 @@ class ReachabilityTracker( currentNodeWithLocation?.let { node.usedByAstNodes += it } } } + return false } else { - super.visitNameRef(nameRef) + return true } } @@ -179,6 +192,7 @@ class ReachabilityTracker( } else if (!node.declarationReachable) { node.declarationReachable = true + reachableNodesImpl += node node.original.qualifier?.parent?.let { reportAndNest("reach-decl: parent $it", null) { @@ -197,8 +211,8 @@ class ReachabilityTracker( override fun visitPrefixOperation(x: JsPrefixOperation) { if (x.operator == JsUnaryOperator.TYPEOF) { val arg = x.arg - if (arg is JsNameRef && arg.qualifier == null) { - context.extractNode(arg)?.let { reachDeclaration(it) } + context.extractNode(arg)?.let { + reachDeclaration(it) return } } diff --git a/js/js.parser/src/com/google/gwt/dev/js/JsAstMapper.java b/js/js.parser/src/com/google/gwt/dev/js/JsAstMapper.java index b75d37f61e8..47628058e1a 100644 --- a/js/js.parser/src/com/google/gwt/dev/js/JsAstMapper.java +++ b/js/js.parser/src/com/google/gwt/dev/js/JsAstMapper.java @@ -562,15 +562,18 @@ public class JsAstMapper { Node fromFnNameNode = fnNode.getFirstChild(); Node fromParamNode = fnNode.getFirstChild().getNext().getFirstChild(); Node fromBodyNode = fnNode.getFirstChild().getNext().getNext(); - JsFunction toFn = scopeContext.enterFunction(); // Decide the function's name, if any. // String fnNameIdent = fromFnNameNode.getString(); + JsName functionName = null; if (fnNameIdent != null && fnNameIdent.length() > 0) { - toFn.setName(scopeContext.globalNameFor(fnNameIdent)); + functionName = scopeContext.localNameFor(fnNameIdent); } + JsFunction toFn = scopeContext.enterFunction(); + toFn.setName(functionName); + while (fromParamNode != null) { String fromParamName = fromParamNode.getString(); JsName name = scopeContext.localNameFor(fromParamName); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt index a96ac8355bb..e247999a393 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt @@ -542,13 +542,14 @@ abstract class BasicBoxTest( val testFunctionFqn = testModuleName + (if (testPackage.isNullOrEmpty()) "" else ".$testPackage") + ".$testFunction" val additionalReachableNodes = setOf( testFunctionFqn, "kotlin.kotlin.io.BufferedOutput", "kotlin.kotlin.io.output.flush", - "kotlin.kotlin.io.output.buffer", "kotlin-test.kotlin.test.overrideAsserter_wbnzx$" + "kotlin.kotlin.io.output.buffer", "kotlin-test.kotlin.test.overrideAsserter_wbnzx$", + "kotlin-test.kotlin.test.DefaultAsserter" ) val allFilesToMinify = filesToMinify.values + kotlinJsInputFile + kotlinTestJsInputFile val dceResult = DeadCodeElimination.run(allFilesToMinify, additionalReachableNodes) { _, _ -> } val reachableNodes = dceResult.reachableNodes - minificationThresholdChecker(reachableNodes.size) + minificationThresholdChecker(reachableNodes.count { it.reachable }) val runList = mutableListOf() runList += kotlinJsLibOutput diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/DceTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/DceTestGenerated.java index 7f36ea54e35..4961eda9af0 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/DceTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/DceTestGenerated.java @@ -42,6 +42,12 @@ public class DceTestGenerated extends AbstractDceTest { doTest(fileName); } + @TestMetadata("arrayAccess.js") + public void testArrayAccess() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/dce/arrayAccess.js"); + doTest(fileName); + } + @TestMetadata("commonjs.js") public void testCommonjs() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/dce/commonjs.js"); @@ -53,4 +59,16 @@ public class DceTestGenerated extends AbstractDceTest { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/dce/cycle.js"); doTest(fileName); } + + @TestMetadata("localVarAndFunction.js") + public void testLocalVarAndFunction() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/dce/localVarAndFunction.js"); + doTest(fileName); + } + + @TestMetadata("typeOf.js") + public void testTypeOf() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/dce/typeOf.js"); + doTest(fileName); + } } diff --git a/js/js.translator/testData/box/extensionFunction/extensionFunctionCalledFromFor.kt b/js/js.translator/testData/box/extensionFunction/extensionFunctionCalledFromFor.kt index a5724ea0686..7e373845bb7 100644 --- a/js/js.translator/testData/box/extensionFunction/extensionFunctionCalledFromFor.kt +++ b/js/js.translator/testData/box/extensionFunction/extensionFunctionCalledFromFor.kt @@ -1,4 +1,4 @@ -// EXPECTED_REACHABLE_NODES: 1230 +// EXPECTED_REACHABLE_NODES: 1106 package foo class SimpleEnumerator { diff --git a/js/js.translator/testData/box/multideclaration/multiValForMap.kt b/js/js.translator/testData/box/multideclaration/multiValForMap.kt index 79570f21038..76ee3e70691 100644 --- a/js/js.translator/testData/box/multideclaration/multiValForMap.kt +++ b/js/js.translator/testData/box/multideclaration/multiValForMap.kt @@ -1,4 +1,4 @@ -// EXPECTED_REACHABLE_NODES: 1222 +// EXPECTED_REACHABLE_NODES: 1098 package foo diff --git a/js/js.translator/testData/box/multideclaration/multiValForRange.kt b/js/js.translator/testData/box/multideclaration/multiValForRange.kt index bd217f167a3..b151b55141a 100644 --- a/js/js.translator/testData/box/multideclaration/multiValForRange.kt +++ b/js/js.translator/testData/box/multideclaration/multiValForRange.kt @@ -1,4 +1,4 @@ -// EXPECTED_REACHABLE_NODES: 1249 +// EXPECTED_REACHABLE_NODES: 1123 package foo class C(val i: Int) : Comparable, A() { diff --git a/js/js.translator/testData/dce/arrayAccess.js b/js/js.translator/testData/dce/arrayAccess.js new file mode 100644 index 00000000000..c5135d0b371 --- /dev/null +++ b/js/js.translator/testData/dce/arrayAccess.js @@ -0,0 +1,17 @@ +(function(_) { + var x = {}; + function foo() { + return x["bar"](); + } + x.bar = function() { + return "bar"; + }; + x.baz = function() { + return "baz"; + }; + _.foo = foo; +})(module.exports); + +// REQUEST_REACHABLE: main.foo +// ASSERT_REACHABLE: x.bar +// ASSERT_UNREACHABLE: x.baz \ No newline at end of file diff --git a/js/js.translator/testData/dce/localVarAndFunction.js b/js/js.translator/testData/dce/localVarAndFunction.js new file mode 100644 index 00000000000..ed818f029b6 --- /dev/null +++ b/js/js.translator/testData/dce/localVarAndFunction.js @@ -0,0 +1,27 @@ +(function(_, Kotlin) { + Kotlin.defineInlineFunction("bar", Kotlin.wrapFunction(function() { + function baz() { + return "bar"; + } + return function() { + return baz(); + }; + })); + return _; +})(module.exports, require("kotlin")); + +(function(_) { + function baz() { + return "baz"; + } + + function foo() { + return baz(); + } + _.foo = foo; +})(module.exports); + + +// REQUEST_REACHABLE: main.foo +// ASSERT_REACHABLE: main.foo +// ASSERT_REACHABLE: baz \ No newline at end of file diff --git a/js/js.translator/testData/dce/typeOf.js b/js/js.translator/testData/dce/typeOf.js new file mode 100644 index 00000000000..ac971fa1962 --- /dev/null +++ b/js/js.translator/testData/dce/typeOf.js @@ -0,0 +1,15 @@ +(function(_) { + var x = {}; + x.y = {}; + x.y.bar = function() { + return "bar"; + }; + function foo() { + return typeof x.y; + } + _.foo = foo; +})(module.exports); + +// REQUEST_REACHABLE: main.foo +// ASSERT_REACHABLE: x.y +// ASSERT_UNREACHABLE: x.y.bar \ No newline at end of file