[groovy] Don't infer invocation kind for closure (see javadoc for FunctionalExpressionFlowUtil.kt)

GitOrigin-RevId: ca38d7cc1fc9ef84badf63e2367c34ba3352be9d
This commit is contained in:
Konstantin Nisht
2022-01-20 23:15:10 +03:00
committed by intellij-monorepo-bot
parent 7ceb24eed4
commit 2110088009
3 changed files with 31 additions and 18 deletions

View File

@@ -21,6 +21,33 @@ import org.jetbrains.plugins.groovy.lang.psi.util.skipParenthesesDown
import org.jetbrains.plugins.groovy.lang.resolve.api.ExpressionArgument
import org.jetbrains.plugins.groovy.lang.resolve.impl.getArguments
/**
* Invocation kinds for closures are __temporarily__ disabled.
*
* At first, I tried to solve the problem of handling side effects of functional expressions, such as
* ```
* if (a instanceof A) {
* with(1) {
* // here a has type A
* }
* }
* ```
* Not every closure should be inlined in that way, so it required kotlin-like distinction of types for closure invocation.
* That is why this class was created. Unfortunately, it has some drawbacks:
* it required enormous amount of computation power to determine the kind of closure, like resolve of the method, invoking the closure,
* and some auxiliary precomputations before actually running type inference. Also, increased number of dependencies in control flow graph
* implied more time spent in thread-local resolves.
* It must be noted that most of the closures are actually [UNKNOWN] or [IN_PLACE_UNKNOWN], because amount of [IN_PLACE_ONCE] closures is very low.
* Users do not have the ability to specify the kind of closure execution, and, considering groovy popularity, it's unlikely to appear.
* Therefore, it makes little sense in [IN_PLACE_ONCE].
*
* [UNKNOWN], on the other hand, is likely to be the most popular invocation kind. But correct handling of it is too complicated: we need to
* track all side effects, happening in the unknown closure, and considering them at __every__ usage of side-effect-affected object.
*
* So at last I decided to remove the effects of [UNKNOWN] and [IN_PLACE_ONCE] in favor of [IN_PLACE_UNKNOWN], because its handling is relatively easy.
* I do not completely lose my hope to distinguish different closures and reach the primary goal specified above, but the
* low number of code parts that can benefit from these changes (in their current implementation) do not worth performance degradation.
*/
enum class InvocationKind {
/**
* Indicates that functional expressions will be invoked inplace and only one time.

View File

@@ -8,7 +8,6 @@ import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
import it.unimi.dsi.fastutil.objects.Object2IntMap;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.plugins.groovy.lang.psi.api.GrFunctionalExpression;
import org.jetbrains.plugins.groovy.lang.psi.api.GroovyMethodResult;
import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable;
@@ -67,7 +66,7 @@ class TypeDfaInstance implements DfaInstance<TypeDfaState> {
newState = handleStartFunctionalExpression(state, instruction);
}
else if (instruction instanceof FunctionalBlockEndInstruction) {
newState = handleFunctionalExpression(state, (FunctionalBlockEndInstruction)instruction);
newState = handleFunctionalExpression(state);
}
else {
newState = state;
@@ -103,13 +102,12 @@ class TypeDfaInstance implements DfaInstance<TypeDfaState> {
return true;
}
private TypeDfaState handleFunctionalExpression(@NotNull TypeDfaState state, @NotNull FunctionalBlockEndInstruction instruction) {
GrFunctionalExpression block = instruction.getStartNode().getElement();
private TypeDfaState handleFunctionalExpression(@NotNull TypeDfaState state) {
ClosureFrame currentClosureFrame = state.getTopClosureFrame();
if (currentClosureFrame.getStartInstructionState() == state || hasNoChanges(currentClosureFrame.getStartInstructionState(), state.getRawVarTypes())) {
return currentClosureFrame.getStartInstructionState().withRemovedBindings(state.getRemovedBindings());
}
InvocationKind kind = getInvocationKind(block, state, instruction);
InvocationKind kind = InvocationKind.IN_PLACE_UNKNOWN; // FunctionalExpressionFlowUtil.computeInvocationKind(block, state, instruction);
TypeDfaState newState = state.withoutTopClosureState();
switch (kind) {
case IN_PLACE_ONCE:
@@ -134,18 +132,6 @@ class TypeDfaInstance implements DfaInstance<TypeDfaState> {
return newState;
}
@NotNull
private InvocationKind getInvocationKind(GrFunctionalExpression block,
@NotNull TypeDfaState state,
@NotNull FunctionalBlockEndInstruction instruction) {
if (myFlowInfo.getAcyclicInstructions().contains(instruction)) {
return FunctionalExpressionFlowUtil.getInvocationKind(block, true);
}
else {
return runWithoutCaching(state, () -> FunctionalExpressionFlowUtil.getInvocationKind(block, false));
}
}
private TypeDfaState handleMixin(@NotNull final TypeDfaState state, @NotNull final MixinTypeInstruction instruction) {
final VariableDescriptor descriptor = instruction.getVariableDescriptor();
if (descriptor == null) return state;

View File

@@ -1404,7 +1404,7 @@ def foo(def p) {
}
<caret>x
}
''', JAVA_LANG_INTEGER
''', "[java.io.Serializable,java.lang.Comparable<? extends java.io.Serializable>]"
}
void 'test assignment inside closure 2'() {