You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/09/09 17:19:48 UTC
[groovy] branch GROOVY_3_0_X updated: GROOVY-9790: tweak lambda
with the parameter of no explicit type
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new 4611b20 GROOVY-9790: tweak lambda with the parameter of no explicit type
4611b20 is described below
commit 4611b20e05b10e5d8e3449fa721786448bc0f8ab
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Apr 24 16:19:52 2021 +0800
GROOVY-9790: tweak lambda with the parameter of no explicit type
Conflicts:
src/main/java/org/codehaus/groovy/classgen/asm/sc/AbstractFunctionalInterfaceWriter.java
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesLambdaWriter.java
subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/runtime/AsciiTableMaker.groovy
---
.../asm/sc/AbstractFunctionalInterfaceWriter.java | 11 +-
.../classgen/asm/sc/StaticTypesLambdaWriter.java | 30 +++--
...StaticTypesMethodReferenceExpressionWriter.java | 130 +++++++++------------
src/test/groovy/bugs/Groovy9790.groovy | 14 +++
4 files changed, 91 insertions(+), 94 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/AbstractFunctionalInterfaceWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/AbstractFunctionalInterfaceWriter.java
index 57c681c..654b54a 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/AbstractFunctionalInterfaceWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/AbstractFunctionalInterfaceWriter.java
@@ -107,8 +107,8 @@ public interface AbstractFunctionalInterfaceWriter {
return argumentList.toArray();
}
- default ClassNode convertParameterType(ClassNode parameterType, ClassNode inferredType) {
- if (!getWrapper(inferredType.redirect()).isDerivedFrom(getWrapper(parameterType.redirect()))) {
+ default ClassNode convertParameterType(ClassNode targetType, ClassNode parameterType, ClassNode inferredType) {
+ if (!getWrapper(inferredType).isDerivedFrom(getWrapper(parameterType))) {
throw new RuntimeParserException("The inferred type[" + inferredType.redirect() + "] is not compatible with the parameter type[" + parameterType.redirect() + "]", parameterType);
}
@@ -116,9 +116,12 @@ public interface AbstractFunctionalInterfaceWriter {
boolean isParameterTypePrimitive = ClassHelper.isPrimitiveType(parameterType);
boolean isInferredTypePrimitive = ClassHelper.isPrimitiveType(inferredType);
if (!isParameterTypePrimitive && isInferredTypePrimitive) {
- if (parameterType != getUnwrapper(parameterType) && inferredType != getWrapper(inferredType)) {
+ if (ClassHelper.DYNAMIC_TYPE.equals(parameterType) && ClassHelper.isPrimitiveType(targetType) // (1)
+ || !parameterType.equals(getUnwrapper(parameterType)) && !inferredType.equals(getWrapper(inferredType)) // (2)
+ ) {
// GROOVY-9790: bootstrap method initialization exception raised when lambda parameter type is wrong
- // java.lang.BootstrapMethodError: bootstrap method initialization exception
+ // (1) java.lang.invoke.LambdaConversionException: Type mismatch for instantiated parameter 0: class java.lang.Integer is not a subtype of int
+ // (2) java.lang.BootstrapMethodError: bootstrap method initialization exception
type = inferredType;
} else {
// The non-primitive type and primitive type are not allowed to mix since Java 9+
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesLambdaWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesLambdaWriter.java
index fd178f7..be6e8f5 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesLambdaWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesLambdaWriter.java
@@ -33,6 +33,8 @@ import org.codehaus.groovy.ast.expr.LambdaExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.stmt.BlockStatement;
import org.codehaus.groovy.ast.stmt.Statement;
+import org.codehaus.groovy.ast.tools.ClosureUtils;
+import org.codehaus.groovy.ast.tools.GeneralUtils;
import org.codehaus.groovy.classgen.BytecodeInstruction;
import org.codehaus.groovy.classgen.BytecodeSequence;
import org.codehaus.groovy.classgen.asm.BytecodeHelper;
@@ -71,7 +73,6 @@ import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_STATIC;
-import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
import static org.objectweb.asm.Opcodes.ALOAD;
import static org.objectweb.asm.Opcodes.CHECKCAST;
import static org.objectweb.asm.Opcodes.DUP;
@@ -277,7 +278,7 @@ public class StaticTypesLambdaWriter extends LambdaWriter implements AbstractFun
}
private MethodNode addSyntheticLambdaMethodNode(final LambdaExpression expression, final ClassNode lambdaClass, final MethodNode abstractMethod) {
- Parameter[] parametersWithExactType = createParametersWithExactType(expression);
+ Parameter[] parametersWithExactType = createParametersWithExactType(expression, abstractMethod);
Parameter[] localVariableParameters = getLambdaSharedVariables(expression);
removeInitialValues(localVariableParameters);
@@ -296,24 +297,19 @@ public class StaticTypesLambdaWriter extends LambdaWriter implements AbstractFun
return doCallMethod;
}
- private Parameter[] createParametersWithExactType(final LambdaExpression expression) {
- Parameter[] parameters = expression.getParameters();
- if (parameters == null) {
- parameters = Parameter.EMPTY_ARRAY;
- }
-
- for (Parameter parameter : parameters) {
+ private Parameter[] createParametersWithExactType(final LambdaExpression expression, MethodNode abstractMethod) {
+ Parameter[] targetParameters = GeneralUtils.cloneParams(abstractMethod.getParameters());
+ Parameter[] parameters = ClosureUtils.getParametersSafe(expression);
+ for (int i = 0; i < parameters.length; i++) {
+ Parameter targetParameter = targetParameters[i];
+ Parameter parameter = parameters[i];
ClassNode inferredType = parameter.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
- if (inferredType == null) {
- continue;
+ if (inferredType != null) {
+ ClassNode type = convertParameterType(targetParameter.getType(), parameter.getType(), inferredType);
+ parameter.setOriginType(type);
+ parameter.setType(type);
}
-
- ClassNode type = convertParameterType(parameter.getType(), inferredType);
-
- parameter.setType(type);
- parameter.setOriginType(type);
}
-
return parameters;
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
index 14af6ba..a56631c 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
@@ -41,8 +41,8 @@ import org.codehaus.groovy.transform.stc.ExtensionMethodNode;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
import org.objectweb.asm.Opcodes;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.LinkedList;
import java.util.List;
import java.util.stream.Collectors;
@@ -66,50 +66,40 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isAssi
*/
public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceExpressionWriter implements AbstractFunctionalInterfaceWriter {
- public StaticTypesMethodReferenceExpressionWriter(WriterController controller) {
+ public StaticTypesMethodReferenceExpressionWriter(final WriterController controller) {
super(controller);
}
@Override
- public void writeMethodReferenceExpression(MethodReferenceExpression methodReferenceExpression) {
+ public void writeMethodReferenceExpression(final MethodReferenceExpression methodReferenceExpression) {
ClassNode functionalInterfaceType = getFunctionalInterfaceType(methodReferenceExpression);
- if (null == functionalInterfaceType) {
- // if the parameter type failed to be inferred, generate the default bytecode, which is actually a method closure
+ if (!ClassHelper.isFunctionalInterface(functionalInterfaceType)) {
+ // generate the default bytecode; most likely a method closure
super.writeMethodReferenceExpression(methodReferenceExpression);
return;
}
ClassNode redirect = functionalInterfaceType.redirect();
- if (!ClassHelper.isFunctionalInterface(redirect)) {
- // if the parameter type is not real FunctionalInterface, generate the default bytecode, which is actually a method closure
- super.writeMethodReferenceExpression(methodReferenceExpression);
- return;
- }
-
- MethodNode abstractMethodNode = ClassHelper.findSAM(redirect);
-
- String abstractMethodDesc = createMethodDescriptor(abstractMethodNode);
+ MethodNode abstractMethod = ClassHelper.findSAM(redirect);
+ String abstractMethodDesc = createMethodDescriptor(abstractMethod);
ClassNode classNode = controller.getClassNode();
- boolean isInterface = classNode.isInterface();
-
Expression typeOrTargetRef = methodReferenceExpression.getExpression();
boolean isClassExpression = (typeOrTargetRef instanceof ClassExpression);
ClassNode typeOrTargetRefType = isClassExpression ? typeOrTargetRef.getType()
: controller.getTypeChooser().resolveType(typeOrTargetRef, classNode);
ClassNode[] methodReferenceParamTypes = methodReferenceExpression.getNodeMetaData(StaticTypesMarker.CLOSURE_ARGUMENTS);
- Parameter[] parametersWithExactType = createParametersWithExactType(abstractMethodNode, methodReferenceParamTypes);
-
+ Parameter[] parametersWithExactType = createParametersWithExactType(abstractMethod, methodReferenceParamTypes);
String methodRefName = methodReferenceExpression.getMethodName().getText();
boolean isConstructorReference = isConstructorReference(methodRefName);
MethodNode methodRefMethod;
if (isConstructorReference) {
- methodRefName = genSyntheticMethodNameForConstructorReference();
+ methodRefName = controller.getContext().getNextConstructorReferenceSyntheticMethodName(controller.getMethodNode());
methodRefMethod = addSyntheticMethodForConstructorReference(methodRefName, typeOrTargetRefType, parametersWithExactType);
} else {
- // TODO move the `findMethodRefMethod` and checking to `StaticTypeCheckingVisitor`
+ // TODO: move the findMethodRefMethod and checking to StaticTypeCheckingVisitor
methodRefMethod = findMethodRefMethod(methodRefName, parametersWithExactType, typeOrTargetRef, typeOrTargetRefType);
}
@@ -123,17 +113,16 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
}
typeOrTargetRefType = methodRefMethod.getDeclaringClass();
- ClassExpression classExpression = classX(typeOrTargetRefType);
+ Expression classExpression = classX(typeOrTargetRefType);
classExpression.setSourcePosition(typeOrTargetRef);
typeOrTargetRef = classExpression;
- isClassExpression = true;
}
methodRefMethod.putNodeMetaData(ORIGINAL_PARAMETERS_WITH_EXACT_TYPE, parametersWithExactType);
if (!isClassExpression) {
if (isConstructorReference) {
- // TODO: move the checking code to the Parrot parser
+ // TODO: move the checking code to the parser
addFatalError("Constructor reference must be className::new", methodReferenceExpression);
} else if (methodRefMethod.isStatic()) {
ClassExpression classExpression = classX(typeOrTargetRefType);
@@ -146,14 +135,16 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
}
controller.getMethodVisitor().visitInvokeDynamicInsn(
- abstractMethodNode.getName(),
+ abstractMethod.getName(),
createAbstractMethodDesc(functionalInterfaceType, typeOrTargetRef),
- createBootstrapMethod(isInterface, false),
+ createBootstrapMethod(classNode.isInterface(), false),
createBootstrapMethodArguments(
abstractMethodDesc,
methodRefMethod.isStatic() || isConstructorReference ? Opcodes.H_INVOKESTATIC : Opcodes.H_INVOKEVIRTUAL,
isConstructorReference ? controller.getClassNode() : typeOrTargetRefType,
- methodRefMethod, false)
+ methodRefMethod,
+ false
+ )
);
if (isClassExpression) {
@@ -163,8 +154,8 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
}
}
- private void validate(MethodReferenceExpression methodReferenceExpression, Expression typeOrTargetRef, ClassNode typeOrTargetRefType, String methodRefName, Parameter[] parametersWithExactType, MethodNode methodRefMethod) {
- if (null == methodRefMethod) {
+ private void validate(final MethodReferenceExpression methodReferenceExpression, final Expression typeOrTargetRef, final ClassNode typeOrTargetRefType, final String methodRefName, final Parameter[] parametersWithExactType, final MethodNode methodRefMethod) {
+ if (methodRefMethod == null) {
addFatalError("Failed to find the expected method["
+ methodRefName + "("
+ Arrays.stream(parametersWithExactType)
@@ -179,11 +170,7 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
}
}
- private static boolean isExtensionMethod(MethodNode methodRefMethod) {
- return methodRefMethod instanceof ExtensionMethodNode;
- }
-
- private MethodNode addSyntheticMethodForDGSM(MethodNode mn) {
+ private MethodNode addSyntheticMethodForDGSM(final MethodNode mn) {
Parameter[] parameters = removeFirstParameter(mn.getParameters());
ArgumentListExpression args = args(parameters);
args.getExpressions().add(0, nullX());
@@ -206,7 +193,7 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
return delegateMethod;
}
- private MethodNode addSyntheticMethodForConstructorReference(String syntheticMethodName, ClassNode returnType, Parameter[] parametersWithExactType) {
+ private MethodNode addSyntheticMethodForConstructorReference(final String syntheticMethodName, final ClassNode returnType, final Parameter[] parametersWithExactType) {
ArgumentListExpression ctorArgs = args(parametersWithExactType);
Expression returnValue;
@@ -235,20 +222,8 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
return delegateMethod;
}
- private String genSyntheticMethodNameForConstructorReference() {
- return controller.getContext().getNextConstructorReferenceSyntheticMethodName(controller.getMethodNode());
- }
-
- private boolean isConstructorReference(String methodRefName) {
- return "new".equals(methodRefName);
- }
-
- private static boolean isClassExpr(Expression methodRef) {
- return methodRef instanceof ClassExpression;
- }
-
- private String createAbstractMethodDesc(ClassNode functionalInterfaceType, Expression methodRef) {
- List<Parameter> methodReferenceSharedVariableList = new LinkedList<>();
+ private String createAbstractMethodDesc(final ClassNode functionalInterfaceType, final Expression methodRef) {
+ List<Parameter> methodReferenceSharedVariableList = new ArrayList<>();
if (!(methodRef instanceof ClassExpression)) {
prependParameter(methodReferenceSharedVariableList, "__METHODREF_EXPR_INSTANCE",
@@ -258,23 +233,24 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
return BytecodeHelper.getMethodDescriptor(functionalInterfaceType.redirect(), methodReferenceSharedVariableList.toArray(Parameter.EMPTY_ARRAY));
}
- private Parameter[] createParametersWithExactType(MethodNode abstractMethodNode, ClassNode[] inferredParameterTypes) {
- // We MUST clone the parameters to avoid impacting the original parameter type of SAM
- Parameter[] parameters = GeneralUtils.cloneParams(abstractMethodNode.getParameters());
- if (parameters == null) {
- parameters = Parameter.EMPTY_ARRAY;
- }
- if (inferredParameterTypes != null) {
+ private Parameter[] createParametersWithExactType(final MethodNode abstractMethod, final ClassNode[] inferredParamTypes) {
+ // MUST clone the parameters to avoid impacting the original parameter type of SAM
+ Parameter[] parameters = GeneralUtils.cloneParams(abstractMethod.getParameters());
+
+ if (inferredParamTypes != null) {
for (int i = 0, n = parameters.length; i < n; i += 1) {
- ClassNode inferredParamType = inferredParameterTypes[i];
+ ClassNode inferredParamType = inferredParamTypes[i];
if (inferredParamType == null) continue;
+
Parameter parameter = parameters[i];
+ Parameter targetParameter = parameter;
- ClassNode type = convertParameterType(parameter.getType(), inferredParamType);
+ ClassNode type = convertParameterType(targetParameter.getType(), parameter.getType(), inferredParamType);
parameter.setOriginType(type);
parameter.setType(type);
}
}
+
return parameters;
}
@@ -305,19 +281,34 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
return methods;
}
- private static Parameter[] removeFirstParameter(Parameter[] parameters) {
- return Arrays.stream(parameters).skip(1).toArray(Parameter[]::new);
+ private void addFatalError(final String msg, final ASTNode node) {
+ controller.getSourceUnit().addFatalError(msg, node);
}
- private static boolean isTypeReferingInstanceMethod(Expression typeOrTargetRef, MethodNode mn) { // class::instanceMethod
- return (!mn.isStatic() || (isExtensionMethod(mn) && !((ExtensionMethodNode) mn).isStaticExtension()))
- && isClassExpr(typeOrTargetRef);
+ //--------------------------------------------------------------------------
+
+ private static boolean isConstructorReference(final String methodRefName) {
+ return "new".equals(methodRefName);
+ }
+
+ private static boolean isExtensionMethod(final MethodNode methodRefMethod) {
+ return (methodRefMethod instanceof ExtensionMethodNode);
+ }
+
+ private static boolean isTypeReferingInstanceMethod(final Expression typeOrTargetRef, final MethodNode mn) {
+ // class::instanceMethod
+ return (typeOrTargetRef instanceof ClassExpression) && ((mn != null && !mn.isStatic())
+ || (isExtensionMethod(mn) && !((ExtensionMethodNode) mn).isStaticExtension()));
+ }
+
+ private static Parameter[] removeFirstParameter(final Parameter[] parameters) {
+ return Arrays.stream(parameters).skip(1).toArray(Parameter[]::new);
}
/**
* Choose the best method node for method reference.
*/
- private MethodNode chooseMethodRefMethodCandidate(Expression methodRef, List<MethodNode> candidates) {
+ private static MethodNode chooseMethodRefMethodCandidate(final Expression methodRef, final List<MethodNode> candidates) {
if (1 == candidates.size()) return candidates.get(0);
return candidates.stream()
@@ -327,22 +318,19 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
.orElse(null);
}
- private static Integer matchingScore(MethodNode mn, Expression typeOrTargetRef) {
+ private static Integer matchingScore(final MethodNode mn, final Expression typeOrTargetRef) {
ClassNode typeOrTargetRefType = typeOrTargetRef.getType();
int score = 9;
for (ClassNode cn = mn.getDeclaringClass(); null != cn && !cn.equals(typeOrTargetRefType); cn = cn.getSuperClass()) {
- score--;
+ score -= 1;
}
if (score < 0) {
score = 0;
}
score *= 10;
- boolean isClassExpr = isClassExpr(typeOrTargetRef);
- boolean isStaticMethod = mn.isStatic();
-
- if (isClassExpr && isStaticMethod || !isClassExpr && !isStaticMethod) {
+ if ((typeOrTargetRef instanceof ClassExpression) == mn.isStatic()) {
score += 9;
}
@@ -352,8 +340,4 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
return score;
}
-
- private void addFatalError(String msg, ASTNode node) {
- controller.getSourceUnit().addFatalError(msg, node);
- }
}
diff --git a/src/test/groovy/bugs/Groovy9790.groovy b/src/test/groovy/bugs/Groovy9790.groovy
index 85b17a4..ca04ddd 100644
--- a/src/test/groovy/bugs/Groovy9790.groovy
+++ b/src/test/groovy/bugs/Groovy9790.groovy
@@ -56,6 +56,20 @@ final class Groovy9790 {
}
@Test
+ void testLambdaWithNoExplicitType() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ void test() {
+ java.util.stream.IntStream.range(0, 2).forEach(
+ i -> { assert i >= 0 && i < 2 }
+ )
+ }
+
+ test()
+ '''
+ }
+
+ @Test
void testLambdaWithIncompatibleType() {
def err = shouldFail '''
@groovy.transform.CompileStatic