From 76346732131a98a5df0027e894701b17dc7fddf9 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Mon, 10 Jul 2017 19:43:05 +0300 Subject: [PATCH] Fix module re-importing when inlining functions in JS BE When inliner reads function's body from other module, it performs substitution _ -> moduleAlias. However, local alias can't be used for this purpose, since call site can be in public inline function itself, so the correct substitution would be -> _.$$imports$$.alias --- .../kotlin/js/inline/FunctionReader.kt | 2 +- .../js/test/semantics/BoxJsTestGenerated.java | 6 ++ .../js/translate/context/StaticContext.java | 56 ++++++++++++++--- .../translate/context/TranslationContext.java | 63 +++---------------- .../inlineMultiModule/reexportDuringInline.kt | 16 +++++ 5 files changed, 80 insertions(+), 63 deletions(-) create mode 100644 js/js.translator/testData/box/inlineMultiModule/reexportDuringInline.kt diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt index 4f0d954c5d4..d6fbe67979c 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt @@ -194,7 +194,7 @@ class FunctionReader( val position = info.offsetToSourceMapping[offset] val function = parseFunction(source, info.filePath, position, offset, ThrowExceptionOnErrorReporter, JsRootScope(JsProgram())) - val moduleReference = moduleNameMap[tag] ?: currentModuleName.makeRef() + val moduleReference = moduleNameMap[tag]?.deepCopy() ?: currentModuleName.makeRef() val sourceMap = info.sourceMap if (sourceMap != null) { diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java index 6883235fcf3..f289e50a8a2 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java @@ -4778,6 +4778,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { doTest(fileName); } + @TestMetadata("reexportDuringInline.kt") + public void testReexportDuringInline() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/inlineMultiModule/reexportDuringInline.kt"); + doTest(fileName); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/inlineMultiModule/simple.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java index 1e15a0a6414..41b7938100d 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java @@ -33,6 +33,7 @@ import org.jetbrains.kotlin.js.backend.ast.metadata.MetadataProperties; import org.jetbrains.kotlin.js.backend.ast.metadata.SideEffectKind; import org.jetbrains.kotlin.js.config.JsConfig; import org.jetbrains.kotlin.js.naming.NameSuggestion; +import org.jetbrains.kotlin.js.naming.NameSuggestionKt; import org.jetbrains.kotlin.js.naming.SuggestedName; import org.jetbrains.kotlin.js.translate.context.generator.Generator; import org.jetbrains.kotlin.js.translate.context.generator.Rule; @@ -48,16 +49,12 @@ import org.jetbrains.kotlin.resolve.BindingTrace; import org.jetbrains.kotlin.resolve.DescriptorUtils; import org.jetbrains.kotlin.resolve.calls.tasks.DynamicCallsKt; import org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilsKt; -import org.jetbrains.kotlin.resolve.source.KotlinSourceElement; import org.jetbrains.kotlin.resolve.source.KotlinSourceElementKt; import org.jetbrains.kotlin.serialization.js.ModuleKind; import org.jetbrains.kotlin.types.KotlinType; import org.jetbrains.kotlin.types.typeUtil.TypeUtilsKt; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import static org.jetbrains.kotlin.js.config.JsConfig.UNKNOWN_EXTERNAL_MODULE_NAME; import static org.jetbrains.kotlin.js.translate.utils.AnnotationsUtils.isLibraryObject; @@ -142,6 +139,8 @@ public final class StaticContext { @Nullable private JsName nameForImportsForInline; + private final Map modulesImportedForInline = new HashMap<>(); + public StaticContext( @NotNull BindingTrace bindingTrace, @NotNull JsConfig config, @@ -707,7 +706,7 @@ public final class StaticContext { } @NotNull - public static String suggestModuleName(@NotNull ModuleDescriptor module) { + private static String suggestModuleName(@NotNull ModuleDescriptor module) { if (module == module.getBuiltIns().getBuiltInsModule()) { return Namer.KOTLIN_LOWER_NAME; } @@ -800,11 +799,15 @@ public final class StaticContext { public void addInlineCall(@NotNull CallableDescriptor descriptor) { String tag = Namer.getFunctionTag(descriptor, config); - fragment.getInlineModuleMap().put(tag, getModuleExpressionFor(descriptor)); + JsExpression moduleExpression = exportModuleForInline(DescriptorUtils.getContainingModule(descriptor)); + if (moduleExpression == null) { + moduleExpression = getModuleExpressionFor(descriptor); + } + fragment.getInlineModuleMap().put(tag, moduleExpression); } @NotNull - public JsName getNameForImportsForInline() { + private JsName getNameForImportsForInline() { if (nameForImportsForInline == null) { JsName name = JsScope.declareTemporaryName(Namer.IMPORTS_FOR_INLINE_PROPERTY); fragment.getNameBindings().add(new JsNameBinding(Namer.IMPORTS_FOR_INLINE_PROPERTY, name)); @@ -815,4 +818,41 @@ public final class StaticContext { return nameForImportsForInline; } } + + @Nullable + public JsExpression exportModuleForInline(@NotNull ModuleDescriptor declaration) { + if (getCurrentModule().getBuiltIns().getBuiltInsModule() == declaration) return null; + + String moduleName = suggestModuleName(declaration); + if (moduleName.equals(Namer.KOTLIN_LOWER_NAME)) return null; + + return exportModuleForInline(moduleName, getInnerNameForDescriptor(declaration)); + } + + @NotNull + public JsExpression exportModuleForInline(@NotNull String moduleId, @NotNull JsName moduleName) { + return modulesImportedForInline.computeIfAbsent(moduleId, k -> { + JsExpression currentModuleRef = pureFqn(getInnerNameForDescriptor(getCurrentModule()), null); + JsExpression importsRef = pureFqn(Namer.IMPORTS_FOR_INLINE_PROPERTY, currentModuleRef); + JsExpression currentImports = pureFqn(getNameForImportsForInline(), null); + + JsExpression moduleRef; + JsExpression lhsModuleRef; + if (NameSuggestionKt.isValidES5Identifier(moduleId)) { + moduleRef = pureFqn(moduleId, importsRef); + lhsModuleRef = pureFqn(moduleId, currentImports); + } + else { + moduleRef = new JsArrayAccess(importsRef, new JsStringLiteral(moduleId)); + MetadataProperties.setSideEffects(moduleRef, SideEffectKind.PURE); + lhsModuleRef = new JsArrayAccess(currentImports, new JsStringLiteral(moduleId)); + } + + JsExpressionStatement importStmt = new JsExpressionStatement(JsAstUtils.assignment(lhsModuleRef, moduleName.makeRef())); + MetadataProperties.setExportedTag(importStmt, "imports:" + moduleId); + getFragment().getExportBlock().getStatements().add(importStmt); + + return moduleRef; + }).deepCopy(); + } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java index 448760217e1..f7bafa508c4 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java @@ -26,9 +26,7 @@ import org.jetbrains.kotlin.descriptors.impl.TypeAliasConstructorDescriptor; import org.jetbrains.kotlin.incremental.components.NoLookupLocation; import org.jetbrains.kotlin.js.backend.ast.*; import org.jetbrains.kotlin.js.backend.ast.metadata.MetadataProperties; -import org.jetbrains.kotlin.js.backend.ast.metadata.SideEffectKind; import org.jetbrains.kotlin.js.config.JsConfig; -import org.jetbrains.kotlin.js.naming.NameSuggestionKt; import org.jetbrains.kotlin.js.naming.SuggestedName; import org.jetbrains.kotlin.js.translate.intrinsic.Intrinsics; import org.jetbrains.kotlin.js.translate.reference.CallExpressionTranslator; @@ -74,8 +72,6 @@ public class TranslationContext { @Nullable private final VariableDescriptor continuationParameterDescriptor; - private final Set modulesImportedForInline = new HashSet<>(); - @NotNull public static TranslationContext rootContext(@NotNull StaticContext staticContext) { DynamicContext rootDynamicContext = DynamicContext.rootContext( @@ -284,9 +280,11 @@ public class TranslationContext { } else { ModuleDescriptor module = DescriptorUtils.getContainingModule(descriptor); - ModuleDescriptor currentModule = staticContext.getCurrentModule(); - if (module != currentModule && !isInlineFunction(descriptor)) { - result = exportModuleForInline(currentModule, module, result); + if (module != staticContext.getCurrentModule() && !isInlineFunction(descriptor)) { + JsExpression replacement = staticContext.exportModuleForInline(module); + if (replacement != null) { + result = replaceModuleReference(result, getInnerNameForDescriptor(module), replacement); + } } } } @@ -298,50 +296,6 @@ public class TranslationContext { return CallExpressionTranslator.shouldBeInlined((CallableDescriptor) descriptor, this); } - @NotNull - private JsExpression exportModuleForInline( - @NotNull ModuleDescriptor currentModule, @NotNull ModuleDescriptor module, - @NotNull JsExpression fqn - ) { - if (currentModule.getBuiltIns().getBuiltInsModule() == module) return fqn; - - String moduleName = StaticContext.suggestModuleName(module); - if (moduleName.equals(Namer.KOTLIN_LOWER_NAME)) return fqn; - - return exportModuleForInline(currentModule, moduleName, staticContext.getInnerNameForDescriptor(module), fqn); - } - - private JsExpression exportModuleForInline( - @NotNull ModuleDescriptor currentModule, - @NotNull String moduleId, @NotNull JsName moduleName, - @NotNull JsExpression fqn) { - JsExpression currentModuleRef = pureFqn(staticContext.getInnerNameForDescriptor(currentModule), null); - JsExpression importsRef = pureFqn(Namer.IMPORTS_FOR_INLINE_PROPERTY, currentModuleRef); - JsExpression currentImports = pureFqn(staticContext.getNameForImportsForInline(), null); - - JsExpression moduleRef; - JsExpression lhsModuleRef; - if (NameSuggestionKt.isValidES5Identifier(moduleId)) { - moduleRef = pureFqn(moduleId, importsRef); - lhsModuleRef = pureFqn(moduleId, currentImports); - } - else { - moduleRef = new JsArrayAccess(importsRef, new JsStringLiteral(moduleId)); - MetadataProperties.setSideEffects(moduleRef, SideEffectKind.PURE); - lhsModuleRef = new JsArrayAccess(currentImports, new JsStringLiteral(moduleId)); - } - - fqn = replaceModuleReference(fqn, moduleName, moduleRef); - - if (modulesImportedForInline.add(moduleId)) { - JsExpressionStatement importStmt = new JsExpressionStatement(JsAstUtils.assignment(lhsModuleRef, moduleName.makeRef())); - MetadataProperties.setExportedTag(importStmt, "imports:" + moduleId); - staticContext.getFragment().getExportBlock().getStatements().add(importStmt); - } - - return fqn; - } - private static JsExpression replaceModuleReference( @NotNull JsExpression expression, @NotNull JsName expectedModuleName, @@ -378,14 +332,15 @@ public class TranslationContext { if (suggested != null && getConfig().getModuleKind() != ModuleKind.PLAIN && isPublicInlineFunction()) { String moduleId = AnnotationsUtils.getModuleName(suggested.getDescriptor()); if (moduleId != null) { - result = exportModuleForInline(getCurrentModule(), moduleId, name, result); + JsExpression replacement = staticContext.exportModuleForInline(moduleId, name); + result = replaceModuleReference(result, name, replacement); } else if (isNativeObject(suggested.getDescriptor()) && DescriptorUtils.isTopLevelDeclaration(suggested.getDescriptor())) { String fileModuleId = AnnotationsUtils.getFileModuleName(bindingContext(), suggested.getDescriptor()); if (fileModuleId != null) { JsName fileModuleName = staticContext.getImportedModule(fileModuleId, null).getInternalName(); - result = exportModuleForInline(getCurrentModule(), fileModuleId, fileModuleName, - staticContext.getQualifiedReference(descriptor)); + JsExpression replacement = staticContext.exportModuleForInline(fileModuleId, fileModuleName); + result = replaceModuleReference(staticContext.getQualifiedReference(descriptor), fileModuleName, replacement); } } } diff --git a/js/js.translator/testData/box/inlineMultiModule/reexportDuringInline.kt b/js/js.translator/testData/box/inlineMultiModule/reexportDuringInline.kt new file mode 100644 index 00000000000..5908d1c0500 --- /dev/null +++ b/js/js.translator/testData/box/inlineMultiModule/reexportDuringInline.kt @@ -0,0 +1,16 @@ +// EXPECTED_REACHABLE_NODES: 490 +// MODULE: lib1 +// FILE: lib1.kt +inline fun foo() = bar() + +fun bar() = "OK" + +// MODULE: lib2(lib1) +// FILE: lib2.kt + +inline fun baz() = foo() + +// MODULE: main(lib2) +// FILE: main.kt + +fun box() = baz() \ No newline at end of file