From f4a9b99e4d0aa12279832072bd1d64a8d4561048 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 28 Feb 2017 20:41:27 +0300 Subject: [PATCH] JS: fix incremental generation of source maps --- .../backend/JsToStringGenerationVisitor.java | 4 ++- .../kotlin/js/backend/ast/JsProgram.java | 27 ++--------------- .../serialization/js/ast/JsAstDeserializer.kt | 29 +++++++++---------- .../serialization/js/ast/JsAstSerializer.kt | 20 ++++++------- .../jetbrains/kotlin/js/test/BasicBoxTest.kt | 21 +++++++++----- .../js/facade/SourceMapBuilderConsumer.java | 25 +++++++++------- 6 files changed, 56 insertions(+), 70 deletions(-) diff --git a/js/js.ast/src/org/jetbrains/kotlin/js/backend/JsToStringGenerationVisitor.java b/js/js.ast/src/org/jetbrains/kotlin/js/backend/JsToStringGenerationVisitor.java index e4f93583afa..a37e4941834 100644 --- a/js/js.ast/src/org/jetbrains/kotlin/js/backend/JsToStringGenerationVisitor.java +++ b/js/js.ast/src/org/jetbrains/kotlin/js/backend/JsToStringGenerationVisitor.java @@ -464,7 +464,9 @@ public class JsToStringGenerationVisitor extends JsVisitor { rightParen(); nestedPush(x.getBody()); - accept(x.getBody()); + if (x.getBody() != null) { + accept(x.getBody()); + } nestedPop(x.getBody()); } diff --git a/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/JsProgram.java b/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/JsProgram.java index 1cbe6db24b3..e1e63a2961b 100644 --- a/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/JsProgram.java +++ b/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/JsProgram.java @@ -19,11 +19,7 @@ import java.util.Map; public final class JsProgram extends SourceInfoAwareJsNode { private final JsGlobalBlock globalBlock = new JsGlobalBlock(); - private final TDoubleObjectHashMap doubleLiteralMap = new TDoubleObjectHashMap(); - private final TIntObjectHashMap intLiteralMap = new TIntObjectHashMap(); - private final JsRootScope rootScope; - private final Map stringLiteralMap = new THashMap(); private final JsObjectScope topScope; public JsProgram() { @@ -36,23 +32,11 @@ public final class JsProgram extends SourceInfoAwareJsNode { } public JsNumberLiteral getNumberLiteral(double value) { - JsDoubleLiteral literal = doubleLiteralMap.get(value); - if (literal == null) { - literal = new JsDoubleLiteral(value); - doubleLiteralMap.put(value, literal); - } - - return literal; + return new JsDoubleLiteral(value); } public JsNumberLiteral getNumberLiteral(int value) { - JsIntLiteral literal = intLiteralMap.get(value); - if (literal == null) { - literal = new JsIntLiteral(value); - intLiteralMap.put(value, literal); - } - - return literal; + return new JsIntLiteral(value); } /** @@ -77,12 +61,7 @@ public final class JsProgram extends SourceInfoAwareJsNode { */ @NotNull public JsStringLiteral getStringLiteral(String value) { - JsStringLiteral literal = stringLiteralMap.get(value); - if (literal == null) { - literal = new JsStringLiteral(value); - stringLiteralMap.put(value, literal); - } - return literal; + return new JsStringLiteral(value); } @Override diff --git a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstDeserializer.kt b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstDeserializer.kt index ab72b067dbc..4254d6d9671 100644 --- a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstDeserializer.kt +++ b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstDeserializer.kt @@ -31,7 +31,7 @@ class JsAstDeserializer(private val program: JsProgram) { private val stringTable = mutableListOf() private val nameTable = mutableListOf() private val nameCache = mutableListOf() - private val locationStack: Deque = ArrayDeque() + private val fileStack: Deque = ArrayDeque() fun deserialize(input: InputStream): JsProgramFragment { return deserialize(Chunk.parseFrom(CodedInputStream.newInstance(input).apply { setRecursionLimit(4096) })) @@ -491,28 +491,27 @@ class JsAstDeserializer(private val program: JsProgram) { } private fun withLocation(fileId: Int?, location: Location?, action: () -> T): T { - val lastLocation = locationStack.peek() - val deserializedLocation = if (fileId != null || location != null) { - val file = fileId?.let { deserializeString(it) } ?: lastLocation?.file!! - val (startLine, startChar) = if (location != null) { - Pair(location.startLine, location.startChar) - } - else { - Pair(lastLocation.startLine, lastLocation.startChar) - } - JsLocation(file, startLine, startChar) + val deserializedFile = fileId?.let { deserializeString(it) } + val file = deserializedFile ?: fileStack.peek() + val deserializedLocation = if (file != null && location != null) { + JsLocation(file, location.startLine, location.startChar) } else { null } - if (deserializedLocation != null) { - locationStack.push(deserializedLocation) + val shouldUpdateFile = location != null && deserializedFile != null && deserializedFile != fileStack.peek() + if (shouldUpdateFile) { + fileStack.push(deserializedFile) } val node = action() if (deserializedLocation != null) { - node.source = deserializedLocation - locationStack.pop() + if (node !is JsLiteral.JsBooleanLiteral && node !is JsLiteral.JsThisRef && node !is JsNullLiteral) { + node.source = deserializedLocation + } + } + if (shouldUpdateFile) { + fileStack.pop() } return node diff --git a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstSerializer.kt b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstSerializer.kt index b136c492548..77bb714ddfa 100644 --- a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstSerializer.kt +++ b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstSerializer.kt @@ -31,7 +31,7 @@ class JsAstSerializer { private val stringTableBuilder = StringTable.newBuilder() private val nameMap = mutableMapOf() private val stringMap = mutableMapOf() - private val locationStack: Deque = ArrayDeque() + private val fileStack: Deque = ArrayDeque() fun serialize(fragment: JsProgramFragment, output: OutputStream) { serialize(fragment).writeTo(output) @@ -561,27 +561,25 @@ class JsAstSerializer { } private inline fun withLocation(node: JsNode, fileConsumer: (Int) -> Unit, locationConsumer: (Location) -> Unit, inner: () -> Unit) { - val lastLocation = locationStack.peek() val location = extractLocation(node) - val locationStackModified = if (lastLocation != location && location != null) { - locationStack.push(location) - if (lastLocation == null || lastLocation.file != location.file) { + var fileChanged = false + if (location != null) { + val lastFile = fileStack.peek() + fileChanged = lastFile != location.file + if (fileChanged) { fileConsumer(serialize(location.file)) + fileStack.push(location.file) } val locationBuilder = Location.newBuilder() locationBuilder.startLine = location.startLine locationBuilder.startChar = location.startChar locationConsumer(locationBuilder.build()) - true - } - else { - false } inner() - if (locationStackModified) { - locationStack.pop() + if (fileChanged) { + fileStack.pop() } } 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 f7c90c1bd3e..0d9c9c6e865 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 @@ -270,10 +270,17 @@ abstract class BasicBoxTest( translateFiles(allTranslationUnits, recompiledOutputFile, recompiledConfig) val originalOutput = FileUtil.loadFile(outputFile) - val recompiledOutput = FileUtil.loadFile(recompiledOutputFile) + val recompiledOutput = removeRecompiledSuffix(FileUtil.loadFile(recompiledOutputFile)) TestCase.assertEquals("Output file changed after recompilation", originalOutput, recompiledOutput) + + val originalSourceMap = FileUtil.loadFile(File(outputFile.parentFile, outputFile.name + ".map")) + val recompiledSourceMap = removeRecompiledSuffix( + FileUtil.loadFile(File(recompiledOutputFile.parentFile, recompiledOutputFile.name + ".map"))) + TestCase.assertEquals("Source map file changed after recompilation", originalSourceMap, recompiledSourceMap) } - }protected fun translateFiles(units: List, outputFile: File, config: JsConfig, + } + + private fun removeRecompiledSuffix(text: String): String = text.replace("-recompiled.js", ".js")protected fun translateFiles(units: List, outputFile: File, config: JsConfig, outputPrefixFile: File?, outputPostfixFile: File?, mainCallParameters: MainCallParameters) { @@ -362,9 +369,11 @@ abstract class BasicBoxTest( configuration.put(JSConfigurationKeys.TARGET, EcmaVersion.v5) configuration.put(JSConfigurationKeys.SOURCE_MAP, generateSourceMap) + val hasFilesToRecompile = module.hasFilesToRecompile configuration.put(JSConfigurationKeys.META_INFO, multiModule) configuration.put(JSConfigurationKeys.SERIALIZE_FRAGMENTS, hasFilesToRecompile) + configuration.put(JSConfigurationKeys.SOURCE_MAP, hasFilesToRecompile) if (additionalMetadata != null) { configuration.put(JSConfigurationKeys.FALLBACK_METADATA, additionalMetadata.map { FileUtil.loadFileBytes(it) }) @@ -413,9 +422,7 @@ abstract class BasicBoxTest( currentModule.languageVersion = LanguageVersion.fromVersionString(version) } - return TestFile(temporaryFile.absolutePath, currentModule).apply { - recompile = RECOMPILE_PATTERN.matcher(text).find() - } + return TestFile(temporaryFile.absolutePath, currentModule, recompile = RECOMPILE_PATTERN.matcher(text).find()) } override fun createModule(name: String, dependencies: List): TestModule? { @@ -427,12 +434,10 @@ abstract class BasicBoxTest( } } - private class TestFile(val fileName: String, val module: TestModule) { + private class TestFile(val fileName: String, val module: TestModule, val recompile: Boolean) { init { module.files += this } - - var recompile = false } private class TestModule( diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/facade/SourceMapBuilderConsumer.java b/js/js.translator/src/org/jetbrains/kotlin/js/facade/SourceMapBuilderConsumer.java index 5db76012fb9..d0f6f90721a 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/facade/SourceMapBuilderConsumer.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/facade/SourceMapBuilderConsumer.java @@ -20,22 +20,25 @@ import com.intellij.openapi.editor.Document; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; import com.intellij.util.PairConsumer; +import org.jetbrains.kotlin.js.backend.ast.JsLocation; import org.jetbrains.kotlin.js.sourceMap.SourceMapBuilder; class SourceMapBuilderConsumer implements PairConsumer { @Override public void consume(SourceMapBuilder builder, Object sourceInfo) { - if (!(sourceInfo instanceof PsiElement)) { - return; + if (sourceInfo instanceof PsiElement) { + PsiElement element = (PsiElement) sourceInfo; + PsiFile file = element.getContainingFile(); + int offset = element.getNode().getStartOffset(); + Document document = file.getViewProvider().getDocument(); + assert document != null; + int line = document.getLineNumber(offset); + int column = offset - document.getLineStartOffset(line); + builder.addMapping(file.getViewProvider().getVirtualFile().getPath(), line, column); + } + else if (sourceInfo instanceof JsLocation) { + JsLocation location = (JsLocation) sourceInfo; + builder.addMapping(location.getFile(), location.getStartLine(), location.getStartChar()); } - - PsiElement element = (PsiElement) sourceInfo; - PsiFile file = element.getContainingFile(); - int offset = element.getNode().getStartOffset(); - Document document = file.getViewProvider().getDocument(); - assert document != null; - int line = document.getLineNumber(offset); - int column = offset - document.getLineStartOffset(line); - builder.addMapping(file.getViewProvider().getVirtualFile().getPath(), line, column); } }