From 24f2121f9b64eb48ec6f86b2bfec9bcb11db1341 Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov Date: Tue, 10 Mar 2015 14:59:30 +0300 Subject: [PATCH] JS: report error from backend, when inline function is called recursively --- .../js/ast/metadata/metadataProperties.kt | 5 +- .../diagnostics/DefaultErrorMessagesJs.kt | 2 +- .../js/resolve/diagnostics/ErrorsJs.java | 2 + js/js.inliner/js.inliner.iml | 1 + .../jetbrains/kotlin/js/inline/JsInliner.java | 97 ++++++++++++++++--- .../exception/InlineRecursionException.java | 23 ----- .../kotlin/js/facade/K2JSTranslator.java | 4 +- .../reference/CallExpressionTranslator.java | 21 +--- .../kotlin/js/translate/utils/PsiUtils.java | 20 ++++ .../kotlin/js/translate/utils/inlineUtils.kt | 5 +- 10 files changed, 121 insertions(+), 59 deletions(-) delete mode 100644 js/js.inliner/src/org/jetbrains/kotlin/js/inline/exception/InlineRecursionException.java diff --git a/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/metadata/metadataProperties.kt b/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/metadata/metadataProperties.kt index 2aba228f01c..888fcc8cb49 100644 --- a/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/metadata/metadataProperties.kt +++ b/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/metadata/metadataProperties.kt @@ -18,15 +18,18 @@ package com.google.dart.compiler.backend.js.ast.metadata import com.google.dart.compiler.backend.js.ast.* import org.jetbrains.kotlin.builtins.InlineStrategy -import kotlin.properties.Delegates import org.jetbrains.kotlin.descriptors.CallableDescriptor +import com.intellij.psi.PsiElement + public var JsName.staticRef: JsNode? by MetadataProperty(default = null) public var JsInvocation.inlineStrategy: InlineStrategy? by MetadataProperty(default = null) public var JsInvocation.descriptor: CallableDescriptor? by MetadataProperty(default = null) +public var JsInvocation.psiElement: PsiElement? by MetadataProperty(default = null) + public var JsFunction.isLocal: Boolean by MetadataProperty(default = false) public var JsParameter.hasDefaultValue: Boolean by MetadataProperty(default = false) diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt index 35c5f002a58..434a1252a6c 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt @@ -38,7 +38,7 @@ private val DIAGNOSTIC_FACTORY_TO_RENDERER by Delegates.lazy { put(ErrorsJs.REFERENCE_TO_BUILTIN_MEMBERS_NOT_SUPPORTED, "Callable references for builtin members are not supported yet: ''{0}''", RenderFirstLineOfElementText) put(ErrorsJs.NON_TOPLEVEL_CLASS_DECLARATION, "Non-toplevel {0} declarations not supported yet", Renderers.STRING) put(ErrorsJs.SECONDARY_CONSTRUCTOR, "Secondary constructors not supported yet") - + put(ErrorsJs.INLINE_CALL_CYCLE, "The call is a part of inline cycle") this } diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java index bb606c4436d..ca1f59c70b8 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.js.resolve.diagnostics; +import com.intellij.psi.PsiElement; import org.jetbrains.kotlin.diagnostics.DiagnosticFactory0; import org.jetbrains.kotlin.diagnostics.DiagnosticFactory1; import org.jetbrains.kotlin.diagnostics.DiagnosticFactory2; @@ -44,6 +45,7 @@ public interface ErrorsJs { DiagnosticFactory1 REFERENCE_TO_BUILTIN_MEMBERS_NOT_SUPPORTED = DiagnosticFactory1.create(ERROR, DEFAULT); DiagnosticFactory1 NON_TOPLEVEL_CLASS_DECLARATION = DiagnosticFactory1.create(ERROR, DECLARATION_NAME); DiagnosticFactory0 SECONDARY_CONSTRUCTOR = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT); + DiagnosticFactory0 INLINE_CALL_CYCLE = DiagnosticFactory0.create(ERROR, DEFAULT); @SuppressWarnings("UnusedDeclaration") Object _initializer = new Object() { diff --git a/js/js.inliner/js.inliner.iml b/js/js.inliner/js.inliner.iml index 190520eb877..6872754b48b 100644 --- a/js/js.inliner/js.inliner.iml +++ b/js/js.inliner/js.inliner.iml @@ -13,5 +13,6 @@ + \ No newline at end of file diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java index 0d7717b47a7..3a33f104b82 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java @@ -18,16 +18,16 @@ package org.jetbrains.kotlin.js.inline; import com.google.dart.compiler.backend.js.ast.*; import com.google.dart.compiler.backend.js.ast.metadata.MetadataPackage; +import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.builtins.InlineStrategy; +import org.jetbrains.kotlin.diagnostics.DiagnosticSink; import org.jetbrains.kotlin.js.inline.context.*; -import org.jetbrains.kotlin.js.inline.exception.InlineRecursionException; +import org.jetbrains.kotlin.js.resolve.diagnostics.ErrorsJs; import org.jetbrains.kotlin.js.translate.context.TranslationContext; -import java.util.IdentityHashMap; -import java.util.Set; -import java.util.Stack; +import java.util.*; import static org.jetbrains.kotlin.js.inline.FunctionInlineMutator.getInlineableCallReplacement; import static org.jetbrains.kotlin.js.inline.clean.CleanPackage.removeUnusedFunctionDefinitions; @@ -42,6 +42,11 @@ public class JsInliner extends JsVisitorWithContextImpl { private final Set processedFunctions = IdentitySet(); private final Set inProcessFunctions = IdentitySet(); private final FunctionReader functionReader; + private final DiagnosticSink trace; + + // these are needed for error reporting, when inliner detects cycle + private final Stack namedFunctionsStack = new Stack(); + private final LinkedList inlineCallInfos = new LinkedList(); /** * A statement can contain more, than one inlineable sub-expressions. @@ -76,7 +81,7 @@ public class JsInliner extends JsVisitorWithContextImpl { public static JsProgram process(@NotNull TranslationContext context) { JsProgram program = context.program(); IdentityHashMap functions = collectNamedFunctions(program); - JsInliner inliner = new JsInliner(functions, new FunctionReader(context)); + JsInliner inliner = new JsInliner(functions, new FunctionReader(context), context.bindingTrace()); inliner.accept(program); removeUnusedFunctionDefinitions(program, functions); return program; @@ -84,19 +89,24 @@ public class JsInliner extends JsVisitorWithContextImpl { private JsInliner( @NotNull IdentityHashMap functions, - @NotNull FunctionReader functionReader + @NotNull FunctionReader functionReader, + @NotNull DiagnosticSink trace ) { this.functions = functions; this.functionReader = functionReader; + this.trace = trace; } @Override public boolean visit(JsFunction function, JsContext context) { inliningContexts.push(new JsInliningContext(function)); - - if (inProcessFunctions.contains(function)) throw new InlineRecursionException(); + assert !inProcessFunctions.contains(function): "Inliner has revisited function"; inProcessFunctions.add(function); + if (functions.containsValue(function)) { + namedFunctionsStack.push(function); + } + return super.visit(function, context); } @@ -112,16 +122,27 @@ public class JsInliner extends JsVisitorWithContextImpl { inProcessFunctions.remove(function); inliningContexts.pop(); + + if (!namedFunctionsStack.empty() && namedFunctionsStack.peek() == function) { + namedFunctionsStack.pop(); + } } @Override public boolean visit(JsInvocation call, JsContext context) { - if (call == null) { - return false; - } - if (shouldInline(call) && canInline(call)) { + JsFunction containingFunction = getCurrentNamedFunction(); + if (containingFunction != null) { + inlineCallInfos.add(new JsCallInfo(call, containingFunction)); + } + JsFunction definition = getFunctionContext().getFunctionDefinition(call); + + if (inProcessFunctions.contains(definition)) { + reportInlineCycle(call, definition); + return false; + } + if (!processedFunctions.contains(definition)) { accept(definition); } @@ -132,6 +153,19 @@ public class JsInliner extends JsVisitorWithContextImpl { return !lastStatementWasShifted; } + @Override + public void endVisit(JsInvocation x, JsContext ctx) { + JsCallInfo lastCallInfo = null; + + if (!inlineCallInfos.isEmpty()) { + lastCallInfo = inlineCallInfos.getLast(); + } + + if (lastCallInfo != null && lastCallInfo.call == x) { + inlineCallInfos.removeLast(); + } + } + private void inline(@NotNull JsInvocation call, @NotNull JsContext context) { JsInliningContext inliningContext = getInliningContext(); FunctionContext functionContext = getFunctionContext(); @@ -141,6 +175,7 @@ public class JsInliner extends JsVisitorWithContextImpl { JsStatement inlineableBody = inlineableResult.getInlineableBody(); JsExpression resultExpression = inlineableResult.getResultExpression(); StatementContext statementContext = inliningContext.getStatementContext(); + accept(inlineableBody); /** * Assumes, that resultExpression == null, when result is not needed. @@ -183,6 +218,30 @@ public class JsInliner extends JsVisitorWithContextImpl { return getInliningContext().getFunctionContext(); } + @Nullable + private JsFunction getCurrentNamedFunction() { + if (namedFunctionsStack.empty()) return null; + return namedFunctionsStack.peek(); + } + + private void reportInlineCycle(@NotNull JsInvocation call, @NotNull JsFunction calledFunction) { + MetadataPackage.setInlineStrategy(call, InlineStrategy.NOT_INLINE); + Iterator it = inlineCallInfos.descendingIterator(); + + while (it.hasNext()) { + JsCallInfo callInfo = it.next(); + PsiElement psiElement = MetadataPackage.getPsiElement(callInfo.call); + + if (psiElement != null) { + trace.report(ErrorsJs.INLINE_CALL_CYCLE.on(psiElement)); + } + + if (callInfo.containingFunction == calledFunction) { + break; + } + } + } + private boolean canInline(@NotNull JsInvocation call) { FunctionContext functionContext = getFunctionContext(); return functionContext.hasFunctionDefinition(call); @@ -193,7 +252,6 @@ public class JsInliner extends JsVisitorWithContextImpl { return strategy != null && strategy.isInline(); } - private class JsInliningContext implements InliningContext { private final FunctionContext functionContext; @@ -245,4 +303,17 @@ public class JsInliner extends JsVisitorWithContextImpl { return functionContext; } } + + private static class JsCallInfo { + @NotNull + public final JsInvocation call; + + @NotNull + public final JsFunction containingFunction; + + private JsCallInfo(@NotNull JsInvocation call, @NotNull JsFunction function) { + this.call = call; + containingFunction = function; + } + } } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/exception/InlineRecursionException.java b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/exception/InlineRecursionException.java deleted file mode 100644 index 127c6fa52ce..00000000000 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/exception/InlineRecursionException.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2010-2015 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.kotlin.js.inline.exception; - -public class InlineRecursionException extends RuntimeException { - public InlineRecursionException() { - super("Encountered mutual inline recursion, inline will lead to stack overflow"); - } -} diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.java b/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.java index 31070a8d462..3ad81c0e764 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.java @@ -71,12 +71,14 @@ public final class K2JSTranslator { BindingTrace bindingTrace = analysisResult.getBindingTrace(); TopDownAnalyzerFacadeForJS.checkForErrors(Config.withJsLibAdded(files, config), bindingTrace.getBindingContext()); ModuleDescriptor moduleDescriptor = analysisResult.getModuleDescriptor(); - TranslationContext context = Translation.generateAst(bindingTrace, files, mainCallParameters, moduleDescriptor, config); Diagnostics diagnostics = bindingTrace.getBindingContext().getDiagnostics(); + TranslationContext context = Translation.generateAst(bindingTrace, files, mainCallParameters, moduleDescriptor, config); if (hasError(diagnostics)) return new TranslationResult.Fail(diagnostics); JsProgram program = JsInliner.process(context); + if (hasError(diagnostics)) return new TranslationResult.Fail(diagnostics); + return new TranslationResult.Success(config, files, program, diagnostics); } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/reference/CallExpressionTranslator.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/reference/CallExpressionTranslator.java index 029c450c74b..bb0db6ee201 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/reference/CallExpressionTranslator.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/reference/CallExpressionTranslator.java @@ -33,9 +33,7 @@ import org.jetbrains.kotlin.psi.JetStringTemplateExpression; import org.jetbrains.kotlin.psi.ValueArgument; import org.jetbrains.kotlin.resolve.BindingTrace; import org.jetbrains.kotlin.resolve.TemporaryBindingTrace; -import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilPackage; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; -import org.jetbrains.kotlin.resolve.calls.model.VariableAsFunctionResolvedCall; import org.jetbrains.kotlin.resolve.constants.CompileTimeConstant; import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator; import org.jetbrains.kotlin.types.JetType; @@ -45,6 +43,7 @@ import java.io.StringReader; import java.util.ArrayList; import java.util.List; +import static org.jetbrains.kotlin.js.translate.utils.PsiUtils.getFunctionDescriptor; import static org.jetbrains.kotlin.js.translate.utils.UtilsPackage.setInlineCallMetadata; import static org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilPackage.getFunctionResolvedCallWithAssert; import static org.jetbrains.kotlin.js.resolve.diagnostics.JsCallChecker.isJsCall; @@ -64,10 +63,9 @@ public final class CallExpressionTranslator extends AbstractCallExpressionTransl } JsExpression callExpression = (new CallExpressionTranslator(expression, receiver, context)).translate(); - CallableDescriptor descriptor = getFunctionDescriptor(expression, context); if (!resolvedCall.isSafeCall() && shouldBeInlined(expression, context)) { - setInlineCallMetadata(callExpression, descriptor, context); + setInlineCallMetadata(callExpression, expression, context); } return callExpression; @@ -94,21 +92,6 @@ public final class CallExpressionTranslator extends AbstractCallExpressionTransl return false; } - @NotNull - private static CallableDescriptor getFunctionDescriptor( - @NotNull JetCallExpression expression, - @NotNull TranslationContext context - ) { - ResolvedCall resolvedCall = CallUtilPackage.getResolvedCall(expression, context.bindingContext()); - assert resolvedCall != null; - - if (resolvedCall instanceof VariableAsFunctionResolvedCall) { - return ((VariableAsFunctionResolvedCall) resolvedCall).getVariableCall().getCandidateDescriptor(); - } - - return resolvedCall.getCandidateDescriptor(); - } - private CallExpressionTranslator( @NotNull JetCallExpression expression, @Nullable JsExpression receiver, diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/PsiUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/PsiUtils.java index 786522d2802..66fa20dc16e 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/PsiUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/PsiUtils.java @@ -19,9 +19,14 @@ package org.jetbrains.kotlin.js.translate.utils; import com.intellij.psi.tree.IElementType; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.kotlin.descriptors.CallableDescriptor; +import org.jetbrains.kotlin.js.translate.context.TranslationContext; import org.jetbrains.kotlin.lexer.JetToken; import org.jetbrains.kotlin.lexer.JetTokens; import org.jetbrains.kotlin.psi.*; +import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilPackage; +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; +import org.jetbrains.kotlin.resolve.calls.model.VariableAsFunctionResolvedCall; import java.util.Collections; import java.util.List; @@ -132,4 +137,19 @@ public final class PsiUtils { assert rangeExpression != null; return rangeExpression; } + + @NotNull + public static CallableDescriptor getFunctionDescriptor( + @NotNull JetCallExpression expression, + @NotNull TranslationContext context + ) { + ResolvedCall resolvedCall = CallUtilPackage.getResolvedCall(expression, context.bindingContext()); + assert resolvedCall != null; + + if (resolvedCall instanceof VariableAsFunctionResolvedCall) { + return ((VariableAsFunctionResolvedCall) resolvedCall).getVariableCall().getCandidateDescriptor(); + } + + return resolvedCall.getCandidateDescriptor(); + } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/inlineUtils.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/inlineUtils.kt index 606502bbbe1..961ec9a9b46 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/inlineUtils.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/inlineUtils.kt @@ -24,6 +24,7 @@ import org.jetbrains.kotlin.js.inline.util.isCallInvocation import org.jetbrains.kotlin.js.translate.context.TranslationContext import org.jetbrains.kotlin.js.translate.reference.CallExpressionTranslator import org.jetbrains.kotlin.descriptors.DeclarationDescriptor +import org.jetbrains.kotlin.psi.JetCallExpression /** * Recursively walks expression and sets metadata for all invocations of descriptor. @@ -38,9 +39,10 @@ import org.jetbrains.kotlin.descriptors.DeclarationDescriptor */ fun setInlineCallMetadata( expression: JsExpression, - descriptor: CallableDescriptor, + jetCall: JetCallExpression, context: TranslationContext ) { + val descriptor = PsiUtils.getFunctionDescriptor(jetCall, context) assert(CallExpressionTranslator.shouldBeInlined(descriptor)) { "Expected descriptor of callable, that should be inlined, but got: $descriptor" } @@ -56,6 +58,7 @@ fun setInlineCallMetadata( if (name == invocation.name) { invocation.descriptor = descriptor invocation.inlineStrategy = InlineStrategy.IN_PLACE + invocation.psiElement = jetCall } } }