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/01/11 22:46:28 UTC
[groovy] branch master updated (20d895a -> 50b77b2)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from 20d895a Add a test to reproduce "GROOVY-7812: Static inner classes cannot be accessed from other files when running by 'groovy' command"
new 5be8010 GROOVY-8954: VerifyError due to incorrect bytecode produced when a trait super property call also comes from an interface (closes #852)
new 50b77b2 GROOVY-8951: Traits defining getter conflicts with generated getter (improvements for pre-compiled case) (closes #851)
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/asm/InvocationWriter.java | 4 +-
.../groovy/tools/javac/JavaStubGenerator.java | 71 ++++++++++++---------
.../groovy/transform/trait/TraitComposer.java | 14 +----
.../codehaus/groovy/transform/trait/Traits.java | 19 ++++++
.../traitx/TraitASTTransformationTest.groovy | 20 ++++++
.../test/groovy/groovy/ant/Groovy8951Test.groovy | 73 ++++++++++++++++++++++
6 files changed, 157 insertions(+), 44 deletions(-)
create mode 100644 subprojects/groovy-ant/src/test/groovy/groovy/ant/Groovy8951Test.groovy
[groovy] 01/02: GROOVY-8954: VerifyError due to incorrect bytecode
produced when a trait super property call also comes from an interface
(closes #852)
Posted by pa...@apache.org.
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 5be8010a3505c2b20fbdd67bc903a4b1a3d010d1
Author: Paul King <pa...@asert.com.au>
AuthorDate: Fri Jan 11 18:20:13 2019 +1000
GROOVY-8954: VerifyError due to incorrect bytecode produced when a trait super property call also comes from an interface (closes #852)
---
.../groovy/classgen/asm/InvocationWriter.java | 4 ++--
.../traitx/TraitASTTransformationTest.groovy | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
index 899d5cb..3287481 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -151,10 +151,10 @@ public class InvocationWriter {
int opcode = INVOKEVIRTUAL;
if (target.isStatic()) {
opcode = INVOKESTATIC;
- } else if (target.isPrivate() || ((receiver instanceof VariableExpression && ((VariableExpression) receiver).isSuperExpression()))) {
- opcode = INVOKESPECIAL;
} else if (declaringClass.isInterface()) {
opcode = INVOKEINTERFACE;
+ } else if (target.isPrivate() || ((receiver instanceof VariableExpression && ((VariableExpression) receiver).isSuperExpression()))) {
+ opcode = INVOKESPECIAL;
}
// handle receiver
diff --git a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
index 27ecd8d..d4c27b3 100644
--- a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
@@ -2668,4 +2668,24 @@ assert c.b() == 2
assert Counter.count == 11
'''
}
+
+ //GROOVY-8954
+ void testTraitWithPropertyAlsoFromInterfaceSC() {
+ assertScript '''
+ interface DomainProp {
+ boolean isNullable()
+ }
+
+ abstract class OrderedProp implements DomainProp { }
+
+ trait Nullable {
+ boolean nullable = true
+ }
+
+ @groovy.transform.CompileStatic
+ abstract class CustomProp extends OrderedProp implements Nullable { }
+
+ assert new CustomProp() {}
+ '''
+ }
}
[groovy] 02/02: GROOVY-8951: Traits defining getter conflicts with
generated getter (improvements for pre-compiled case) (closes #851)
Posted by pa...@apache.org.
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 50b77b2f60baf8e3456c2cc19c944339b28f1d27
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Jan 8 13:13:08 2019 +1000
GROOVY-8951: Traits defining getter conflicts with generated getter (improvements for pre-compiled case) (closes #851)
---
.../groovy/tools/javac/JavaStubGenerator.java | 71 ++++++++++++---------
.../groovy/transform/trait/TraitComposer.java | 14 +----
.../codehaus/groovy/transform/trait/Traits.java | 19 ++++++
.../test/groovy/groovy/ant/Groovy8951Test.groovy | 73 ++++++++++++++++++++++
4 files changed, 135 insertions(+), 42 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java b/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
index 4b9efbb..64b3ee9 100644
--- a/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
+++ b/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
@@ -32,6 +32,7 @@ import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.ModuleNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.PropertyNode;
+import org.codehaus.groovy.ast.decompiled.DecompiledClassNode;
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.ClassExpression;
import org.codehaus.groovy.ast.expr.ClosureExpression;
@@ -150,12 +151,9 @@ public class JavaStubGenerator {
List<Statement> savedStatements = new ArrayList<Statement>(node.getObjectInitializerStatements());
super.visitClass(node);
node.getObjectInitializerStatements().addAll(savedStatements);
- for (ClassNode inode : node.getAllInterfaces()) {
- if (Traits.isTrait(inode)) {
- List<PropertyNode> traitProps = inode.getProperties();
- for (PropertyNode pn : traitProps) {
- super.visitProperty(pn);
- }
+ for (ClassNode trait : Traits.findTraits(node)) {
+ for (PropertyNode traitProperty : trait.getProperties()) {
+ super.visitProperty(traitProperty);
}
}
}
@@ -322,38 +320,53 @@ public class JavaStubGenerator {
printMethod(out, classNode, method);
}
- for (ClassNode node : classNode.getAllInterfaces()) {
- if (Traits.isTrait(node)) {
- List<MethodNode> traitMethods = node.getMethods();
- for (MethodNode traitMethod : traitMethods) {
- MethodNode method = classNode.getMethod(traitMethod.getName(), traitMethod.getParameters());
- if (method == null) {
- for (MethodNode methodNode : propertyMethods) {
- if (methodNode.getName().equals(traitMethod.getName())) {
- boolean sameParams = sameParameterTypes(methodNode);
- if (sameParams) {
- method = methodNode;
- break;
- }
- }
- }
- if (method==null && !traitMethod.isAbstract()) {
- printMethod(out, classNode, traitMethod);
+ // print the methods from traits
+ for (ClassNode trait : Traits.findTraits(classNode)) {
+ List<MethodNode> traitMethods = trait.getMethods();
+ for (MethodNode traitMethod : traitMethods) {
+ MethodNode existingMethod = classNode.getMethod(traitMethod.getName(), traitMethod.getParameters());
+ if (existingMethod != null) continue;
+ for (MethodNode propertyMethod : propertyMethods) {
+ if (propertyMethod.getName().equals(traitMethod.getName())) {
+ boolean sameParams = sameParameterTypes(propertyMethod, traitMethod);
+ if (sameParams) {
+ existingMethod = propertyMethod;
+ break;
}
}
}
+ if (existingMethod != null) continue;
+ boolean isCandidate = isCandidateTraitMethod(trait, traitMethod);
+ if (!isCandidate) continue;
+ printMethod(out, classNode, traitMethod);
}
}
+ }
+
+ private boolean isCandidateTraitMethod(ClassNode trait, MethodNode traitMethod) {
+ boolean precompiled = trait.redirect() instanceof DecompiledClassNode;
+ if (!precompiled) return !traitMethod.isAbstract();
+ List<MethodNode> helperMethods = Traits.findHelper(trait).getMethods();
+ for (MethodNode helperMethod : helperMethods) {
+ boolean isSynthetic = (traitMethod.getModifiers() & Opcodes.ACC_SYNTHETIC) != 0;
+ if (helperMethod.getName().equals(traitMethod.getName()) && !isSynthetic && !traitMethod.getName().contains("$")) {
+ Parameter[] origParams = helperMethod.getParameters();
+ Parameter[] newParams = Arrays.copyOfRange(origParams, 1, origParams.length);
+ if (sameParameterTypes(newParams, traitMethod.getParameters())) return true;
+ }
+ }
+ return false;
+ }
+ private static boolean sameParameterTypes(final MethodNode firstMethod, final MethodNode secondMethod) {
+ return sameParameterTypes(firstMethod.getParameters(), secondMethod.getParameters());
}
- private static boolean sameParameterTypes(final MethodNode methodNode) {
- Parameter[] a = methodNode.getParameters();
- Parameter[] b = methodNode.getParameters();
- boolean sameParams = a.length == b.length;
+ private static boolean sameParameterTypes(final Parameter[] firstParams, final Parameter[] secondParams) {
+ boolean sameParams = firstParams.length == secondParams.length;
if (sameParams) {
- for (int i = 0; i < a.length; i++) {
- if (!a[i].getType().equals(b[i].getType())) {
+ for (int i = 0; i < firstParams.length; i++) {
+ if (!firstParams[i].getType().equals(secondParams[i].getType())) {
sameParams = false;
break;
}
diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
index 5b80061..e85fe4d 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
@@ -106,7 +106,7 @@ public abstract class TraitComposer {
return;
}
if (!cNode.getNameWithoutPackage().endsWith(Traits.TRAIT_HELPER)) {
- List<ClassNode> traits = findTraits(cNode);
+ List<ClassNode> traits = Traits.findTraits(cNode);
for (ClassNode trait : traits) {
TraitHelpersTuple helpers = Traits.findHelpers(trait);
applyTrait(trait, cNode, helpers, unit);
@@ -119,18 +119,6 @@ public abstract class TraitComposer {
}
}
- private static List<ClassNode> findTraits(ClassNode cNode) {
- LinkedHashSet<ClassNode> interfaces = new LinkedHashSet<ClassNode>();
- Traits.collectAllInterfacesReverseOrder(cNode, interfaces);
- List<ClassNode> traits = new LinkedList<ClassNode>();
- for (ClassNode candidate : interfaces) {
- if (Traits.isAnnotatedWithTrait(candidate)) {
- traits.add(candidate);
- }
- }
- return traits;
- }
-
private static void checkTraitAllowed(final ClassNode bottomTrait, final SourceUnit unit) {
ClassNode superClass = bottomTrait.getSuperClass();
if (superClass==null || ClassHelper.OBJECT_TYPE.equals(superClass)) return;
diff --git a/src/main/java/org/codehaus/groovy/transform/trait/Traits.java b/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
index c83a71d..2703de4 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
@@ -44,6 +44,7 @@ import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Iterator;
import java.util.LinkedHashSet;
+import java.util.LinkedList;
import java.util.List;
/**
@@ -363,6 +364,24 @@ public abstract class Traits {
}
/**
+ * Find all traits associated with the given classnode
+ *
+ * @param cNode the given classnode
+ * @return the list of ordered trait classnodes
+ */
+ public static List<ClassNode> findTraits(ClassNode cNode) {
+ LinkedHashSet<ClassNode> interfaces = new LinkedHashSet<ClassNode>();
+ collectAllInterfacesReverseOrder(cNode, interfaces);
+ List<ClassNode> traits = new LinkedList<ClassNode>();
+ for (ClassNode candidate : interfaces) {
+ if (isAnnotatedWithTrait(candidate)) {
+ traits.add(candidate);
+ }
+ }
+ return traits;
+ }
+
+ /**
* Internal annotation used to indicate which methods in a trait interface have a
* default implementation.
*/
diff --git a/subprojects/groovy-ant/src/test/groovy/groovy/ant/Groovy8951Test.groovy b/subprojects/groovy-ant/src/test/groovy/groovy/ant/Groovy8951Test.groovy
new file mode 100644
index 0000000..e4a5229
--- /dev/null
+++ b/subprojects/groovy-ant/src/test/groovy/groovy/ant/Groovy8951Test.groovy
@@ -0,0 +1,73 @@
+/*
+ * 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 groovy.ant
+
+class Groovy8951Test extends AntTestCase {
+ void testSetterAppearsInTraitClass() {
+// def debugLogger = new org.apache.tools.ant.DefaultLogger()
+// debugLogger.setMessageOutputLevel(4)
+// debugLogger.setOutputPrintStream(System.out)
+// debugLogger.setErrorPrintStream(System.err)
+
+ doInTmpDir { ant, baseDir ->
+ baseDir.src {
+ p1 {
+ 'Validateable.groovy'('''
+ package p1
+ @groovy.transform.CompileStatic
+ trait Validateable {
+ List errors
+ List getErrors() {
+ errors
+ }
+ abstract void foo()
+ }
+ ''')
+ }
+ p2 {
+ 'Main.java'('''
+ package p2;
+ public class Main {
+ MyValidateable temp = new MyValidateable();
+ }
+ ''')
+ 'OtherTrait.groovy'('''
+ package p2
+ trait OtherTrait {
+ void otherMethod() { println new Date() }
+ }
+ ''')
+ 'MyValidateable.groovy'('''
+ package p2
+ class MyValidateable implements OtherTrait, p1.Validateable {
+ void foo() {}
+ }
+ ''')
+ }
+ }
+// ant.project.addBuildListener(debugLogger)
+ ant.mkdir(dir: 'build')
+ ant.taskdef(name: 'groovyc', classname: 'org.codehaus.groovy.ant.Groovyc')
+ ant.groovyc(srcdir: 'src', destdir: 'build', includes: 'p1/*')
+ ant.groovyc(srcdir: 'src', destdir: 'build', includes: 'p2/*' /*, keepStubs: true*/) {
+ javac()
+ }
+ }
+ }
+}