You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2019/04/08 12:58:53 UTC
[groovy] 01/20: GROOVY-9008: Prefactoring
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit eebbd6531f7f19f68544ea3cfb30ed070870c04c
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Feb 23 00:54:46 2019 +0800
GROOVY-9008: Prefactoring
---
.../org/codehaus/groovy/ast/GroovyCodeVisitor.java | 5 +++
.../groovy/ast/expr/MethodPointerExpression.java | 5 +--
.../groovy/ast/expr/MethodReferenceExpression.java | 22 +++++++++-
.../groovy/classgen/AsmClassGenerator.java | 17 ++++----
.../groovy/classgen/asm/DelegatingController.java | 10 +++++
.../asm/MethodPointerExpressionWriter.java | 48 ++++++++++++++++++++++
.../asm/MethodReferenceExpressionWriter.java} | 25 ++++-------
.../groovy/classgen/asm/WriterController.java | 13 ++++++
...taticTypesMethodReferenceExpressionWriter.java} | 26 +++++-------
.../asm/sc/StaticTypesWriterController.java | 12 ++++++
.../groovy/bugs/Groovy9008.groovy} | 44 +++++++++++---------
11 files changed, 165 insertions(+), 62 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/GroovyCodeVisitor.java b/src/main/java/org/codehaus/groovy/ast/GroovyCodeVisitor.java
index eb00281..22fee36 100644
--- a/src/main/java/org/codehaus/groovy/ast/GroovyCodeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/ast/GroovyCodeVisitor.java
@@ -41,6 +41,7 @@ import org.codehaus.groovy.ast.expr.MapEntryExpression;
import org.codehaus.groovy.ast.expr.MapExpression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.expr.MethodPointerExpression;
+import org.codehaus.groovy.ast.expr.MethodReferenceExpression;
import org.codehaus.groovy.ast.expr.NotExpression;
import org.codehaus.groovy.ast.expr.PostfixExpression;
import org.codehaus.groovy.ast.expr.PrefixExpression;
@@ -161,6 +162,10 @@ public interface GroovyCodeVisitor {
void visitMethodPointerExpression(MethodPointerExpression expression);
+ default void visitMethodReferenceExpression(MethodReferenceExpression expression) {
+ visitMethodPointerExpression(expression);
+ }
+
void visitConstantExpression(ConstantExpression expression);
void visitClassExpression(ClassExpression expression);
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/MethodPointerExpression.java b/src/main/java/org/codehaus/groovy/ast/expr/MethodPointerExpression.java
index f4d2422..07422bb 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/MethodPointerExpression.java
+++ b/src/main/java/org/codehaus/groovy/ast/expr/MethodPointerExpression.java
@@ -32,9 +32,8 @@ import org.codehaus.groovy.ast.GroovyCodeVisitor;
* </code>
*/
public class MethodPointerExpression extends Expression {
-
- private final Expression expression;
- private final Expression methodName;
+ protected final Expression expression;
+ protected final Expression methodName;
public MethodPointerExpression(Expression expression, Expression methodName) {
this.expression = expression;
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java b/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
index 8258120..4eafd58 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
+++ b/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
@@ -16,9 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
-
package org.codehaus.groovy.ast.expr;
+import org.codehaus.groovy.ast.GroovyCodeVisitor;
+
/**
* Represents a method reference or a constructor reference,
* e.g. System.out::println OR Objects::requireNonNull OR Integer::new OR int[]::new
@@ -29,6 +30,25 @@ public class MethodReferenceExpression extends MethodPointerExpression {
}
@Override
+ public void visit(GroovyCodeVisitor visitor) {
+ visitor.visitMethodReferenceExpression(this);
+ }
+
+ @Override
+ public Expression transformExpression(ExpressionTransformer transformer) {
+ Expression ret;
+ Expression mname = transformer.transform(methodName);
+ if (expression == null) {
+ ret = new MethodReferenceExpression(VariableExpression.THIS_EXPRESSION, mname);
+ } else {
+ ret = new MethodReferenceExpression(transformer.transform(expression), mname);
+ }
+ ret.setSourcePosition(this);
+ ret.copyNodeMetaData(this);
+ return ret;
+ }
+
+ @Override
public String getText() {
Expression expression = this.getExpression();
Expression methodName = this.getMethodName();
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 203c518..d9bea88 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -61,6 +61,7 @@ import org.codehaus.groovy.ast.expr.MapEntryExpression;
import org.codehaus.groovy.ast.expr.MapExpression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.expr.MethodPointerExpression;
+import org.codehaus.groovy.ast.expr.MethodReferenceExpression;
import org.codehaus.groovy.ast.expr.NotExpression;
import org.codehaus.groovy.ast.expr.PostfixExpression;
import org.codehaus.groovy.ast.expr.PrefixExpression;
@@ -153,8 +154,7 @@ public class AsmClassGenerator extends ClassGenerator {
// spread expressions
static final MethodCaller spreadMap = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "spreadMap");
static final MethodCaller despreadList = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "despreadList");
- // Closure
- static final MethodCaller getMethodPointer = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "getMethodPointer");
+
// type conversions
static final MethodCaller createListMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createList");
@@ -776,13 +776,14 @@ public class AsmClassGenerator extends ClassGenerator {
controller.getOperandStack().replace(ClassHelper.OBJECT_TYPE);
}
+ @Override
public void visitMethodPointerExpression(MethodPointerExpression expression) {
- Expression subExpression = expression.getExpression();
- subExpression.visit(this);
- controller.getOperandStack().box();
- controller.getOperandStack().pushDynamicName(expression.getMethodName());
- getMethodPointer.call(controller.getMethodVisitor());
- controller.getOperandStack().replace(ClassHelper.CLOSURE_TYPE,2);
+ controller.getMethodPointerExpressionWriter().writeMethodPointerExpression(expression);
+ }
+
+ @Override
+ public void visitMethodReferenceExpression(MethodReferenceExpression expression) {
+ controller.getMethodReferenceExpressionWriter().writeMethodReferenceExpression(expression);
}
public void visitUnaryMinusExpression(UnaryMinusExpression expression) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java b/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java
index 0b521e9..aa61413 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java
@@ -117,6 +117,16 @@ public class DelegatingController extends WriterController {
public LambdaWriter getLambdaWriter() {
return delegationController.getLambdaWriter();
}
+
+ @Override
+ public MethodPointerExpressionWriter getMethodPointerExpressionWriter() {
+ return delegationController.getMethodPointerExpressionWriter();
+ }
+
+ @Override
+ public MethodReferenceExpressionWriter getMethodReferenceExpressionWriter() {
+ return delegationController.getMethodReferenceExpressionWriter();
+ }
@Override
public CompileStack getCompileStack() {
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/MethodPointerExpressionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/MethodPointerExpressionWriter.java
new file mode 100644
index 0000000..12c6db5
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/MethodPointerExpressionWriter.java
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.codehaus.groovy.classgen.asm;
+
+import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MethodPointerExpression;
+import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
+
+/**
+ * A helper class used to generate bytecode for method pointer expressions.
+ * @since 3.0.0
+ */
+public class MethodPointerExpressionWriter {
+ // Closure
+ static final MethodCaller getMethodPointer = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "getMethodPointer");
+
+ private final WriterController controller;
+
+ public MethodPointerExpressionWriter(final WriterController controller) {
+ this.controller = controller;
+ }
+
+ public void writeMethodPointerExpression(MethodPointerExpression expression) {
+ Expression subExpression = expression.getExpression();
+ subExpression.visit(controller.getAcg());
+ controller.getOperandStack().box();
+ controller.getOperandStack().pushDynamicName(expression.getMethodName());
+ getMethodPointer.call(controller.getMethodVisitor());
+ controller.getOperandStack().replace(ClassHelper.CLOSURE_TYPE,2);
+ }
+}
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java b/src/main/java/org/codehaus/groovy/classgen/asm/MethodReferenceExpressionWriter.java
similarity index 54%
copy from src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
copy to src/main/java/org/codehaus/groovy/classgen/asm/MethodReferenceExpressionWriter.java
index 8258120..8296e5f 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/MethodReferenceExpressionWriter.java
@@ -16,27 +16,20 @@
* specific language governing permissions and limitations
* under the License.
*/
+package org.codehaus.groovy.classgen.asm;
-package org.codehaus.groovy.ast.expr;
+import org.codehaus.groovy.ast.expr.MethodReferenceExpression;
/**
- * Represents a method reference or a constructor reference,
- * e.g. System.out::println OR Objects::requireNonNull OR Integer::new OR int[]::new
+ * A helper class used to generate bytecode for method reference expressions.
+ * @since 3.0.0
*/
-public class MethodReferenceExpression extends MethodPointerExpression {
- public MethodReferenceExpression(Expression expression, Expression methodName) {
- super(expression, methodName);
+public class MethodReferenceExpressionWriter extends MethodPointerExpressionWriter {
+ public MethodReferenceExpressionWriter(final WriterController controller) {
+ super(controller);
}
- @Override
- public String getText() {
- Expression expression = this.getExpression();
- Expression methodName = this.getMethodName();
-
- if (expression == null) {
- return "::" + methodName;
- } else {
- return expression.getText() + "::" + methodName.getText();
- }
+ public void writeMethodReferenceExpression(MethodReferenceExpression expression) {
+ super.writeMethodPointerExpression(expression);
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
index cb3d266..ef27fff 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
@@ -76,6 +76,8 @@ public class WriterController {
private int lineNumber = -1;
private int helperMethodIndex = 0;
private List<String> superMethodNames = new ArrayList<String>();
+ private MethodPointerExpressionWriter methodPointerExpressionWriter;
+ private MethodReferenceExpressionWriter methodReferenceExpressionWriter;
public void init(AsmClassGenerator asmClassGenerator, GeneratorContext gcon, ClassVisitor cv, ClassNode cn) {
CompilerConfiguration config = cn.getCompileUnit().getConfig();
@@ -122,6 +124,8 @@ public class WriterController {
this.assertionWriter = new AssertionWriter(this);
this.closureWriter = new ClosureWriter(this);
this.lambdaWriter = new LambdaWriter(this);
+ this.methodPointerExpressionWriter = new MethodPointerExpressionWriter(this);
+ this.methodReferenceExpressionWriter = new MethodReferenceExpressionWriter(this);
this.internalBaseClassName = BytecodeHelper.getClassInternalName(classNode.getSuperClass());
this.acg = asmClassGenerator;
this.sourceUnit = acg.getSourceUnit();
@@ -219,6 +223,14 @@ public class WriterController {
}
}
+ public MethodPointerExpressionWriter getMethodPointerExpressionWriter() {
+ return methodPointerExpressionWriter;
+ }
+
+ public MethodReferenceExpressionWriter getMethodReferenceExpressionWriter() {
+ return methodReferenceExpressionWriter;
+ }
+
public UnaryExpressionHelper getUnaryExpressionHelper() {
if (fastPath) {
return fastPathUnaryExpressionHelper;
@@ -401,4 +413,5 @@ public class WriterController {
public List<String> getSuperMethodNames() {
return superMethodNames;
}
+
}
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
similarity index 53%
copy from src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
copy to src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
index 8258120..92b4ab9 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
@@ -16,27 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
+package org.codehaus.groovy.classgen.asm.sc;
-package org.codehaus.groovy.ast.expr;
+import org.codehaus.groovy.ast.expr.MethodReferenceExpression;
+import org.codehaus.groovy.classgen.asm.MethodReferenceExpressionWriter;
+import org.codehaus.groovy.classgen.asm.WriterController;
/**
- * Represents a method reference or a constructor reference,
- * e.g. System.out::println OR Objects::requireNonNull OR Integer::new OR int[]::new
+ * Writer responsible for generating method reference in statically compiled mode.
+ * @since 3.0.0
*/
-public class MethodReferenceExpression extends MethodPointerExpression {
- public MethodReferenceExpression(Expression expression, Expression methodName) {
- super(expression, methodName);
+public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceExpressionWriter {
+ public StaticTypesMethodReferenceExpressionWriter(WriterController controller) {
+ super(controller);
}
@Override
- public String getText() {
- Expression expression = this.getExpression();
- Expression methodName = this.getMethodName();
-
- if (expression == null) {
- return "::" + methodName;
- } else {
- return expression.getText() + "::" + methodName.getText();
- }
+ public void writeMethodReferenceExpression(MethodReferenceExpression expression) {
+ super.writeMethodReferenceExpression(expression); // TODO generate native method reference bytecode here
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java
index ab7b537..edfd27f 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java
@@ -32,6 +32,7 @@ import org.codehaus.groovy.classgen.asm.ClosureWriter;
import org.codehaus.groovy.classgen.asm.DelegatingController;
import org.codehaus.groovy.classgen.asm.InvocationWriter;
import org.codehaus.groovy.classgen.asm.LambdaWriter;
+import org.codehaus.groovy.classgen.asm.MethodReferenceExpressionWriter;
import org.codehaus.groovy.classgen.asm.StatementWriter;
import org.codehaus.groovy.classgen.asm.TypeChooser;
import org.codehaus.groovy.classgen.asm.UnaryExpressionHelper;
@@ -60,6 +61,7 @@ public class StaticTypesWriterController extends DelegatingController {
private UnaryExpressionHelper unaryExpressionHelper;
private ClosureWriter closureWriter;
private LambdaWriter lambdaWriter;
+ private MethodReferenceExpressionWriter methodReferenceExpressionWriter;
public StaticTypesWriterController(WriterController normalController) {
super(normalController);
@@ -75,6 +77,7 @@ public class StaticTypesWriterController extends DelegatingController {
this.invocationWriter = new StaticInvocationWriter(this);
this.closureWriter = new StaticTypesClosureWriter(this);
this.lambdaWriter = new StaticTypesLambdaWriter(this);
+ this.methodReferenceExpressionWriter = new StaticTypesMethodReferenceExpressionWriter(this);
this.unaryExpressionHelper = new StaticTypesUnaryExpressionHelper(this);
CompilerConfiguration config = cn.getCompileUnit().getConfig();
@@ -172,6 +175,15 @@ public class StaticTypesWriterController extends DelegatingController {
}
@Override
+ public MethodReferenceExpressionWriter getMethodReferenceExpressionWriter() {
+ if (isInStaticallyCheckedMethod) {
+ return methodReferenceExpressionWriter;
+ }
+
+ return super.getMethodReferenceExpressionWriter();
+ }
+
+ @Override
public UnaryExpressionHelper getUnaryExpressionHelper() {
if (isInStaticallyCheckedMethod) {
return unaryExpressionHelper;
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java b/src/test/groovy/bugs/Groovy9008.groovy
similarity index 51%
copy from src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
copy to src/test/groovy/bugs/Groovy9008.groovy
index 8258120..fba2a02 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/MethodReferenceExpression.java
+++ b/src/test/groovy/bugs/Groovy9008.groovy
@@ -16,27 +16,33 @@
* specific language governing permissions and limitations
* under the License.
*/
+package groovy.bugs
-package org.codehaus.groovy.ast.expr;
-
-/**
- * Represents a method reference or a constructor reference,
- * e.g. System.out::println OR Objects::requireNonNull OR Integer::new OR int[]::new
- */
-public class MethodReferenceExpression extends MethodPointerExpression {
- public MethodReferenceExpression(Expression expression, Expression methodName) {
- super(expression, methodName);
+class Groovy9008 extends GroovyTestCase {
+ void testMethodReference() {
+ assertScript '''
+ import java.util.stream.Collectors
+
+ void p() {
+ def result = [1, 2, 3].stream().map(Object::toString).collect(Collectors.toList())
+ assert ['1', '2', '3'] == result
+ }
+
+ p()
+ '''
}
- @Override
- public String getText() {
- Expression expression = this.getExpression();
- Expression methodName = this.getMethodName();
-
- if (expression == null) {
- return "::" + methodName;
- } else {
- return expression.getText() + "::" + methodName.getText();
- }
+ void testMethodReferenceSC() {
+ assertScript '''
+ import java.util.stream.Collectors
+
+ @groovy.transform.CompileStatic
+ void p() {
+ def result = [1, 2, 3].stream().map(Object::toString).collect(Collectors.toList())
+ assert ['1', '2', '3'] == result
+ }
+
+ p()
+ '''
}
}