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:48:07 UTC
[groovy] branch GROOVY_2_5_X updated (241df9a -> 3dcbfb1)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a change to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from 241df9a bump baseline version
new bcd8b69 GROOVY-8954: VerifyError due to incorrect bytecode produced when a trait super property call also comes from an interface (closes #852)
new 3dcbfb1 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 GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit bcd8b6914f88ba39e04db05b9b25c0e8f9b888af
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 a7dd34c..79e9946 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 GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 3dcbfb1613e0da519443b77892e585e37a313298
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()
+ }
+ }
+ }
+}