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 2019/11/25 23:26:16 UTC
[groovy] branch master updated (683ff9e -> 3cbc7e3)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from 683ff9e GROOVY-8762: add test case
new ceb9f95 minor edits
new 3cbc7e3 GROOVY-8762: make WriterController#isInClosure() more selective
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../groovy/classgen/AsmClassGenerator.java | 2 +-
.../groovy/classgen/asm/DelegatingController.java | 94 +++++++++++-----------
.../groovy/classgen/asm/WriterController.java | 85 +++++++++----------
src/test/groovy/bugs/Groovy8762.groovy | 22 ++---
4 files changed, 89 insertions(+), 114 deletions(-)
[groovy] 01/02: minor edits
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit ceb9f95d8a7ae6a1da48160aee6e56a99f71df74
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 25 17:21:08 2019 -0600
minor edits
---
.../groovy/classgen/AsmClassGenerator.java | 2 +-
.../groovy/classgen/asm/DelegatingController.java | 94 +++++++++++-----------
.../groovy/classgen/asm/WriterController.java | 78 ++++++++----------
3 files changed, 79 insertions(+), 95 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 3e50687..da3d97c 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -934,7 +934,7 @@ public class AsmClassGenerator extends ClassGenerator {
if (field != null && field.isPrivate()) {
privateSuperField = true;
}
- } else if (controller.isNotExplicitThisInClosure(expression.isImplicitThis())) {
+ } else if (expression.isImplicitThis() || !controller.isInClosure()) {
field = classNode.getDeclaredField(name);
ClassNode outer = classNode.getOuterClass();
if (field == null && outer != null) {
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 aa61413..de5cedf 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java
@@ -29,15 +29,16 @@ import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.MethodVisitor;
/**
- * This class will delegate all calls to a WriterController given in the constructor.
+ * This class will delegate all calls to a WriterController given in the constructor.
*/
public class DelegatingController extends WriterController {
+
private final WriterController delegationController;
-
+
public DelegatingController(WriterController normalController) {
this.delegationController = normalController;
}
-
+
@Override
public void init(final AsmClassGenerator asmClassGenerator, final GeneratorContext gcon, final ClassVisitor cv, final ClassNode cn) {
delegationController.init(asmClassGenerator, gcon, cv, cn);
@@ -52,37 +53,37 @@ public class DelegatingController extends WriterController {
public void setConstructorNode(final ConstructorNode cn) {
delegationController.setConstructorNode(cn);
}
-
+
@Override
public boolean isFastPath() {
return delegationController.isFastPath();
}
-
+
@Override
public CallSiteWriter getCallSiteWriter() {
return delegationController.getCallSiteWriter();
}
-
+
@Override
public StatementWriter getStatementWriter() {
- return delegationController.getStatementWriter();
+ return delegationController.getStatementWriter();
}
-
+
@Override
public TypeChooser getTypeChooser() {
return delegationController.getTypeChooser();
}
-
+
@Override
public AsmClassGenerator getAcg() {
return delegationController.getAcg();
}
-
+
@Override
public AssertionWriter getAssertionWriter() {
return delegationController.getAssertionWriter();
}
-
+
@Override
public BinaryExpressionHelper getBinaryExpressionHelper() {
return delegationController.getBinaryExpressionHelper();
@@ -97,17 +98,17 @@ public class DelegatingController extends WriterController {
public String getClassName() {
return delegationController.getClassName();
}
-
+
@Override
public ClassNode getClassNode() {
return delegationController.getClassNode();
}
-
+
@Override
public ClassVisitor getClassVisitor() {
return delegationController.getClassVisitor();
}
-
+
@Override
public ClosureWriter getClosureWriter() {
return delegationController.getClosureWriter();
@@ -127,72 +128,72 @@ public class DelegatingController extends WriterController {
public MethodReferenceExpressionWriter getMethodReferenceExpressionWriter() {
return delegationController.getMethodReferenceExpressionWriter();
}
-
+
@Override
public CompileStack getCompileStack() {
return delegationController.getCompileStack();
}
-
+
@Override
public ConstructorNode getConstructorNode() {
return delegationController.getConstructorNode();
}
-
+
@Override
public GeneratorContext getContext() {
return delegationController.getContext();
}
-
+
@Override
public ClassVisitor getCv() {
return delegationController.getCv();
}
-
+
@Override
public InterfaceHelperClassNode getInterfaceClassLoadingClass() {
return delegationController.getInterfaceClassLoadingClass();
}
-
+
@Override
public String getInternalBaseClassName() {
return delegationController.getInternalBaseClassName();
}
-
+
@Override
public String getInternalClassName() {
return delegationController.getInternalClassName();
}
-
+
@Override
public InvocationWriter getInvocationWriter() {
return delegationController.getInvocationWriter();
}
-
+
@Override
public MethodNode getMethodNode() {
return delegationController.getMethodNode();
}
-
+
@Override
public MethodVisitor getMethodVisitor() {
return delegationController.getMethodVisitor();
}
-
+
@Override
public OperandStack getOperandStack() {
return delegationController.getOperandStack();
}
-
+
@Override
public ClassNode getOutermostClass() {
return delegationController.getOutermostClass();
}
-
+
@Override
public ClassNode getReturnType() {
return delegationController.getReturnType();
}
-
+
@Override
public SourceUnit getSourceUnit() {
return delegationController.getSourceUnit();
@@ -202,87 +203,82 @@ public class DelegatingController extends WriterController {
public boolean isConstructor() {
return delegationController.isConstructor();
}
-
+
@Override
public boolean isInClosure() {
return delegationController.isInClosure();
}
-
+
@Override
public boolean isInClosureConstructor() {
return delegationController.isInClosureConstructor();
}
-
+
@Override
public boolean isNotClinit() {
return delegationController.isNotClinit();
}
-
+
@Override
public boolean isInScriptBody() {
return delegationController.isInScriptBody();
}
-
- @Override
- public boolean isNotExplicitThisInClosure(boolean implicitThis) {
- return delegationController.isNotExplicitThisInClosure(implicitThis);
- }
-
+
@Override
public boolean isStaticConstructor() {
return delegationController.isStaticConstructor();
}
-
+
@Override
public boolean isStaticContext() {
return delegationController.isStaticContext();
}
-
+
@Override
public boolean isStaticMethod() {
return delegationController.isStaticMethod();
}
-
+
@Override
public void setInterfaceClassLoadingClass(InterfaceHelperClassNode ihc) {
delegationController.setInterfaceClassLoadingClass(ihc);
}
-
+
@Override
public void setMethodVisitor(MethodVisitor methodVisitor) {
delegationController.setMethodVisitor(methodVisitor);
}
-
+
@Override
public boolean shouldOptimizeForInt() {
return delegationController.shouldOptimizeForInt();
}
-
+
@Override
public void switchToFastPath() {
delegationController.switchToFastPath();
}
-
+
@Override
public void switchToSlowPath() {
delegationController.switchToSlowPath();
}
-
+
@Override
public int getBytecodeVersion() {
return delegationController.getBytecodeVersion();
}
-
+
@Override
public void setLineNumber(int n) {
delegationController.setLineNumber(n);
}
-
+
@Override
public int getLineNumber() {
return delegationController.getLineNumber();
}
-
+
@Override
public void resetLineNumber() {
delegationController.resetLineNumber();
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 954ae2f..a3cb6df 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
@@ -22,7 +22,6 @@ import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.ConstructorNode;
-import org.codehaus.groovy.ast.InnerClassNode;
import org.codehaus.groovy.ast.InterfaceHelperClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.classgen.AsmClassGenerator;
@@ -44,8 +43,9 @@ import java.util.Map;
import static org.apache.groovy.util.SystemUtil.getBooleanSafe;
public class WriterController {
- private static final String GROOVY_LOG_CLASSGEN = "groovy.log.classgen";
- private final boolean LOG_CLASSGEN = getBooleanSafe(GROOVY_LOG_CLASSGEN);
+
+ private final boolean LOG_CLASSGEN = getBooleanSafe("groovy.log.classgen");
+
private AsmClassGenerator acg;
private MethodVisitor methodVisitor;
private CompileStack compileStack;
@@ -74,23 +74,23 @@ public class WriterController {
private int bytecodeVersion = Opcodes.V1_8;
private int lineNumber = -1;
private int helperMethodIndex = 0;
- private List<String> superMethodNames = new ArrayList<String>();
+ private List<String> superMethodNames = new ArrayList<>();
private MethodPointerExpressionWriter methodPointerExpressionWriter;
private MethodReferenceExpressionWriter methodReferenceExpressionWriter;
- public void init(AsmClassGenerator asmClassGenerator, GeneratorContext gcon, ClassVisitor cv, ClassNode cn) {
+ public void init(final AsmClassGenerator asmClassGenerator, final GeneratorContext gcon, final ClassVisitor cv, final ClassNode cn) {
CompilerConfiguration config = cn.getCompileUnit().getConfig();
Map<String,Boolean> optOptions = config.getOptimizationOptions();
- boolean invokedynamic=false;
+ boolean invokedynamic = false;
if (optOptions.isEmpty()) {
// IGNORE
} else if (Boolean.FALSE.equals(optOptions.get("all"))) {
- optimizeForInt=false;
+ optimizeForInt = false;
// set other optimizations options to false here
} else {
- if (Boolean.TRUE.equals(optOptions.get(CompilerConfiguration.INVOKEDYNAMIC))) invokedynamic=true;
- if (Boolean.FALSE.equals(optOptions.get("int"))) optimizeForInt=false;
- if (invokedynamic) optimizeForInt=false;
+ if (Boolean.TRUE.equals(optOptions.get(CompilerConfiguration.INVOKEDYNAMIC))) invokedynamic = true;
+ if (Boolean.FALSE.equals(optOptions.get("int"))) optimizeForInt = false;
+ if (invokedynamic) optimizeForInt = false;
// set other optimizations options to false here
}
this.classNode = cn;
@@ -139,7 +139,7 @@ public class WriterController {
this.typeChooser = new StatementMetaTypeChooser();
}
- private ClassVisitor createClassVisitor(ClassVisitor cv) {
+ private ClassVisitor createClassVisitor(final ClassVisitor cv) {
if (!LOG_CLASSGEN) {
return cv;
}
@@ -167,7 +167,7 @@ public class WriterController {
return acg;
}
- public void setMethodVisitor(MethodVisitor methodVisitor) {
+ public void setMethodVisitor(final MethodVisitor methodVisitor) {
this.methodVisitor = methodVisitor;
}
@@ -255,22 +255,18 @@ public class WriterController {
return methodNode;
}
- public void setMethodNode(MethodNode mn) {
- methodNode = mn;
- constructorNode = null;
+ public void setMethodNode(final MethodNode methodNode) {
+ this.methodNode = methodNode;
+ this.constructorNode = null;
}
public ConstructorNode getConstructorNode(){
return constructorNode;
}
- public void setConstructorNode(ConstructorNode cn) {
- constructorNode = cn;
- methodNode = null;
- }
-
- public boolean isNotClinit() {
- return methodNode == null || !methodNode.getName().equals("<clinit>");
+ public void setConstructorNode(final ConstructorNode constructorNode) {
+ this.constructorNode = constructorNode;
+ this.methodNode = null;
}
public SourceUnit getSourceUnit() {
@@ -278,12 +274,12 @@ public class WriterController {
}
public boolean isStaticContext() {
- if (compileStack!=null && compileStack.getScope()!=null) {
+ if (compileStack != null && compileStack.getScope() != null) {
return compileStack.getScope().isInStaticContext();
}
if (!isInClosure()) return false;
- if (constructorNode != null) return false;
- return classNode.isStaticClass() || methodNode.isStatic();
+ if (isConstructor()) return false;
+ return classNode.isStaticClass() || isStaticMethod();
}
public boolean isInClosure() {
@@ -297,11 +293,6 @@ public class WriterController {
&& classNode.getSuperClass() == ClassHelper.CLOSURE_TYPE;
}
- public boolean isNotExplicitThisInClosure(boolean implicitThis) {
- return implicitThis || !isInClosure();
- }
-
-
public boolean isStaticMethod() {
return methodNode != null && methodNode.isStatic();
}
@@ -316,12 +307,16 @@ public class WriterController {
}
}
+ public boolean isNotClinit() {
+ return methodNode == null || !methodNode.getName().equals("<clinit>");
+ }
+
public boolean isStaticConstructor() {
return methodNode != null && methodNode.getName().equals("<clinit>");
}
public boolean isConstructor() {
- return constructorNode!=null;
+ return constructorNode != null;
}
/**
@@ -329,11 +324,7 @@ public class WriterController {
* local variables but are properties
*/
public boolean isInScriptBody() {
- if (classNode.isScriptBody()) {
- return true;
- } else {
- return classNode.isScript() && methodNode != null && methodNode.getName().equals("run");
- }
+ return classNode.isScriptBody() || (classNode.isScript() && methodNode != null && methodNode.getName().equals("run"));
}
public String getClassName() {
@@ -348,10 +339,8 @@ public class WriterController {
public ClassNode getOutermostClass() {
if (outermostClass == null) {
- outermostClass = classNode;
- while (outermostClass instanceof InnerClassNode) {
- outermostClass = outermostClass.getOuterClass();
- }
+ List<ClassNode> outers = classNode.getOuterClasses();
+ outermostClass = !outers.isEmpty() ? outers.get(outers.size() - 1) : classNode;
}
return outermostClass;
}
@@ -360,7 +349,7 @@ public class WriterController {
return context;
}
- public void setInterfaceClassLoadingClass(InterfaceHelperClassNode ihc) {
+ public void setInterfaceClassLoadingClass(final InterfaceHelperClassNode ihc) {
interfaceClassLoadingClass = ihc;
}
@@ -398,8 +387,8 @@ public class WriterController {
return lineNumber;
}
- public void setLineNumber(int n) {
- lineNumber = n;
+ public void setLineNumber(final int lineNumber) {
+ this.lineNumber = lineNumber;
}
public void resetLineNumber() {
@@ -407,11 +396,10 @@ public class WriterController {
}
public int getNextHelperMethodIndex() {
- return helperMethodIndex++;
+ return helperMethodIndex += 1;
}
public List<String> getSuperMethodNames() {
return superMethodNames;
}
-
}
[groovy] 02/02: GROOVY-8762: make WriterController#isInClosure()
more selective
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 3cbc7e3170330d250dc5a2036f910dbf15664199
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 25 17:26:10 2019 -0600
GROOVY-8762: make WriterController#isInClosure() more selective
- any inner class that extended Closure got closure literal treatment
---
.../groovy/classgen/asm/WriterController.java | 7 +++----
src/test/groovy/bugs/Groovy8762.groovy | 22 +++++++---------------
2 files changed, 10 insertions(+), 19 deletions(-)
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 a3cb6df..7b88390 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
@@ -284,13 +284,12 @@ public class WriterController {
public boolean isInClosure() {
return classNode.getOuterClass() != null
- && classNode.getSuperClass() == ClassHelper.CLOSURE_TYPE;
+ && classNode.getSuperClass().equals(ClassHelper.CLOSURE_TYPE)
+ && classNode.implementsInterface(ClassHelper.GENERATED_CLOSURE_Type);
}
public boolean isInClosureConstructor() {
- return constructorNode != null
- && classNode.getOuterClass() != null
- && classNode.getSuperClass() == ClassHelper.CLOSURE_TYPE;
+ return isConstructor() && isInClosure();
}
public boolean isStaticMethod() {
diff --git a/src/test/groovy/bugs/Groovy8762.groovy b/src/test/groovy/bugs/Groovy8762.groovy
index cb8c390..9fd5383 100644
--- a/src/test/groovy/bugs/Groovy8762.groovy
+++ b/src/test/groovy/bugs/Groovy8762.groovy
@@ -18,7 +18,6 @@
*/
package groovy.bugs
-import groovy.test.NotYetImplemented
import groovy.transform.CompileStatic
import org.junit.Test
@@ -27,7 +26,7 @@ import static groovy.test.GroovyAssert.assertScript
@CompileStatic
final class Groovy8762 {
- @Test @NotYetImplemented
+ @Test
void testExplicitThisObjectExpressionInInnerClassConstructor() {
assertScript '''
class Outer {
@@ -40,23 +39,15 @@ final class Groovy8762 {
this.y = i // NPE at groovy.bugs.Outer$Inner.<init>
}
- int getMaximumNumberOfParameters() {
- throw new UnsupportedOperationException()
- }
-
- Class<?>[] getParameterTypes() {
- throw new UnsupportedOperationException()
+ def doCall(... args) {
+ return 42
}
- Object call(Object... args) {
- throw new UnsupportedOperationException()
- }
-
- Object call(Object arguments) {
+ int getMaximumNumberOfParameters() {
throw new UnsupportedOperationException()
}
- Object call() {
+ Class<?>[] getParameterTypes() {
throw new UnsupportedOperationException()
}
}
@@ -66,7 +57,8 @@ final class Groovy8762 {
}
}
- assert new Outer().makeCallable()
+ def callback = new Outer().makeCallable()
+ assert callback() == 42
'''
}
}