JS: FunctionReader cleanup

js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionReader.kt

@@ -83,8 +83,8 @@ class FunctionReader(
         val offsetToSourceMapping by lazy(offsetToSourceMappingProvider)
 
         val wrapFunctionRegex = specialFunctions.entries
-            .filter { (_, v) -> v == SpecialFunction.WRAP_FUNCTION } // TODO This is a hack! Investigate duplicates!
+            .singleOrNull { (_, v) -> v == SpecialFunction.WRAP_FUNCTION }?.key
-            .map { Regex("\\s*${it.key}\\s*\\(\\s*").toPattern() }
+            ?.let { Regex("\\s*$it\\s*\\(\\s*").toPattern() }
     }
 
     private val moduleNameToInfo by lazy {
@@ -224,11 +224,9 @@ class FunctionReader(
         return null
     }
 
-    // TODO move renamings to a proper place
     private fun readFunctionFromSource(descriptor: CallableDescriptor, info: ModuleInfo): FunctionWithWrapper? {
         val source = info.fileContent
         var tag = Namer.getFunctionTag(descriptor, config)
-//        val tagForModule = tag
         var index = source.indexOf(tag)
 
         // Hack for compatibility with old versions of stdlib
@@ -247,30 +245,21 @@ class FunctionReader(
         }
 
         val sourcePart = ShallowSubSequence(source, offset, source.length)
-        var isWrapped = false
+        val wrapFunctionMatcher = info.wrapFunctionRegex?.matcher(sourcePart)
-        for (regex in info.wrapFunctionRegex) {
+        val isWrapped = wrapFunctionMatcher?.lookingAt() == true
-            val wrapFunctionMatcher = regex.matcher(sourcePart)
+        if (isWrapped) {
-            isWrapped = wrapFunctionMatcher.lookingAt() == true
+            offset += wrapFunctionMatcher!!.end()
-            if (isWrapped) {
-                offset += wrapFunctionMatcher!!.end()
-                break
-            }
         }
 
         val position = info.offsetToSourceMapping[offset]
         val jsScope = JsRootScope(JsProgram())
-        val functionExpr = try {
+        val functionExpr = parseFunction(source, info.filePath, position, offset, ThrowExceptionOnErrorReporter, jsScope) ?: return null
-            parseFunction(source, info.filePath, position, offset, ThrowExceptionOnErrorReporter, jsScope) ?: return null
-        } catch (t: Throwable) {
-            throw Error("Exception while reading function '$tag' from ${info.filePath}", t)
-        }
         functionExpr.fixForwardNameReferences()
         val (function, wrapper) = if (isWrapped) {
             InlineMetadata.decomposeWrapper(functionExpr) ?: return null
         } else {
             FunctionWithWrapper(functionExpr, null)
         }
-//        val moduleReference = moduleNameMap[tagForModule]?.deepCopy() ?: currentModuleName.makeRef()
         val wrapperStatements = wrapper?.statements?.filter { it !is JsReturn }
 
         val sourceMap = info.sourceMap
@@ -283,14 +272,9 @@ class FunctionReader(
         }
 
         val allDefinedNames = collectDefinedNamesInAllScopes(function)
-//        val replacements = hashMapOf(info.moduleVariable to moduleReference,
-//                                     info.kotlinVariable to Namer.kotlinObject())
-//        replaceExternalNames(function, replacements, allDefinedNames)
-//        wrapperStatements?.forEach { replaceExternalNames(it, replacements, allDefinedNames) }
 
         function.markInlineArguments(descriptor)
         markDefaultParams(function)
-        // TODO maybe move elsewhere?
         markSpecialFunctions(function, allDefinedNames, info, jsScope)
 
         val namesWithoutSideEffects = wrapperStatements.orEmpty().asSequence()

js/js.inliner/src/org/jetbrains/kotlin/js/inline/InlinerImpl.kt

@@ -15,7 +15,6 @@ import org.jetbrains.kotlin.js.inline.util.refreshLabelNames
 import org.jetbrains.kotlin.js.translate.expression.InlineMetadata
 import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
 
-// TODO stateless?
 class InlinerImpl(
     val jsInliner: JsInliner,
     val scope: InliningScope
@@ -89,8 +88,6 @@ class InlinerImpl(
         }
     }
 
-    // TODO This could be extracted into a separate pass.
-    // TODO Don't forget to run it on the freshly inlined code!
     override fun doAcceptStatementList(statements: MutableList<JsStatement>) {
         var i = 0
 
@@ -114,7 +111,6 @@ class InlinerImpl(
     private fun patchReturnsFromSecondaryConstructor(function: JsFunction) {
         // Support non-local return from secondary constructor
         // Returns from secondary constructors should return `$this` object.
-        // TODO This seems brittle
         function.forcedReturnVariable?.let { returnVariable ->
             function.body.accept(object : RecursiveJsVisitor() {
                 override fun visitReturn(x: JsReturn) {

js/js.inliner/src/org/jetbrains/kotlin/js/inline/InliningScope.kt

@@ -45,8 +45,6 @@ sealed class InliningScope {
     fun importFunctionDefinition(definition: InlineFunctionDefinition): JsFunction {
         // Apparently we should avoid this trick when we implement fair support for crossinline
         // That's because crossinline lambdas inline into the declaration block and specialize those.
-
-        // TODO cache the function itself instead
         val result = computeIfAbsent(definition.tag, definition.fn.function) {
             val newReplacements = HashMap<JsName, JsNameRef>()
 
@@ -146,9 +144,7 @@ class ProgramFragmentInliningScope(
     private val additionalDeclarations = mutableListOf<JsStatement>()
 
     override fun update() {
-        // TODO fix the order
+        // TODO fix the order?
-        // TODO this probably will be replaced with a special tag -> block map for the imported stuff, so that we can merge same imports.
-        // TODO in that case this method will become obsolete
         fragment.declarationBlock.statements.addAll(0, additionalDeclarations)
 
         // post-processing
@@ -161,7 +157,6 @@ class ProgramFragmentInliningScope(
         removeUnusedImports(fragment)
     }
 
-    // TODO !!!!! This localAlias thing will get copied, inlined, and lost during IC =(
     override fun hasImport(name: JsName, tag: String): JsName? {
         return name.localAlias?.let {(name, tag) ->
             if (tag != null) {
@@ -217,7 +212,6 @@ class ProgramFragmentInliningScope(
     private fun addInlinedModule(module: JsImportedModule): JsName {
         return existingModules.computeIfAbsent(module.key) {
             // Copy so that the Merger.kt doesn't operate on the same instance in different fragments.
-            // TODO What about nameBindings?
             JsImportedModule(module.externalName, module.internalName, module.plainReference).also {
                 fragment.importedModules.add(it)
             }

js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.kt

@@ -82,7 +82,6 @@ class JsInliner(
         InlinerImpl(this@JsInliner, this).accept(node)
     }
 
-    // TODO a lot of code... Probably a bad sign
     fun inline(scope: InliningScope, call: JsInvocation, currentStatement: JsStatement?): InlineableResult {
         val definition = functionContext.getFunctionDefinition(call, scope)