You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2019/12/01 10:41:38 UTC
[groovy] branch GROOVY_3_0_X updated (efbf991 -> 4fb0893)
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a change to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from efbf991 GROOVY-9326: Bump CodeNarc to 1.5 (#1113)
new 90f44af organize members and other minor edits
new 91a65f0 remove dead code
new 58b35d4 reset flags after constructor (like after method) and other minor edits
new 5515c09 GROOVY-9317: record field expression value for assert
new 45610c5 GROOVY-9318: add support for ** syntax in star import white/black lists
new 4d4be63 refactor to remove repeated creation of Class<Type>
new 285e82e GROOVY-8648: fix ASM error for this or super attribute expression on LHS
new b8f8728 GROOVY-8762: add test case
new 496524c minor edits
new d62a3f0 GROOVY-8762: make WriterController#isInClosure() more selective
new a95cdb3 refactor to move property-only field checking to visitPropertyExpression
new b977dc4 add test case for super attribte expression
new 4dfdb1a make parameters final and remove dead path in visitAttributeExpression
new 1c9d4e5 refactor InvocationWriter to reduce duplication around makeCall
new 4259a53 GROOVY-8468: add test case
new 2731e71 GROOVY-8361: add test case
new 8815125 refactor and reorganize
new 9dd7eda refactor dynamic variable handling
new e15269a track "in closure" and other minor edits
new 4fb0893 fix for "in closure" tracking
The 20 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 | 534 ++++++++---------
.../groovy/classgen/VariableScopeVisitor.java | 578 +++++++++---------
.../groovy/classgen/asm/DelegatingController.java | 94 ++-
.../groovy/classgen/asm/InvocationWriter.java | 401 +++++--------
.../classgen/asm/OptimizingStatementWriter.java | 649 +++++++++++----------
.../groovy/classgen/asm/WriterController.java | 85 ++-
.../classgen/asm/sc/StaticInvocationWriter.java | 13 +-
.../control/customizers/SecureASTCustomizer.java | 84 +--
.../transform/stc/StaticTypeCheckingVisitor.java | 39 +-
src/spec/test/semantics/PowerAssertTest.groovy | 24 +-
src/test/groovy/bugs/Groovy8361.groovy | 115 ++++
.../bugs/{Groovy9265.groovy => Groovy8468.groovy} | 21 +-
.../bugs/{Groovy9265.groovy => Groovy8648.groovy} | 26 +-
.../bugs/{Groovy6996.groovy => Groovy8762.groovy} | 49 +-
.../test/groovy/bugs/groovy8468/Face.java | 46 +-
.../groovy/bugs/groovy8468/FaceImpl.java} | 46 +-
.../groovy/bugs/groovy8468/Factory.java} | 47 +-
.../groovy/bugs/groovy8468/FactoryImpl.java} | 58 +-
.../groovy/classgen/asm/sc/bugs/Groovy7300.groovy | 22 +-
.../customizers/SecureASTCustomizerTest.groovy | 449 ++++++++------
.../powerassert/AssertionRenderingTest.groovy | 290 +++++----
.../runtime/powerassert/ValueRenderingTest.groovy | 121 ++--
22 files changed, 1973 insertions(+), 1818 deletions(-)
create mode 100644 src/test/groovy/bugs/Groovy8361.groovy
copy src/test/groovy/bugs/{Groovy9265.groovy => Groovy8468.groovy} (72%)
copy src/test/groovy/bugs/{Groovy9265.groovy => Groovy8648.groovy} (68%)
copy src/test/groovy/bugs/{Groovy6996.groovy => Groovy8762.groovy} (54%)
copy subprojects/parser-antlr4/src/test/resources/fail/FieldDeclaration_01x.groovy => src/test/groovy/bugs/groovy8468/Face.java (91%)
copy src/{main/java/groovy/lang/Buildable.java => test/groovy/bugs/groovy8468/FaceImpl.java} (89%)
copy src/{main/java/groovy/lang/Buildable.java => test/groovy/bugs/groovy8468/Factory.java} (85%)
copy src/{main/java/org/codehaus/groovy/runtime/wrappers/ShortWrapper.java => test/groovy/bugs/groovy8468/FactoryImpl.java} (64%)
[groovy] 15/20: GROOVY-8468: add test case
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 4259a533a729e53fa287134f80bcd7ad5e950453
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Nov 27 08:06:11 2019 -0600
GROOVY-8468: add test case
(cherry picked from commit 4edad9994113a54ee3c3d47d658c3e369f866c82)
---
src/test/groovy/bugs/Groovy8468.groovy | 46 ++++++++++++++++++++++++
src/test/groovy/bugs/groovy8468/Face.java | 22 ++++++++++++
src/test/groovy/bugs/groovy8468/FaceImpl.java | 22 ++++++++++++
src/test/groovy/bugs/groovy8468/Factory.java | 23 ++++++++++++
src/test/groovy/bugs/groovy8468/FactoryImpl.java | 32 +++++++++++++++++
5 files changed, 145 insertions(+)
diff --git a/src/test/groovy/bugs/Groovy8468.groovy b/src/test/groovy/bugs/Groovy8468.groovy
new file mode 100644
index 0000000..b15a561
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8468.groovy
@@ -0,0 +1,46 @@
+/*
+ * 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.bugs
+
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+@CompileStatic
+final class Groovy8468 {
+
+ @Test
+ void testGenericArrayType() {
+ assertScript '''
+ import groovy.bugs.groovy8468.*
+
+ @groovy.transform.CompileStatic
+ def test() {
+ Factory factory = new FactoryImpl()
+ Face[] array = factory.makeArray(FaceImpl) // NoSuchMethodError: Factory.makeArray(Ljava/lang/Class;)[Ljava/lang/Object;
+ return array
+ }
+
+ def result = test()
+ assert result != null
+ assert result.length == 0
+ '''
+ }
+}
diff --git a/src/test/groovy/bugs/groovy8468/Face.java b/src/test/groovy/bugs/groovy8468/Face.java
new file mode 100644
index 0000000..1ccc9c3
--- /dev/null
+++ b/src/test/groovy/bugs/groovy8468/Face.java
@@ -0,0 +1,22 @@
+/*
+ * 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.bugs.groovy8468;
+
+public interface Face {
+}
diff --git a/src/test/groovy/bugs/groovy8468/FaceImpl.java b/src/test/groovy/bugs/groovy8468/FaceImpl.java
new file mode 100644
index 0000000..25e51d9
--- /dev/null
+++ b/src/test/groovy/bugs/groovy8468/FaceImpl.java
@@ -0,0 +1,22 @@
+/*
+ * 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.bugs.groovy8468;
+
+public class FaceImpl implements Face {
+}
diff --git a/src/test/groovy/bugs/groovy8468/Factory.java b/src/test/groovy/bugs/groovy8468/Factory.java
new file mode 100644
index 0000000..2281466
--- /dev/null
+++ b/src/test/groovy/bugs/groovy8468/Factory.java
@@ -0,0 +1,23 @@
+/*
+ * 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.bugs.groovy8468;
+
+public interface Factory {
+ public <T extends Face> T[] makeArray(Class<T> clazz);
+}
diff --git a/src/test/groovy/bugs/groovy8468/FactoryImpl.java b/src/test/groovy/bugs/groovy8468/FactoryImpl.java
new file mode 100644
index 0000000..1d25532
--- /dev/null
+++ b/src/test/groovy/bugs/groovy8468/FactoryImpl.java
@@ -0,0 +1,32 @@
+/*
+ * 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.bugs.groovy8468;
+
+public class FactoryImpl implements Factory {
+ @Override @SuppressWarnings("unchecked")
+ public <T extends Face> T[] makeArray(Class<T> clazz) {
+ return (T[]) java.lang.reflect.Array.newInstance(clazz, 0);
+ }
+
+ /*public static void main(String[] args) {
+ Factory factory = new FactoryImpl();
+ Face[] array = factory.makeArray(FaceImpl.class);
+ System.out.println(array);
+ }*/
+}
[groovy] 04/20: GROOVY-9317: record field expression value for
assert
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 5515c094dfa13171e382156f382bfa7becbde2ca
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Nov 23 19:50:08 2019 -0600
GROOVY-9317: record field expression value for assert
(cherry picked from commit 9d75b171be28d3f33033ac3fa13b1296d5028cb0)
---
.../groovy/classgen/AsmClassGenerator.java | 33 +--
src/spec/test/semantics/PowerAssertTest.groovy | 24 +-
.../powerassert/AssertionRenderingTest.groovy | 290 +++++++++++++--------
.../runtime/powerassert/ValueRenderingTest.groovy | 121 +++++----
4 files changed, 291 insertions(+), 177 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index bcc140f..2c688f8 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -962,18 +962,17 @@ public class AsmClassGenerator extends ClassGenerator {
&& controller.isStaticContext()) {
// GROOVY-6183
ClassNode current = classNode.getSuperClass();
- while (field==null && current!=null) {
+ while (field == null && current != null) {
field = current.getDeclaredField(name);
current = current.getSuperClass();
}
- if (field!=null && (field.isProtected() || field.isPublic())) {
+ if (field != null && (field.isProtected() || field.isPublic())) {
visitFieldExpression(new FieldExpression(field));
return;
}
}
}
- if (field != null && !privateSuperField) {
- // GROOVY-4497: don't visit super field if it is private
+ if (field != null && !privateSuperField) { // GROOVY-4497: don't visit super field if it is private
visitFieldExpression(new FieldExpression(field));
return;
}
@@ -1157,9 +1156,9 @@ public class AsmClassGenerator extends ClassGenerator {
if (name != null) {
FieldNode field = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
if (field != null) {
- FieldExpression exp = new FieldExpression(field);
- exp.setSourcePosition(expression);
- visitFieldExpression(exp);
+ FieldExpression fldExp = new FieldExpression(field);
+ fldExp.setSourcePosition(expression.getProperty());
+ visitFieldExpression(fldExp);
return;
}
}
@@ -1178,17 +1177,17 @@ public class AsmClassGenerator extends ClassGenerator {
if (usesSuper(expression)) adapter = getFieldOnSuper;
}
visitAttributeOrProperty(expression, adapter);
- if (!controller.getCompileStack().isLHS()) {
- controller.getAssertionWriter().record(expression.getProperty());
- } else {
+ if (controller.getCompileStack().isLHS()) {
operandStack.remove(operandStack.getStackLength() - mark);
+ } else {
+ controller.getAssertionWriter().record(expression.getProperty());
}
}
private static boolean usesSuper(PropertyExpression pe) {
- Expression expression = pe.getObjectExpression();
- if (expression instanceof VariableExpression) {
- VariableExpression varExp = (VariableExpression) expression;
+ Expression objExp = pe.getObjectExpression();
+ if (objExp instanceof VariableExpression) {
+ VariableExpression varExp = (VariableExpression) objExp;
String variable = varExp.getName();
return variable.equals("super");
}
@@ -1211,7 +1210,9 @@ public class AsmClassGenerator extends ClassGenerator {
loadInstanceField(expression);
}
}
- if (controller.getCompileStack().isLHS()) controller.getAssertionWriter().record(expression);
+ if (!controller.getCompileStack().isLHS()) {
+ controller.getAssertionWriter().record(expression);
+ }
}
public void loadStaticField(FieldExpression fldExp) {
@@ -1348,7 +1349,9 @@ public class AsmClassGenerator extends ClassGenerator {
} else {
controller.getOperandStack().loadOrStoreVariable(variable, expression.isUseReferenceDirectly());
}
- if (!controller.getCompileStack().isLHS()) controller.getAssertionWriter().record(expression);
+ if (!controller.getCompileStack().isLHS()) {
+ controller.getAssertionWriter().record(expression);
+ }
}
private void loadThis(VariableExpression thisExpression) {
diff --git a/src/spec/test/semantics/PowerAssertTest.groovy b/src/spec/test/semantics/PowerAssertTest.groovy
index 5e42027..85d5b6c 100644
--- a/src/spec/test/semantics/PowerAssertTest.groovy
+++ b/src/spec/test/semantics/PowerAssertTest.groovy
@@ -18,17 +18,21 @@
*/
package semantics
-import asciidoctor.Utils
-import groovy.test.GroovyTestCase
+import org.junit.Test
-class PowerAssertTest extends GroovyTestCase {
+import static asciidoctor.Utils.stripAsciidocMarkup
+import static groovy.test.GroovyAssert.shouldFail
+
+final class PowerAssertTest {
+
+ @Test
void testPowerAssert() {
- def msg = shouldFail {
+ def err = shouldFail {
//tag::assert_code_1[]
assert 1+1 == 3
//end::assert_code_1[]
}
- assert msg == Utils.stripAsciidocMarkup('''
+ assert err.message == stripAsciidocMarkup('''
//tag::assert_error_1[]
assert 1+1 == 3
| |
@@ -37,8 +41,9 @@ assert 1+1 == 3
''')
}
+ @Test
void testPowerAssert2() {
- def msg = shouldFail {
+ def err = shouldFail {
//tag::assert_code_2[]
def x = 2
def y = 7
@@ -47,7 +52,7 @@ assert 1+1 == 3
assert calc(x,y) == [x,z].sum()
//end::assert_code_2[]
}
- assert msg == Utils.stripAsciidocMarkup('''
+ assert err.message == stripAsciidocMarkup('''
//tag::assert_error_2[]
assert calc(x,y) == [x,z].sum()
| | | | | | |
@@ -57,8 +62,9 @@ assert calc(x,y) == [x,z].sum()
''')
}
+ @Test
void testCustomAssertMessage() {
- def msg = shouldFail {
+ def err = shouldFail {
//tag::assert_code_3[]
def x = 2
def y = 7
@@ -67,7 +73,7 @@ assert calc(x,y) == [x,z].sum()
assert calc(x,y) == z*z : 'Incorrect computation result'
//end::assert_code_3[]
}
- assert msg == Utils.stripAsciidocMarkup('''
+ assert err.message == stripAsciidocMarkup('''
//tag::assert_error_3[]
Incorrect computation result. Expression: (calc.call(x, y) == (z * z)). Values: z = 5, z = 5
//end::assert_error_3[]
diff --git a/src/test/org/codehaus/groovy/runtime/powerassert/AssertionRenderingTest.groovy b/src/test/org/codehaus/groovy/runtime/powerassert/AssertionRenderingTest.groovy
index 9ce3a7c..f34d762 100644
--- a/src/test/org/codehaus/groovy/runtime/powerassert/AssertionRenderingTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/powerassert/AssertionRenderingTest.groovy
@@ -18,34 +18,42 @@
*/
package org.codehaus.groovy.runtime.powerassert
-import groovy.test.GroovyTestCase
+import org.junit.Test
-import static AssertionTestUtil.*
import static java.lang.Math.min
+import static org.codehaus.groovy.runtime.powerassert.AssertionTestUtil.isRendered
/**
* Tests rendering of whole assertions.
*/
+final class AssertionRenderingTest {
-class AssertionRenderingTest extends GroovyTestCase {
+ private one(x) { 0 }
+
+ private two(a, b) { 0 }
+
+ private three(a, b, c) { 0 }
+
+ @Test
void testSimpleAssertion() {
- isRendered """
+ isRendered '''
assert x == 1
| |
2 false
- """, {
+ ''', { ->
def x = 2
assert x == 1
}
}
+ @Test
void testMultiLineAssertion() {
- isRendered """
+ isRendered '''
assert 1 + 2 == 4 - 2
| | |
3 | 2
false
- """, {
+ ''', { ->
assert 1 +
2 ==
@@ -58,118 +66,125 @@ assert 1 + 2 == 4 - 2
}
}
- private one(x) { 0 }
-
+ @Test
void testMethodCallExpressionWithImplicitTarget() {
- isRendered """
+ isRendered '''
assert one(a)
| |
0 1
- """, {
+ ''', { ->
def a = 1
assert one(a)
}
}
+ @Test
void testMethodCallExpressionWithExplicitTarget() {
- isRendered """
+ isRendered '''
assert a.get(b) == null
| | | |
| 1 0 false
[1]
- """, {
+ ''', { ->
def a = [1]
def b = 0
assert a.get(b) == null
}
}
+ @Test
void testMethodCallExpressionWithGStringMethod() {
- isRendered """
+ isRendered '''
assert [1]."\$x"(0) == null
| | |
1 'get' false
- """, {
+ ''', { ->
def x = "get"
assert [1]."$x"(0) == null
}
}
+ @Test
void testMethodCallExpressionCallingStaticMethod() {
- isRendered """
+ isRendered '''
assert Math.max(a,b) == null
| | | |
2 1 2 false
- """, {
+ ''', { ->
def a = 1
def b = 2
assert Math.max(a,b) == null
}
}
+ @Test
void testMethodCallExpressionSpreadDot() {
- isRendered """
+ isRendered '''
assert ["1", "22"]*.size() == null
| |
[1, 2] false
- """, {
+ ''', { ->
assert ["1", "22"]*.size() == null
}
}
+ @Test
void testMethodCallExpressionSafe() {
- isRendered """
+ isRendered '''
assert a?.foo()
| |
| null
null
- """, {
+ ''', { ->
def a = null
assert a?.foo()
}
}
+ @Test
void testStaticMethodCallExpression() {
- isRendered """
+ isRendered '''
assert min(a,b) == null
| | | |
1 1 2 false
- """, {
+ ''', { ->
def a = 1
def b = 2
assert min(a,b) == null
}
}
+ @Test
void testConstructorCallExpression() {
- isRendered """
+ isRendered '''
assert new ArrayList(a) == null
| | |
[] 1 false
- """, {
+ ''', { ->
def a = 1
assert new ArrayList(a) == null
}
}
+ @Test
void testTernaryExpression() {
- isRendered """
+ isRendered '''
assert a ? b : c
| |
1 0
- """, {
+ ''', { ->
def a = 1
def b = 0
def c = 1
assert a ? b : c
}
- isRendered """
+ isRendered '''
assert a ? b : c
| |
0 0
- """, {
+ ''', { ->
def a = 0
def b = 1
def c = 0
@@ -177,126 +192,134 @@ assert a ? b : c
}
}
+ @Test
void testShortTernaryExpression() {
- isRendered """
+ isRendered '''
assert (a ?: b) == null
| |
1 false
- """, {
+ ''', { ->
def a = 1
def b = 2
assert (a ?: b) == null
}
- isRendered """
+ isRendered '''
assert a ?: b
| |
0 0
- """, {
+ ''', { ->
def a = 0
def b = 0
assert a ?: b
}
}
+ @Test
void testBinaryExpression() {
- isRendered """
+ isRendered '''
assert a * b
| | |
0 0 1
- """, {
+ ''', { ->
def a = 0
def b = 1
assert a * b
}
- isRendered """
+ isRendered '''
assert a[b]
|||
||0
|false
[false]
- """, {
+ ''', { ->
def a = [false]
def b = 0
assert a[b]
}
}
+ @Test
void testPrefixExpression() {
- isRendered """
+ isRendered '''
assert ++x == null
| | |
1 0 false
- """, {
+ ''', { ->
def x = 0
assert ++x == null
}
}
+ @Test
void testPostfixExpression() {
- isRendered """
+ isRendered '''
assert x++ == null
|| |
|0 false
0
- """, {
+ ''', { ->
def x = 0
assert x++ == null
}
}
+ @Test
void testBooleanExpression() {
- isRendered """
+ isRendered '''
assert a
|
null
- """, {
+ ''', { ->
def a = null
assert a
}
}
+ @Test
void testClosureExpression() {
- isRendered """
+ isRendered '''
assert { 1 + 2 } == null
|
false
- """, {
+ ''', { ->
assert { 1 + 2 } == null
}
}
+ @Test
void testTupleExpression() {
// TupleExpression is only used on LHS of (multi-)assignment,
// but LHS of assignment is not rewritten
- isRendered """
+ isRendered '''
assert ((a,b) = [1,2]) && false
| |
[1, 2] false
- """, {
+ ''', { ->
def a
def b
assert ((a,b) = [1,2]) && false
}
}
+ @Test
void testMapExpression() {
- isRendered """
+ isRendered '''
assert [a:b, c:d] == null
| | |
2 4 false
- """, {
+ ''', { ->
def b = 2
def d = 4
assert [a:b, c:d] == null
}
- isRendered """
+ isRendered '''
assert [(a):b, (c):d] == null
| | | | |
1 2 3 4 false
- """, {
+ ''', { ->
def a = 1
def b = 2
def c = 3
@@ -305,12 +328,13 @@ assert [(a):b, (c):d] == null
}
}
+ @Test
void testListExpression() {
- isRendered """
+ isRendered '''
assert [a,b,c] == null
| | | |
1 2 3 false
- """, {
+ ''', { ->
def a = 1
def b = 2
def c = 3
@@ -318,111 +342,165 @@ assert [a,b,c] == null
}
}
+ @Test
void testRangeExpression() {
- isRendered """
+ isRendered '''
assert (a..b) == null
| | |
1 2 false
- """, {
+ ''', { ->
def a = 1
def b = 2
assert (a..b) == null
}
- isRendered """
+ isRendered '''
assert (a..<b) == null
| | |
1 2 false
- """, {
+ ''', { ->
def a = 1
def b = 2
assert (a..<b) == null
}
}
+ @Test
void testPropertyExpression() {
- isRendered """
+ isRendered '''
assert a.bytes == null
| | |
| [65] false
'A'
-
- """, {
+ ''', { ->
def a = 'A'
assert a.bytes == null
}
- isRendered """
+ isRendered '''
assert Integer.MIN_VALUE == null
| |
| false
-2147483648
- """, {
+ ''', { ->
assert Integer.MIN_VALUE == null
}
}
+ @Test
void testAttributeExpression() {
- isRendered """
+ isRendered '''
assert holder.@x
| |
h 0
- """, {
+ ''', { ->
def holder = new Holder()
assert holder.@x
}
+
+ isRendered '''
+assert holder.@x != 0
+ | | |
+ h 0 false
+ ''', { ->
+ def holder = new Holder()
+ assert holder.@x != 0
+ }
+
+ isRendered '''
+assert 0 != holder.@x
+ | | |
+ | h 0
+ false
+ ''', { ->
+ def holder = new Holder()
+ assert 0 != holder.@x
+ }
+
+ isRendered '''
+assert this.@field == 0
+ | |
+ 1 false
+ ''', { ->
+ new Runnable() {
+ private int field = 1
+ @Override
+ public void run() {
+ assert this.@field == 0
+ }
+ }.run()
+ }
+
+ isRendered '''
+assert 0 == this.@field
+ | |
+ false 1
+ ''', { ->
+ new Runnable() {
+ private int field = 1
+ @Override
+ public void run() {
+ assert 0 == this.@field
+ }
+ }.run()
+ }
}
+ @Test
void testMethodPointerExpression() {
- isRendered """
+ isRendered '''
assert a.&"\$b" == null
| | |
[] | false
'get'
- """, {
+ ''', { ->
def a = []
def b = "get"
assert a.&"$b" == null
}
}
+ @Test
void testConstantExpression() {
- isRendered """
+ isRendered '''
assert 1 == "abc"
|
false
- """, {
+ ''', { ->
assert 1 == "abc"
}
}
+ @Test
void testClassExpression() {
- isRendered """
+ isRendered '''
assert List == String
|
false
- """, {
+ ''', { ->
assert List == String
}
}
+ @Test
void testVariableExpression() {
- isRendered """
+ isRendered '''
assert x
|
0
- """, {
+ ''', { ->
def x = 0
assert x
}
}
+ @Test
void testGStringExpression() {
isRendered '''
assert "$a and ${b + c}" == null
| | | | |
1 2 5 3 false
- ''', {
+ ''', { ->
def a = 1
def b = 2
def c = 3
@@ -430,138 +508,143 @@ assert "$a and ${b + c}" == null
}
}
+ @Test
void testArrayExpression() {
- isRendered """
+ isRendered '''
assert new int[a][b] == null
| | |
1 2 false
- """, {
+ ''', { ->
def a = 1
def b = 2
assert new int[a][b] == null
}
}
- private two(a, b) { 0 }
-
+ @Test
void testSpreadExpression() {
- isRendered """
+ isRendered '''
assert two(*a)
| |
0 [1, 2]
- """, {
+ ''', { ->
def a = [1, 2]
assert two(*a)
}
- isRendered """
+ isRendered '''
assert [1, *a] == null
| |
| false
[2, 3]
- """, {
+ ''', { ->
def a = [2, 3]
assert [1, *a] == null
}
}
+ @Test
void testSpreadMapExpression() {
- isRendered """
+ isRendered '''
assert one(*:m)
| |
0 ['a':1, 'b':2]
- """, {
+ ''', { ->
def m = [a:1, b:2]
assert one(*:m)
}
- isRendered """
+ isRendered '''
assert [a:1, *:m] == null
| |
| false
['b':2, 'c':3]
- """, {
+ ''', { ->
def m = [b:2, c:3]
assert [a:1, *:m] == null
}
}
+ @Test
void testNotExpression() {
- isRendered """
+ isRendered '''
assert !a
||
|true
false
- """, {
+ ''', { ->
def a = true
assert !a
}
}
+ @Test
void testUnaryMinusExpression() {
- isRendered """
+ isRendered '''
assert -a == null
|| |
|1 false
-1
- """, {
+ ''', { ->
def a = 1
assert -a == null
}
}
+ @Test
void testUnaryPlusExpression() {
- isRendered """
+ isRendered '''
assert +a == null
|| |
|1 false
1
- """, {
+ ''', { ->
def a = 1
assert +a == null
}
}
+ @Test
void testBitwiseNegationExpression() {
- isRendered """
+ isRendered '''
assert ~a == null
|| |
|1 false
-2
- """, {
+ ''', { ->
def a = 1
assert ~a == null
}
}
+ @Test
void testCastExpression() {
- isRendered """
+ isRendered '''
assert (List)a
|
null
- """, {
+ ''', { ->
def a = null
assert (List)a
}
- isRendered """
+ isRendered '''
assert a as int[]
|
null
- """, {
+ ''', { ->
def a = null
assert a as int[]
}
}
- private three(a,b,c) { 0 }
-
+ @Test
void testArgumentListExpression() {
- isRendered """
+ isRendered '''
assert three(a, b,c)
| | | |
0 1 2 3
- """, {
+ ''', { ->
def a = 1
def b = 2
def c = 3
@@ -569,6 +652,7 @@ assert three(a, b,c)
}
}
+ @Test
void testExplicitClosureCall() {
def func = { it }
@@ -577,7 +661,7 @@ assert func.call(42) == null
| | |
| 42 false
${func.toString()}
- """, {
+ """, { ->
assert func.call(42) == null
}
}
@@ -612,5 +696,5 @@ assert func.call(42) == null
@groovy.transform.PackageScope class Holder {
public x = 0
def getX() { 9 }
- String toString() { "h" }
+ String toString() { 'h' }
}
diff --git a/src/test/org/codehaus/groovy/runtime/powerassert/ValueRenderingTest.groovy b/src/test/org/codehaus/groovy/runtime/powerassert/ValueRenderingTest.groovy
index 3c69d96..7f08416 100644
--- a/src/test/org/codehaus/groovy/runtime/powerassert/ValueRenderingTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/powerassert/ValueRenderingTest.groovy
@@ -18,85 +18,92 @@
*/
package org.codehaus.groovy.runtime.powerassert
-import groovy.test.GroovyTestCase
+import org.junit.Test
-import static AssertionTestUtil.*
+import static org.codehaus.groovy.runtime.powerassert.AssertionTestUtil.isRendered
/**
* Tests rendering of individual values.
*/
+final class ValueRenderingTest {
-class ValueRenderingTest extends GroovyTestCase {
+ @Test
void testNullValue() {
- isRendered """
+ isRendered '''
assert x
|
null
- """, {
+ ''', { ->
def x = null
assert x
}
}
+ @Test
void testCharValue() {
- isRendered """
+ isRendered '''
assert x == null
| |
c false
- """, {
+ ''', { ->
def x = "c" as char
assert x == null
}
}
+ @Test
void testStringValue() {
- isRendered """
+ isRendered '''
assert x == null
| |
| false
'foo'
- """, {
+ ''', { ->
def x = "foo"
assert x == null
}
}
+ @Test
void testMultiLineStringValue() {
- isRendered """
+ isRendered '''
assert null == x
| |
| 'one\\ntwo\\rthree\\r\\nfour'
false
- """, {
+ ''', { ->
def x = "one\ntwo\rthree\r\nfour"
assert null == x
}
}
+ @Test
void testPrimitiveArrayValue() {
- isRendered """
+ isRendered '''
assert x == null
| |
| false
[1, 2]
- """, {
+ ''', { ->
def x = [1, 2] as int[]
assert x == null
}
}
+ @Test
void testObjectArrayValue() {
- isRendered """
+ isRendered '''
assert x == null
| |
| false
['one', 'two']
- """, {
+ ''', { ->
def x = ["one", "two"] as String[]
assert x == null
}
}
+ @Test
void testEmptyStringValue() {
def x = new String()
@@ -105,11 +112,12 @@ assert x == null
| |
| false
''
- ''', {
+ ''', { ->
assert x == null
}
}
+ @Test
void testEmptyStringBuilderValue() {
def x = new StringBuilder()
@@ -118,11 +126,12 @@ assert x == null
| |
| false
""
- ''', {
+ ''', { ->
assert x == null
}
}
+ @Test
void testEmptyStringBufferValue() {
def x = new StringBuffer()
@@ -131,37 +140,40 @@ assert x == null
| |
| false
""
- ''', {
+ ''', { ->
assert x == null
}
}
+ @Test
void testSingleLineToString() {
- isRendered """
+ isRendered '''
assert x == null
| |
| false
single line
- """, {
+ ''', { ->
def x = new SingleLineToString()
assert x == null
}
}
+ @Test
void testMultiLineToString() {
- isRendered """
+ isRendered '''
assert x == null
| |
| false
mul
tiple
lines
- """, {
+ ''', { ->
def x = new MultiLineToString()
assert x == null
}
}
+ @Test
void testNullToString() {
def x = new NullToString()
@@ -170,11 +182,12 @@ assert x == null
| |
| false
${x.objectToString()} (toString() == null)
- """, {
+ """, { ->
assert x == null
}
}
+ @Test
void testEmptyToString() {
def x = new EmptyToString()
@@ -183,11 +196,12 @@ assert x == null
| |
| false
${x.objectToString()} (toString() == \"\")
- """, {
+ """, { ->
assert x == null
}
}
+ @Test
void testThrowingToString() {
def x = new ThrowingToString()
@@ -196,46 +210,53 @@ assert x == null
| |
| false
${x.objectToString()} (toString() threw java.lang.UnsupportedOperationException)
- """, {
+ """, { ->
assert x == null
}
}
-}
-@groovy.transform.PackageScope class SingleLineToString {
- String toString() {
- "single line"
- }
-}
+ //--------------------------------------------------------------------------
-@groovy.transform.PackageScope class MultiLineToString {
- String toString() {
- "mul\ntiple\n lines"
+ private static class SingleLineToString {
+ @Override
+ String toString() {
+ 'single line'
+ }
}
-}
-@groovy.transform.PackageScope class NullToString {
- String objectToString() {
- super.toString()
+ private static class MultiLineToString {
+ @Override
+ String toString() {
+ 'mul\ntiple\n lines'
+ }
}
- String toString() { null }
-}
+ private static class NullToString {
+ String objectToString() {
+ super.toString()
+ }
-@groovy.transform.PackageScope class EmptyToString {
- String objectToString() {
- super.toString()
+ @Override
+ String toString() { null }
}
- String toString() { "" }
-}
+ private static class EmptyToString {
+ String objectToString() {
+ super.toString()
+ }
-@groovy.transform.PackageScope class ThrowingToString {
- String objectToString() {
- super.toString()
+ @Override
+ String toString() { '' }
}
- String toString() {
- throw new UnsupportedOperationException()
+ private static class ThrowingToString {
+ String objectToString() {
+ super.toString()
+ }
+
+ @Override
+ String toString() {
+ throw new UnsupportedOperationException()
+ }
}
}
[groovy] 03/20: reset flags after constructor (like after method)
and other minor edits
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 58b35d466a07bcd41a7c6e4523944aa2bfb35dba
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Nov 23 16:44:25 2019 -0600
reset flags after constructor (like after method) and other minor edits
(cherry picked from commit e03c5a6362de3d4239eedfeb0b5ef96c9c8aa597)
---
.../classgen/asm/OptimizingStatementWriter.java | 129 ++++++++++-----------
1 file changed, 64 insertions(+), 65 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
index 94ae2ef..75fbf73 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
@@ -50,7 +50,6 @@ import org.codehaus.groovy.ast.stmt.IfStatement;
import org.codehaus.groovy.ast.stmt.ReturnStatement;
import org.codehaus.groovy.ast.stmt.Statement;
import org.codehaus.groovy.ast.stmt.WhileStatement;
-import org.codehaus.groovy.ast.tools.ParameterUtils;
import org.codehaus.groovy.classgen.AsmClassGenerator;
import org.codehaus.groovy.classgen.Verifier;
import org.codehaus.groovy.control.SourceUnit;
@@ -72,6 +71,7 @@ import static org.codehaus.groovy.ast.ClassHelper.double_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
import static org.codehaus.groovy.ast.ClassHelper.long_TYPE;
+import static org.codehaus.groovy.ast.tools.ParameterUtils.parametersEqual;
import static org.codehaus.groovy.ast.tools.WideningCategories.isBigDecCategory;
import static org.codehaus.groovy.ast.tools.WideningCategories.isDoubleCategory;
import static org.codehaus.groovy.ast.tools.WideningCategories.isFloatingCategory;
@@ -88,6 +88,7 @@ import static org.objectweb.asm.Opcodes.INVOKEINTERFACE;
public class OptimizingStatementWriter extends StatementWriter {
+ // values correspond to BinaryExpressionMultiTypeDispatcher.typeMapKeyNames
private static final MethodCaller[] guards = {
null,
MethodCaller.newStatic(BytecodeInterface8.class, "isOrigInt"),
@@ -104,24 +105,20 @@ public class OptimizingStatementWriter extends StatementWriter {
private final WriterController controller;
private boolean fastPathBlocked;
- public OptimizingStatementWriter(WriterController controller) {
+ public OptimizingStatementWriter(final WriterController controller) {
super(controller);
this.controller = controller;
}
- private boolean notEnableFastPath(StatementMeta meta) {
- // return false if cannot do fast path and if are already on the path
- return fastPathBlocked || meta==null || !meta.optimize || controller.isFastPath();
- }
+ private FastPathData writeGuards(final StatementMeta meta, final Statement statement) {
+ if (fastPathBlocked || controller.isFastPath() || meta == null || !meta.optimize) return null;
- private FastPathData writeGuards(StatementMeta meta, Statement statement) {
- if (notEnableFastPath(meta)) return null;
controller.getAcg().onLineNumber(statement, null);
MethodVisitor mv = controller.getMethodVisitor();
FastPathData fastPathData = new FastPathData();
Label slowPath = new Label();
- for (int i=0; i<guards.length; i++) {
+ for (int i = 0, n = guards.length; i < n; i += 1) {
if (meta.involvedTypes[i]) {
guards[i].call(mv);
mv.visitJumpInsn(IFEQ, slowPath);
@@ -131,42 +128,40 @@ public class OptimizingStatementWriter extends StatementWriter {
// meta class check with boolean holder
String owner = BytecodeHelper.getClassInternalName(controller.getClassNode());
MethodNode mn = controller.getMethodNode();
- if (mn!=null) {
+ if (mn != null) {
mv.visitFieldInsn(GETSTATIC, owner, Verifier.STATIC_METACLASS_BOOL, "Z");
mv.visitJumpInsn(IFNE, slowPath);
}
- //standard metaclass check
+ // standard metaclass check
disabledStandardMetaClass.call(mv);
mv.visitJumpInsn(IFNE, slowPath);
// other guards here
-
mv.visitJumpInsn(GOTO, fastPathData.pathStart);
mv.visitLabel(slowPath);
return fastPathData;
}
- private void writeFastPathPrelude(FastPathData meta) {
+ private void writeFastPathPrelude(final FastPathData meta) {
MethodVisitor mv = controller.getMethodVisitor();
mv.visitJumpInsn(GOTO, meta.afterPath);
mv.visitLabel(meta.pathStart);
controller.switchToFastPath();
}
- private void writeFastPathEpilogue(FastPathData meta) {
+ private void writeFastPathEpilogue(final FastPathData meta) {
MethodVisitor mv = controller.getMethodVisitor();
mv.visitLabel(meta.afterPath);
controller.switchToSlowPath();
}
@Override
- public void writeBlockStatement(BlockStatement statement) {
+ public void writeBlockStatement(final BlockStatement statement) {
StatementMeta meta = statement.getNodeMetaData(StatementMeta.class);
FastPathData fastPathData = writeGuards(meta, statement);
-
- if (fastPathData==null) {
+ if (fastPathData == null) {
// normal mode with different paths
// important is to not to have a fastpathblock here,
// otherwise the per expression statement improvement
@@ -186,7 +181,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
@Override
- public void writeDoWhileLoop(DoWhileStatement statement) {
+ public void writeDoWhileLoop(final DoWhileStatement statement) {
if (controller.isFastPath()) {
super.writeDoWhileLoop(statement);
} else {
@@ -198,7 +193,7 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeDoWhileLoop(statement);
fastPathBlocked = oldFastPathBlock;
- if (fastPathData==null) return;
+ if (fastPathData == null) return;
writeFastPathPrelude(fastPathData);
super.writeDoWhileLoop(statement);
writeFastPathEpilogue(fastPathData);
@@ -206,7 +201,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
@Override
- protected void writeIteratorHasNext(MethodVisitor mv) {
+ protected void writeIteratorHasNext(final MethodVisitor mv) {
if (controller.isFastPath()) {
mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Iterator", "hasNext", "()Z", true);
} else {
@@ -215,7 +210,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
@Override
- protected void writeIteratorNext(MethodVisitor mv) {
+ protected void writeIteratorNext(final MethodVisitor mv) {
if (controller.isFastPath()) {
mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Iterator", "next", "()Ljava/lang/Object;", true);
} else {
@@ -224,7 +219,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
@Override
- protected void writeForInLoop(ForStatement statement) {
+ protected void writeForInLoop(final ForStatement statement) {
if (controller.isFastPath()) {
super.writeForInLoop(statement);
} else {
@@ -236,7 +231,7 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeForInLoop(statement);
fastPathBlocked = oldFastPathBlock;
- if (fastPathData==null) return;
+ if (fastPathData == null) return;
writeFastPathPrelude(fastPathData);
super.writeForInLoop(statement);
writeFastPathEpilogue(fastPathData);
@@ -244,7 +239,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
@Override
- protected void writeForLoopWithClosureList(ForStatement statement) {
+ protected void writeForLoopWithClosureList(final ForStatement statement) {
if (controller.isFastPath()) {
super.writeForLoopWithClosureList(statement);
} else {
@@ -256,7 +251,7 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeForLoopWithClosureList(statement);
fastPathBlocked = oldFastPathBlock;
- if (fastPathData==null) return;
+ if (fastPathData == null) return;
writeFastPathPrelude(fastPathData);
super.writeForLoopWithClosureList(statement);
writeFastPathEpilogue(fastPathData);
@@ -264,7 +259,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
@Override
- public void writeWhileLoop(WhileStatement statement) {
+ public void writeWhileLoop(final WhileStatement statement) {
if (controller.isFastPath()) {
super.writeWhileLoop(statement);
} else {
@@ -276,7 +271,7 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeWhileLoop(statement);
fastPathBlocked = oldFastPathBlock;
- if (fastPathData==null) return;
+ if (fastPathData == null) return;
writeFastPathPrelude(fastPathData);
super.writeWhileLoop(statement);
writeFastPathEpilogue(fastPathData);
@@ -284,11 +279,10 @@ public class OptimizingStatementWriter extends StatementWriter {
}
@Override
- public void writeIfElse(IfStatement statement) {
+ public void writeIfElse(final IfStatement statement) {
StatementMeta meta = statement.getNodeMetaData(StatementMeta.class);
FastPathData fastPathData = writeGuards(meta, statement);
-
- if (fastPathData==null) {
+ if (fastPathData == null) {
super.writeIfElse(statement);
} else {
boolean oldFastPathBlock = fastPathBlocked;
@@ -302,17 +296,8 @@ public class OptimizingStatementWriter extends StatementWriter {
}
}
- private boolean isNewPathFork(StatementMeta meta) {
- // meta.optimize -> can do fast path
- if (meta==null || !meta.optimize) return false;
- // fastPathBlocked -> slow path
- if (fastPathBlocked) return false;
- // controller.isFastPath() -> fastPath
- return !controller.isFastPath();
- }
-
@Override
- public void writeReturn(ReturnStatement statement) {
+ public void writeReturn(final ReturnStatement statement) {
if (controller.isFastPath()) {
super.writeReturn(statement);
} else {
@@ -329,7 +314,7 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeReturn(statement);
fastPathBlocked = oldFastPathBlock;
- if (fastPathData==null) return;
+ if (fastPathData == null) return;
writeFastPathPrelude(fastPathData);
super.writeReturn(statement);
writeFastPathEpilogue(fastPathData);
@@ -340,7 +325,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
@Override
- public void writeExpressionStatement(ExpressionStatement statement) {
+ public void writeExpressionStatement(final ExpressionStatement statement) {
if (controller.isFastPath()) {
super.writeExpressionStatement(statement);
} else {
@@ -358,7 +343,6 @@ public class OptimizingStatementWriter extends StatementWriter {
// (3) fast path possible and in slow or fastPath. Nothing to do here.
//
// the only case we need to handle is then (2).
-
if (isNewPathFork(meta) && writeDeclarationExtraction(statement)) {
if (meta.declaredVariableExpression != null) {
// declaration was replaced by assignment so we need to define the variable
@@ -371,7 +355,7 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeExpressionStatement(statement);
fastPathBlocked = oldFastPathBlock;
- if (fastPathData==null) return;
+ if (fastPathData == null) return;
writeFastPathPrelude(fastPathData);
super.writeExpressionStatement(statement);
writeFastPathEpilogue(fastPathData);
@@ -381,7 +365,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
}
- private boolean writeDeclarationExtraction(Statement statement) {
+ private boolean writeDeclarationExtraction(final Statement statement) {
Expression ex = null;
if (statement instanceof ReturnStatement) {
ReturnStatement rs = (ReturnStatement) statement;
@@ -390,7 +374,7 @@ public class OptimizingStatementWriter extends StatementWriter {
ExpressionStatement es = (ExpressionStatement) statement;
ex = es.getExpression();
} else {
- throw new GroovyBugError("unknown statement type :"+statement.getClass());
+ throw new GroovyBugError("unknown statement type :" + statement.getClass());
}
if (!(ex instanceof DeclarationExpression)) return true;
DeclarationExpression declaration = (DeclarationExpression) ex;
@@ -419,26 +403,32 @@ public class OptimizingStatementWriter extends StatementWriter {
ExpressionStatement es = (ExpressionStatement) statement;
es.setExpression(assignment);
} else {
- throw new GroovyBugError("unknown statement type :"+statement.getClass());
+ throw new GroovyBugError("unknown statement type :" + statement.getClass());
}
return true;
}
- public static void setNodeMeta(TypeChooser chooser, ClassNode classNode) {
- if (classNode.getNodeMetaData(ClassNodeSkip.class)!=null) return;
+ private boolean isNewPathFork(final StatementMeta meta) {
+ // meta.optimize -> can do fast path
+ if (meta == null || !meta.optimize) return false;
+ // fastPathBlocked -> slow path
+ if (fastPathBlocked) return false;
+ // controller.isFastPath() -> fastPath
+ return !controller.isFastPath();
+ }
+
+ public static void setNodeMeta(final TypeChooser chooser, final ClassNode classNode) {
+ if (classNode.getNodeMetaData(ClassNodeSkip.class) != null) return;
new OptVisitor(chooser).visitClass(classNode);
}
- private static StatementMeta addMeta(ASTNode node) {
- StatementMeta metaOld = node.getNodeMetaData(StatementMeta.class);
- StatementMeta meta = metaOld;
- if (meta==null) meta = new StatementMeta();
+ private static StatementMeta addMeta(final ASTNode node) {
+ StatementMeta meta = node.getNodeMetaData(StatementMeta.class, x -> new StatementMeta());
meta.optimize = true;
- if (metaOld==null) node.setNodeMetaData(StatementMeta.class, meta);
return meta;
}
- private static StatementMeta addMeta(ASTNode node, OptimizeFlagsCollector opt) {
+ private static StatementMeta addMeta(final ASTNode node, final OptimizeFlagsCollector opt) {
StatementMeta meta = addMeta(node);
meta.chainInvolvedTypes(opt);
return meta;
@@ -471,9 +461,15 @@ public class OptimizingStatementWriter extends StatementWriter {
@Override
public String toString() {
- StringBuilder ret = new StringBuilder("optimize=" + optimize + " target=" + target + " type=" + type + " involvedTypes=");
+ StringBuilder ret = new StringBuilder();
+ ret.append("optimize=").append(optimize);
+ ret.append(" target=").append(target);
+ ret.append(" type=").append(type);
+ ret.append(" involvedTypes=");
for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
- if (involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
+ if (involvedTypes[i]) {
+ ret.append(' ').append(typeMapKeyNames[i]);
+ }
}
return ret.toString();
}
@@ -490,13 +486,13 @@ public class OptimizingStatementWriter extends StatementWriter {
private final Deque<OptimizeFlagsEntry> previous = new LinkedList<>();
public void push() {
- previous.addLast(current);
+ previous.push(current);
current = new OptimizeFlagsEntry();
}
public void pop(final boolean propagateFlags) {
OptimizeFlagsEntry old = current;
- current = previous.removeLast();
+ current = previous.pop();
if (propagateFlags) {
chainCanOptimize(old.canOptimize);
chainShouldOptimize(old.shouldOptimize);
@@ -508,17 +504,19 @@ public class OptimizingStatementWriter extends StatementWriter {
@Override
public String toString() {
- StringBuilder ret;
+ StringBuilder ret = new StringBuilder();
if (current.shouldOptimize) {
- ret = new StringBuilder("should optimize, can = " + current.canOptimize);
+ ret.append("should optimize, can = " + current.canOptimize);
} else if (current.canOptimize) {
- ret = new StringBuilder("can optimize");
+ ret.append("can optimize");
} else {
- ret = new StringBuilder("don't optimize");
+ ret.append("don't optimize");
}
ret.append(" involvedTypes =");
for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
- if (current.involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
+ if (current.involvedTypes[i]) {
+ ret.append(' ').append(typeMapKeyNames[i]);
+ }
}
return ret.toString();
}
@@ -595,6 +593,7 @@ public class OptimizingStatementWriter extends StatementWriter {
public void visitConstructor(final ConstructorNode node) {
scope = node.getVariableScope();
super.visitConstructor(node);
+ opt.reset();
}
@Override
@@ -942,7 +941,7 @@ public class OptimizingStatementWriter extends StatementWriter {
List<ConstructorNode> ctors = node.getDeclaredConstructors();
MethodNode result = null;
for (ConstructorNode ctor : ctors) {
- if (ParameterUtils.parametersEqual(ctor.getParameters(), parameters)) {
+ if (parametersEqual(ctor.getParameters(), parameters)) {
result = ctor;
break;
}
[groovy] 12/20: add test case for super attribte expression
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit b977dc482918f596437265ebc72674e3c2bb9b5b
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Nov 26 12:51:30 2019 -0600
add test case for super attribte expression
(cherry picked from commit 528fde0edb57b4c369c6e224afa4b06fd0f5fda8)
---
.../groovy/classgen/AsmClassGenerator.java | 7 +++++++
.../groovy/classgen/asm/sc/bugs/Groovy7300.groovy | 22 +++++++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 680aa30..825a278 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -1123,6 +1123,13 @@ public class AsmClassGenerator extends ClassGenerator {
if (field != null) {
visitFieldExpression(new FieldExpression(field));
visited = true;
+ } else if (isSuperExpression(objectExpression)) {
+ if (controller.getCompileStack().isLHS()) {
+ setPropertyOfSuperClass(classNode, expression, controller.getMethodVisitor());
+ } else {
+ visitMethodCallExpression(new MethodCallExpression(objectExpression, "get" + capitalize(name), MethodCallExpression.NO_ARGUMENTS));
+ }
+ visited = true;
}
}
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7300.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7300.groovy
index 4cc1598..3fba642 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7300.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7300.groovy
@@ -46,7 +46,7 @@ final class Groovy7300 extends StaticTypeCheckingTestCase implements StaticCompi
'''
}
- void testShouldNotThrowStackOverflowWithSuper() {
+ void testShouldNotThrowStackOverflowWithSuper1() {
assertScript '''
class A {
private String field1 = 'test'
@@ -71,4 +71,24 @@ final class Groovy7300 extends StaticTypeCheckingTestCase implements StaticCompi
assert b.field1 == 'test 2'
'''
}
+
+ void testShouldNotThrowStackOverflowWithSuper2() {
+ assertScript '''
+ class A {
+ private String field = 'value'
+ String getField() { return field }
+ void setField(String value) { field = value }
+ }
+
+ class B extends A {
+ @Override
+ String getField() {
+ super.@field = 'reset'
+ return super.field
+ }
+ }
+
+ assert new B().field == 'reset'
+ '''
+ }
}
[groovy] 11/20: refactor to move property-only field checking to
visitPropertyExpression
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit a95cdb399528e12e1de751be414e53f3095aa3e1
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Nov 26 11:23:17 2019 -0600
refactor to move property-only field checking to visitPropertyExpression
replace usesSuper(PropertyExpression) with isSuperExpression(Expression)
(cherry picked from commit 3f37f60c7d0b4393c3d1f34c4880cb02c7296f62)
---
.../groovy/classgen/AsmClassGenerator.java | 207 ++++++++-------------
1 file changed, 78 insertions(+), 129 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index da3d97c..680aa30 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -913,104 +913,27 @@ public class AsmClassGenerator extends ClassGenerator {
}
private void visitAttributeOrProperty(PropertyExpression expression, MethodCallerMultiAdapter adapter) {
- MethodVisitor mv = controller.getMethodVisitor();
-
- Expression objectExpression = expression.getObjectExpression();
ClassNode classNode = controller.getClassNode();
+ String propertyName = expression.getPropertyAsString();
+ Expression objectExpression = expression.getObjectExpression();
- //TODO (blackdrag): this if branch needs a rework. There should be no direct method calls be produced, the
- // handling of this/super could be much simplified (see visitAttributeExpression), the field accessibility check
- // could be moved directly into the search, which would also no longer require the GroovyBugError then
- // the outer class field access seems to be without any tests (if there are tests for that, then the code
- // here is dead code)
- if (isThisOrSuper(objectExpression)) {
- // let's use the field expression if it's available
- String name = expression.getPropertyAsString();
- if (name != null) {
- FieldNode field = null;
- boolean privateSuperField = false;
- if (isSuperExpression(objectExpression)) {
- field = classNode.getSuperClass().getDeclaredField(name);
- if (field != null && field.isPrivate()) {
- privateSuperField = true;
- }
- } else if (expression.isImplicitThis() || !controller.isInClosure()) {
- field = classNode.getDeclaredField(name);
- ClassNode outer = classNode.getOuterClass();
- if (field == null && outer != null) {
- FieldNode outerClassField;
- do {
- outerClassField = outer.getDeclaredField(name);
- if (outerClassField!=null && outerClassField.isStatic() && outerClassField.isFinal()) {
- if (outer != classNode.getOuterClass() && outerClassField.isPrivate()) {
- throw new GroovyBugError("Trying to access private constant field [" + outerClassField.getDeclaringClass() + "#" + outerClassField.getName() + "] from inner class");
- }
- PropertyExpression pexp = new PropertyExpression(
- new ClassExpression(outer),
- expression.getProperty()
- );
- pexp.getObjectExpression().setSourcePosition(objectExpression);
- pexp.visit(controller.getAcg());
- return;
- }
- outer = outer.getSuperClass();
- } while (outer != null);
- }
- if (field == null
- && expression instanceof AttributeExpression
- && isThisExpression(objectExpression)
- && controller.isStaticContext()) {
- // GROOVY-6183
- ClassNode current = classNode.getSuperClass();
- while (field == null && current != null) {
- field = current.getDeclaredField(name);
- current = current.getSuperClass();
- }
- if (field != null && (field.isProtected() || field.isPublic())) {
- visitFieldExpression(new FieldExpression(field));
- return;
- }
- }
- }
- if (field != null && !privateSuperField) { // GROOVY-4497: don't visit super field if it is private
- visitFieldExpression(new FieldExpression(field));
- return;
- }
- if (isSuperExpression(objectExpression)) {
- String prefix;
- if (controller.getCompileStack().isLHS()) {
- setPropertyOfSuperClass(classNode, expression, mv);
- return;
- } else {
- prefix = "get";
- }
- String propName = prefix + capitalize(name);
- visitMethodCallExpression(new MethodCallExpression(objectExpression, propName, MethodCallExpression.NO_ARGUMENTS));
- return;
- }
- }
- }
-
- String propName = expression.getPropertyAsString();
- // TODO: add support for super here too
- if (expression.getObjectExpression() instanceof ClassExpression && "this".equals(propName)) {
+ if (objectExpression instanceof ClassExpression && "this".equals(propertyName)) {
// we have something like A.B.this, and need to make it
// into this.this$0.this$0, where this.this$0 returns
// A.B and this.this$0.this$0 return A.
ClassNode type = objectExpression.getType();
- ClassNode iterType = classNode;
- if (controller.getCompileStack().isInSpecialConstructorCall() && classNode.getOuterClass() != null) {
- boolean staticInnerClass = classNode.isStaticClass();
+ if (controller.getCompileStack().isInSpecialConstructorCall() && type.equals(classNode.getOuterClass())) {
// Outer.this in a special constructor call
- if (classNode.getOuterClass().equals(type)) {
- ConstructorNode ctor = controller.getConstructorNode();
- Expression receiver = !staticInnerClass ? new VariableExpression(ctor.getParameters()[0]) : new ClassExpression(type);
- receiver.setSourcePosition(expression);
- receiver.visit(this);
- return;
- }
+ ConstructorNode ctor = controller.getConstructorNode();
+ Expression receiver = !classNode.isStaticClass() ? new VariableExpression(ctor.getParameters()[0]) : new ClassExpression(type);
+ receiver.setSourcePosition(expression);
+ receiver.visit(this);
+ return;
}
+
+ MethodVisitor mv = controller.getMethodVisitor();
mv.visitVarInsn(ALOAD, 0);
+ ClassNode iterType = classNode;
while (!iterType.equals(type)) {
String ownerName = BytecodeHelper.getClassInternalName(iterType);
if (iterType.getOuterClass() == null) break;
@@ -1018,7 +941,7 @@ public class AsmClassGenerator extends ClassGenerator {
iterType = iterType.getOuterClass();
if (thisField == null) {
// closure within inner class
- while (iterType.isDerivedFrom(ClassHelper.CLOSURE_TYPE)) {
+ while (iterType.isDerivedFrom(ClassHelper.CLOSURE_TYPE) && iterType.implementsInterface(ClassHelper.GENERATED_CLOSURE_Type)) {
// GROOVY-8881: cater for closures within closures - getThisObject is already outer class of all closures
iterType = iterType.getOuterClass();
}
@@ -1040,14 +963,14 @@ public class AsmClassGenerator extends ClassGenerator {
return;
}
- if (propName != null) {
+ if (propertyName != null) {
// TODO: spread safe should be handled inside
if (adapter == getProperty && !expression.isSpreadSafe()) {
- controller.getCallSiteWriter().makeGetPropertySite(objectExpression, propName, expression.isSafe(), expression.isImplicitThis());
+ controller.getCallSiteWriter().makeGetPropertySite(objectExpression, propertyName, expression.isSafe(), expression.isImplicitThis());
} else if (adapter == getGroovyObjectProperty && !expression.isSpreadSafe()) {
- controller.getCallSiteWriter().makeGroovyObjectGetPropertySite(objectExpression, propName, expression.isSafe(), expression.isImplicitThis());
+ controller.getCallSiteWriter().makeGroovyObjectGetPropertySite(objectExpression, propertyName, expression.isSafe(), expression.isImplicitThis());
} else {
- controller.getCallSiteWriter().fallbackAttributeOrPropertySite(expression, objectExpression, propName, adapter);
+ controller.getCallSiteWriter().fallbackAttributeOrPropertySite(expression, objectExpression, propertyName, adapter);
}
} else {
controller.getCallSiteWriter().fallbackAttributeOrPropertySite(expression, objectExpression, null, adapter);
@@ -1113,30 +1036,71 @@ public class AsmClassGenerator extends ClassGenerator {
return objectExpressionType.isDerivedFromGroovyObject();
}
- private boolean isThisOrSuperInStaticContext(Expression objectExpression) {
- return !controller.isInClosure() && controller.isStaticContext() && isThisOrSuper(objectExpression);
- }
-
@Override
public void visitPropertyExpression(PropertyExpression expression) {
Expression objectExpression = expression.getObjectExpression();
OperandStack operandStack = controller.getOperandStack();
int mark = operandStack.getStackLength() - 1;
- MethodCallerMultiAdapter adapter;
- if (controller.getCompileStack().isLHS()) {
- if (isGroovyObject(objectExpression) && !isThisOrSuperInStaticContext(objectExpression)) {
- adapter = setGroovyObjectProperty;
- } else {
- adapter = setProperty;
+ boolean visited = false;
+
+ if (isThisOrSuper(objectExpression)) {
+ String name = expression.getPropertyAsString();
+ if (name != null) {
+ FieldNode field = null;
+ boolean privateSuperField = false;
+ ClassNode classNode = controller.getClassNode();
+
+ if (isSuperExpression(objectExpression)) {
+ field = classNode.getSuperClass().getDeclaredField(name);
+ privateSuperField = (field != null && field.isPrivate());
+ } else if (expression.isImplicitThis() || !controller.isInClosure()) {
+ field = classNode.getDeclaredField(name);
+
+ ClassNode outer = classNode.getOuterClass();
+ if (field == null && outer != null) {
+ do {
+ FieldNode outerClassField = outer.getDeclaredField(name);
+ if (outerClassField != null && outerClassField.isStatic() && outerClassField.isFinal()) {
+ if (outerClassField.isPrivate() && classNode.getOuterClass() != outer) {
+ throw new GroovyBugError("Trying to access private field [" + outerClassField.getDeclaringClass() + "#" + outerClassField.getName() + "] from inner class");
+ }
+ PropertyExpression staticOuterField = new PropertyExpression(new ClassExpression(outer), expression.getProperty());
+ staticOuterField.getObjectExpression().setSourcePosition(objectExpression);
+ staticOuterField.visit(controller.getAcg());
+ return;
+ }
+ outer = outer.getSuperClass();
+ } while (outer != null);
+ }
+ }
+
+ if (field != null && !privateSuperField) { // GROOVY-4497: don't visit super field if it is private
+ visitFieldExpression(new FieldExpression(field));
+ visited = true;
+ } else if (isSuperExpression(objectExpression)) {
+ if (controller.getCompileStack().isLHS()) {
+ setPropertyOfSuperClass(classNode, expression, controller.getMethodVisitor());
+ } else {
+ visitMethodCallExpression(new MethodCallExpression(objectExpression, "get" + capitalize(name), MethodCallExpression.NO_ARGUMENTS));
+ }
+ visited = true;
+ }
}
- } else {
- if (isGroovyObject(objectExpression) && !isThisOrSuperInStaticContext(objectExpression)) {
- adapter = getGroovyObjectProperty;
+ }
+
+ if (!visited) {
+ boolean useMetaObjectProtocol = isGroovyObject(objectExpression)
+ && (!isThisOrSuper(objectExpression) || !controller.isStaticContext() || controller.isInClosure());
+
+ MethodCallerMultiAdapter adapter;
+ if (controller.getCompileStack().isLHS()) {
+ adapter = useMetaObjectProtocol ? setGroovyObjectProperty : setProperty;
} else {
- adapter = getProperty;
+ adapter = useMetaObjectProtocol ? getGroovyObjectProperty : getProperty;
}
+ visitAttributeOrProperty(expression, adapter);
}
- visitAttributeOrProperty(expression, adapter);
+
if (controller.getCompileStack().isLHS()) {
operandStack.remove(operandStack.getStackLength() - mark);
} else {
@@ -1151,18 +1115,13 @@ public class AsmClassGenerator extends ClassGenerator {
int mark = operandStack.getStackLength() - 1;
boolean visited = false;
- // TODO: checking for isThisOrSuper is enough for AttributeExpression, but if this is moved into
- // visitAttributeOrProperty to handle attributes and properties equally, then the extended check should be done
if (isThisOrSuper(objectExpression)) {
- // let's use the field expression if it's available
String name = expression.getPropertyAsString();
if (name != null) {
ClassNode classNode = controller.getClassNode();
FieldNode field = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
if (field != null) {
- FieldExpression fldExp = new FieldExpression(field);
- fldExp.setSourcePosition(expression.getProperty());
- visitFieldExpression(fldExp);
+ visitFieldExpression(new FieldExpression(field));
visited = true;
}
}
@@ -1172,12 +1131,12 @@ public class AsmClassGenerator extends ClassGenerator {
MethodCallerMultiAdapter adapter;
if (controller.getCompileStack().isLHS()) {
adapter = setField;
- if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
- if (usesSuper(expression)) adapter = setFieldOnSuper;
+ if (isSuperExpression(objectExpression)) adapter = setFieldOnSuper;
+ else if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
} else {
adapter = getField;
- if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
- if (usesSuper(expression)) adapter = getFieldOnSuper;
+ if (isSuperExpression(objectExpression)) adapter = getFieldOnSuper;
+ else if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
}
visitAttributeOrProperty(expression, adapter);
}
@@ -1189,16 +1148,6 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private static boolean usesSuper(PropertyExpression pe) {
- Expression objExp = pe.getObjectExpression();
- if (objExp instanceof VariableExpression) {
- VariableExpression varExp = (VariableExpression) objExp;
- String variable = varExp.getName();
- return variable.equals("super");
- }
- return false;
- }
-
@Override
public void visitFieldExpression(FieldExpression expression) {
FieldNode field = expression.getField();
[groovy] 02/20: remove dead code
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 91a65f05f397812f0c57aa81c2b227cd27dab316
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Nov 23 16:12:37 2019 -0600
remove dead code
- a ClassNode will never be equal to an Expression
(cherry picked from commit 1ed200297104752138b59decdad9e4315bfe4ff6)
---
.../classgen/asm/OptimizingStatementWriter.java | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
index 5c4b50c..94ae2ef 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
@@ -29,7 +29,6 @@ import org.codehaus.groovy.ast.VariableScope;
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.BitwiseNegationExpression;
-import org.codehaus.groovy.ast.expr.ClassExpression;
import org.codehaus.groovy.ast.expr.ClosureExpression;
import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
@@ -170,7 +169,7 @@ public class OptimizingStatementWriter extends StatementWriter {
if (fastPathData==null) {
// normal mode with different paths
// important is to not to have a fastpathblock here,
- // otherwise the per expression statement improvement
+ // otherwise the per expression statement improvement
// is impossible
super.writeBlockStatement(statement);
} else {
@@ -346,15 +345,15 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeExpressionStatement(statement);
} else {
StatementMeta meta = statement.getNodeMetaData(StatementMeta.class);
- // we have to have handle DelcarationExpressions special, since their
+ // we have to have handle DelcarationExpressions special, since their
// entry should be outside the optimization path, we have to do that of
- // course only if we are actually going to do two different paths,
+ // course only if we are actually going to do two different paths,
// otherwise it is not needed
//
// there are several cases to be considered now.
// (1) no fast path possible, so just do super
- // (2) fast path possible, and at path split point (meaning not in
- // fast path and not in slow path). Here we have to extract the
+ // (2) fast path possible, and at path split point (meaning not in
+ // fast path and not in slow path). Here we have to extract the
// Declaration and replace by an assignment
// (3) fast path possible and in slow or fastPath. Nothing to do here.
//
@@ -808,13 +807,7 @@ public class OptimizingStatementWriter extends StatementWriter {
if (expression.getNodeMetaData(StatementMeta.class) != null) return;
super.visitMethodCallExpression(expression);
- Expression objectExpression = expression.getObjectExpression();
- boolean setTarget = AsmClassGenerator.isThisExpression(objectExpression);
- if (!setTarget) {
- if (!(objectExpression instanceof ClassExpression)) return;
- setTarget = objectExpression.equals(node); // XXX: comparing Expression and ClassNode
- }
- if (setTarget) {
+ if (AsmClassGenerator.isThisExpression(expression.getObjectExpression())) {
setMethodTarget(expression, expression.getMethodAsString(), expression.getArguments(), true);
}
}
[groovy] 18/20: refactor dynamic variable handling
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 9dd7edac33c1274699089a5d67e1cf0f814d1e49
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Nov 30 10:55:24 2019 -0600
refactor dynamic variable handling
(cherry picked from commit d2e8e00acd014477fb15f4642c286f6ea05fbbf9)
---
.../groovy/classgen/VariableScopeVisitor.java | 37 +++++++++++-----------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index 531e5f6..be90e1e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -234,40 +234,39 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
private Variable checkVariableNameForDeclaration(final String name, final Expression expression) {
if ("super".equals(name) || "this".equals(name)) return null;
+ Variable variable = null;
VariableScope scope = currentScope;
- Variable var = new DynamicVariable(name, currentScope.isInStaticContext());
- Variable orig = var;
- // try to find a declaration of a variable
boolean crossingStaticContext = false;
+ // try to find a declaration of a variable
while (true) {
crossingStaticContext = (crossingStaticContext || scope.isInStaticContext());
- Variable var1 = scope.getDeclaredVariable(var.getName());
- if (var1 != null) {
- var = var1;
+ Variable var = scope.getDeclaredVariable(name);
+ if (var != null) {
+ variable = var;
break;
}
- var1 = scope.getReferencedLocalVariable(var.getName());
- if (var1 != null) {
- var = var1;
+ var = scope.getReferencedLocalVariable(name);
+ if (var != null) {
+ variable = var;
break;
}
- var1 = scope.getReferencedClassVariable(var.getName());
- if (var1 != null) {
- var = var1;
+ var = scope.getReferencedClassVariable(name);
+ if (var != null) {
+ variable = var;
break;
}
ClassNode classScope = scope.getClassScope();
if (classScope != null) {
- Variable member = findClassMember(classScope, var.getName());
+ Variable member = findClassMember(classScope, name);
if (member != null) {
boolean staticScope = (crossingStaticContext || isSpecialConstructorCall), staticMember = member.isInStaticContext();
// prevent a static context (e.g. a static method) from accessing a non-static variable (e.g. a non-static field)
if (!(staticScope && !staticMember)) {
- var = member;
+ variable = member;
}
}
// GROOVY-5961
@@ -275,22 +274,22 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
}
scope = scope.getParent();
}
- if (var == orig && crossingStaticContext) {
- var = new DynamicVariable(var.getName(), true);
+ if (variable == null) {
+ variable = new DynamicVariable(name, crossingStaticContext);
}
VariableScope end = scope;
scope = currentScope;
while (scope != end) {
if (end.isClassScope() || (end.isReferencedClassVariable(name) && end.getDeclaredVariable(name) == null)) {
- scope.putReferencedClassVariable(var);
+ scope.putReferencedClassVariable(variable);
} else {
- scope.putReferencedLocalVariable(var);
+ scope.putReferencedLocalVariable(variable);
}
scope = scope.getParent();
}
- return var;
+ return variable;
}
/**
[groovy] 20/20: fix for "in closure" tracking
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 4fb0893728a35ee7202960b52029e146234c76ab
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Nov 30 14:50:58 2019 -0600
fix for "in closure" tracking
- anon. inner class visited as closure expression
(cherry picked from commit 6f96c2b04eed0ae7051e7b8e18e22681d593d08a)
---
src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index 3efd775..76664b3 100644
--- a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -259,7 +259,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
}
private static boolean isAnonymous(final ClassNode node) {
- return (!node.isEnum() && node instanceof InnerClassNode && ((InnerClassNode) node).isAnonymous());
+ return (node instanceof InnerClassNode && ((InnerClassNode) node).isAnonymous() && !node.isEnum());
}
private void markClosureSharedVariables() {
@@ -435,8 +435,8 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
@Override
public void visitClosureExpression(final ClosureExpression expression) {
pushState();
- inClosure = true;
expression.setVariableScope(currentScope);
+ inClosure = !isAnonymous(currentScope.getParent().getClassScope());
if (expression.isParameterSpecified()) {
for (Parameter parameter : expression.getParameters()) {
[groovy] 08/20: GROOVY-8762: add test case
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit b8f8728af603f6d45e69ab0519f369adffd37646
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 25 14:51:01 2019 -0600
GROOVY-8762: add test case
(cherry picked from commit 683ff9ee6fee665cd4a6407b84188e5b9cc83799)
---
src/test/groovy/bugs/Groovy8762.groovy | 72 ++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/src/test/groovy/bugs/Groovy8762.groovy b/src/test/groovy/bugs/Groovy8762.groovy
new file mode 100644
index 0000000..cb8c390
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8762.groovy
@@ -0,0 +1,72 @@
+/*
+ * 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.bugs
+
+import groovy.test.NotYetImplemented
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+@CompileStatic
+final class Groovy8762 {
+
+ @Test @NotYetImplemented
+ void testExplicitThisObjectExpressionInInnerClassConstructor() {
+ assertScript '''
+ class Outer {
+ static class Inner extends Closure {
+ private int x, y
+
+ Inner(int i) {
+ super(null, null)
+ x = i
+ this.y = i // NPE at groovy.bugs.Outer$Inner.<init>
+ }
+
+ int getMaximumNumberOfParameters() {
+ throw new UnsupportedOperationException()
+ }
+
+ Class<?>[] getParameterTypes() {
+ throw new UnsupportedOperationException()
+ }
+
+ Object call(Object... args) {
+ throw new UnsupportedOperationException()
+ }
+
+ Object call(Object arguments) {
+ throw new UnsupportedOperationException()
+ }
+
+ Object call() {
+ throw new UnsupportedOperationException()
+ }
+ }
+
+ def makeCallable() {
+ new Inner(1)
+ }
+ }
+
+ assert new Outer().makeCallable()
+ '''
+ }
+}
[groovy] 01/20: organize members and other minor edits
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 90f44afb90ac5f5b03584bd5c9d738b87cfe416b
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Nov 23 11:55:04 2019 -0600
organize members and other minor edits
(cherry picked from commit b51fe06dd9aa5d41489106d7a3047dbda76bb68e)
---
.../classgen/asm/OptimizingStatementWriter.java | 527 +++++++++++----------
1 file changed, 273 insertions(+), 254 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
index fb07f94..5c4b50c 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
@@ -60,8 +60,10 @@ import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
+import java.util.Deque;
import java.util.LinkedList;
import java.util.List;
+import java.util.Optional;
import static org.codehaus.groovy.ast.ClassHelper.BigDecimal_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.GROOVY_INTERCEPTABLE_TYPE;
@@ -85,40 +87,8 @@ import static org.objectweb.asm.Opcodes.IFEQ;
import static org.objectweb.asm.Opcodes.IFNE;
import static org.objectweb.asm.Opcodes.INVOKEINTERFACE;
-/**
- * A class to write out the optimized statements
- */
public class OptimizingStatementWriter extends StatementWriter {
- private static class FastPathData {
- private Label pathStart = new Label();
- private Label afterPath = new Label();
- }
-
- public static class ClassNodeSkip{}
-
- public static class StatementMeta {
- private boolean optimize=false;
- protected MethodNode target;
- protected ClassNode type;
- protected VariableExpression declaredVariableExpression;
- protected boolean[] involvedTypes = new boolean[typeMapKeyNames.length];
- public void chainInvolvedTypes(OptimizeFlagsCollector opt) {
- for (int i=0; i<typeMapKeyNames.length; i++) {
- if (opt.current.involvedTypes[i]) {
- this.involvedTypes[i] = true;
- }
- }
- }
- public String toString() {
- StringBuilder ret = new StringBuilder("optimize=" + optimize + " target=" + target + " type=" + type + " involvedTypes=");
- for (int i=0; i<typeMapKeyNames.length; i++) {
- if (involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
- }
- return ret.toString();
- }
- }
-
private static final MethodCaller[] guards = {
null,
MethodCaller.newStatic(BytecodeInterface8.class, "isOrigInt"),
@@ -132,8 +102,8 @@ public class OptimizingStatementWriter extends StatementWriter {
};
private static final MethodCaller disabledStandardMetaClass = MethodCaller.newStatic(BytecodeInterface8.class, "disabledStandardMetaClass");
- private boolean fastPathBlocked = false;
private final WriterController controller;
+ private boolean fastPathBlocked;
public OptimizingStatementWriter(WriterController controller) {
super(controller);
@@ -475,27 +445,69 @@ public class OptimizingStatementWriter extends StatementWriter {
return meta;
}
+ //--------------------------------------------------------------------------
+
+ public static class ClassNodeSkip {
+ }
+
+ private static class FastPathData {
+ private Label pathStart = new Label();
+ private Label afterPath = new Label();
+ }
+
+ public static class StatementMeta {
+ private boolean optimize;
+ protected ClassNode type;
+ protected MethodNode target;
+ protected VariableExpression declaredVariableExpression;
+ protected boolean[] involvedTypes = new boolean[typeMapKeyNames.length];
+
+ public void chainInvolvedTypes(final OptimizeFlagsCollector opt) {
+ for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
+ if (opt.current.involvedTypes[i]) {
+ this.involvedTypes[i] = true;
+ }
+ }
+ }
+
+ @Override
+ public String toString() {
+ StringBuilder ret = new StringBuilder("optimize=" + optimize + " target=" + target + " type=" + type + " involvedTypes=");
+ for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
+ if (involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
+ }
+ return ret.toString();
+ }
+ }
+
private static class OptimizeFlagsCollector {
private static class OptimizeFlagsEntry {
- private boolean canOptimize = false;
- private boolean shouldOptimize = false;
+ private boolean canOptimize;
+ private boolean shouldOptimize;
private boolean[] involvedTypes = new boolean[typeMapKeyNames.length];
}
+
private OptimizeFlagsEntry current = new OptimizeFlagsEntry();
- private final LinkedList<OptimizeFlagsEntry> olderEntries = new LinkedList<OptimizeFlagsEntry>();
+ private final Deque<OptimizeFlagsEntry> previous = new LinkedList<>();
+
public void push() {
- olderEntries.addLast(current);
+ previous.addLast(current);
current = new OptimizeFlagsEntry();
}
- public void pop(boolean propagateFlags){
+
+ public void pop(final boolean propagateFlags) {
OptimizeFlagsEntry old = current;
- current = olderEntries.removeLast();
+ current = previous.removeLast();
if (propagateFlags) {
chainCanOptimize(old.canOptimize);
chainShouldOptimize(old.shouldOptimize);
- for (int i=0; i<typeMapKeyNames.length; i++) current.involvedTypes[i] |= old.involvedTypes[i];
+ for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
+ current.involvedTypes[i] |= old.involvedTypes[i];
+ }
}
}
+
+ @Override
public String toString() {
StringBuilder ret;
if (current.shouldOptimize) {
@@ -506,40 +518,46 @@ public class OptimizingStatementWriter extends StatementWriter {
ret = new StringBuilder("don't optimize");
}
ret.append(" involvedTypes =");
- for (int i=0; i<typeMapKeyNames.length; i++) {
+ for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
if (current.involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
}
return ret.toString();
}
+
/**
* @return true iff we should Optimize - this is almost seen as must
*/
private boolean shouldOptimize() {
return current.shouldOptimize;
}
+
/**
* @return true iff we can optimize, but not have to
*/
private boolean canOptimize() {
return current.canOptimize || current.shouldOptimize;
}
+
/**
* set "should" to true, if not already
*/
- public void chainShouldOptimize(boolean opt) {
+ public void chainShouldOptimize(final boolean opt) {
current.shouldOptimize = shouldOptimize() || opt;
}
+
/**
* set "can" to true, if not already
*/
- public void chainCanOptimize(boolean opt) {
+ public void chainCanOptimize(final boolean opt) {
current.canOptimize = current.canOptimize || opt;
}
- public void chainInvolvedType(ClassNode type) {
+
+ public void chainInvolvedType(final ClassNode type) {
Integer res = typeMap.get(type);
- if (res==null) return;
+ if (res == null) return;
current.involvedTypes[res] = true;
}
+
public void reset() {
current.canOptimize = false;
current.shouldOptimize = false;
@@ -548,125 +566,115 @@ public class OptimizingStatementWriter extends StatementWriter {
}
private static class OptVisitor extends ClassCodeVisitorSupport {
+ private static final VariableScope nonStaticScope = new VariableScope();
+ private final OptimizeFlagsCollector opt = new OptimizeFlagsCollector();
+ private boolean optimizeMethodCall = true;
private final TypeChooser typeChooser;
+ private VariableScope scope;
+ private ClassNode node;
- public OptVisitor(final TypeChooser chooser) {
+ OptVisitor(final TypeChooser chooser) {
this.typeChooser = chooser;
}
- @Override protected SourceUnit getSourceUnit() {return null;}
-
- private ClassNode node;
- private OptimizeFlagsCollector opt = new OptimizeFlagsCollector();
- private boolean optimizeMethodCall = true;
- private VariableScope scope;
- private static final VariableScope nonStaticScope = new VariableScope();
+ @Override
+ protected SourceUnit getSourceUnit() {
+ return null;
+ }
@Override
- public void visitClass(ClassNode node) {
+ public void visitClass(final ClassNode node) {
this.optimizeMethodCall = !node.implementsInterface(GROOVY_INTERCEPTABLE_TYPE);
this.node = node;
this.scope = nonStaticScope;
super.visitClass(node);
- this.scope=null;
- this.node=null;
- }
-
- @Override
- public void visitMethod(MethodNode node) {
- scope = node.getVariableScope();
- super.visitMethod(node);
- opt.reset();
+ this.scope = null;
+ this.node = null;
}
@Override
- public void visitConstructor(ConstructorNode node) {
+ public void visitConstructor(final ConstructorNode node) {
scope = node.getVariableScope();
super.visitConstructor(node);
}
@Override
- public void visitReturnStatement(ReturnStatement statement) {
- opt.push();
- super.visitReturnStatement(statement);
- if (opt.shouldOptimize()) addMeta(statement,opt);
- opt.pop(opt.shouldOptimize());
+ public void visitMethod(final MethodNode node) {
+ scope = node.getVariableScope();
+ super.visitMethod(node);
+ opt.reset();
}
- @Override
- public void visitUnaryMinusExpression(UnaryMinusExpression expression) {
- //TODO: implement int operations for this
- super.visitUnaryMinusExpression(expression);
- StatementMeta meta = addMeta(expression);
- meta.type = OBJECT_TYPE;
- }
+ // statements:
@Override
- public void visitUnaryPlusExpression(UnaryPlusExpression expression) {
- //TODO: implement int operations for this
- super.visitUnaryPlusExpression(expression);
- StatementMeta meta = addMeta(expression);
- meta.type = OBJECT_TYPE;
+ public void visitBlockStatement(final BlockStatement statement) {
+ opt.push();
+ boolean optAll = true;
+ for (Statement stmt : statement.getStatements()) {
+ opt.push();
+ stmt.visit(this);
+ optAll = optAll && opt.canOptimize();
+ opt.pop(true);
+ }
+ if (statement.isEmpty()) {
+ opt.chainCanOptimize(true);
+ opt.pop(true);
+ } else {
+ opt.chainShouldOptimize(optAll);
+ if (optAll) {
+ addMeta(statement, opt);
+ }
+ opt.pop(optAll);
+ }
}
@Override
- public void visitBitwiseNegationExpression(BitwiseNegationExpression expression) {
- //TODO: implement int operations for this
- super.visitBitwiseNegationExpression(expression);
- StatementMeta meta = addMeta(expression);
- meta.type = OBJECT_TYPE;
- }
-
- private void addTypeInformation(Expression expression, Expression orig) {
- ClassNode type = typeChooser.resolveType(expression, node);
- if (isPrimitiveType(type)) {
- StatementMeta meta = addMeta(orig);
- meta.type = type;
- opt.chainShouldOptimize(true);
- opt.chainInvolvedType(type);
+ public void visitExpressionStatement(final ExpressionStatement statement) {
+ if (statement.getNodeMetaData(StatementMeta.class) != null) return;
+ opt.push();
+ super.visitExpressionStatement(statement);
+ if (opt.shouldOptimize()) {
+ addMeta(statement, opt);
}
+ opt.pop(opt.shouldOptimize());
}
@Override
- public void visitPrefixExpression(PrefixExpression expression) {
- super.visitPrefixExpression(expression);
- addTypeInformation(expression.getExpression(),expression);
+ public void visitForLoop(final ForStatement statement) {
+ opt.push();
+ super.visitForLoop(statement);
+ if (opt.shouldOptimize()) {
+ addMeta(statement, opt);
+ }
+ opt.pop(opt.shouldOptimize());
}
@Override
- public void visitPostfixExpression(PostfixExpression expression) {
- super.visitPostfixExpression(expression);
- addTypeInformation(expression.getExpression(),expression);
+ public void visitIfElse(final IfStatement statement) {
+ opt.push();
+ super.visitIfElse(statement);
+ if (opt.shouldOptimize()) {
+ addMeta(statement, opt);
+ }
+ opt.pop(opt.shouldOptimize());
}
@Override
- public void visitDeclarationExpression(DeclarationExpression expression) {
- Expression right = expression.getRightExpression();
- right.visit(this);
-
- ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
- Expression rightExpression = expression.getRightExpression();
- ClassNode rightType = optimizeDivWithIntOrLongTarget(rightExpression, leftType);
- if (rightType==null) rightType = typeChooser.resolveType(expression.getRightExpression(), node);
- if (isPrimitiveType(leftType) && isPrimitiveType(rightType)) {
- // if right is a constant, then we optimize only if it makes
- // a block complete, so we set a maybe
- if (right instanceof ConstantExpression) {
- opt.chainCanOptimize(true);
- } else {
- opt.chainShouldOptimize(true);
- }
- StatementMeta meta = addMeta(expression);
- ClassNode declarationType = typeChooser.resolveType(expression, node);
- meta.type = declarationType!=null?declarationType:leftType;
- opt.chainInvolvedType(leftType);
- opt.chainInvolvedType(rightType);
+ public void visitReturnStatement(final ReturnStatement statement) {
+ opt.push();
+ super.visitReturnStatement(statement);
+ if (opt.shouldOptimize()) {
+ addMeta(statement,opt);
}
+ opt.pop(opt.shouldOptimize());
}
+ // expressions:
+
@Override
- public void visitBinaryExpression(BinaryExpression expression) {
- if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
+ public void visitBinaryExpression(final BinaryExpression expression) {
+ if (expression.getNodeMetaData(StatementMeta.class) != null) return;
super.visitBinaryExpression(expression);
ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
@@ -674,7 +682,7 @@ public class OptimizingStatementWriter extends StatementWriter {
ClassNode resultType = null;
int operation = expression.getOperation().getType();
- if (operation==Types.LEFT_SQUARE_BRACKET && leftType.isArray()) {
+ if (operation == Types.LEFT_SQUARE_BRACKET && leftType.isArray()) {
opt.chainShouldOptimize(true);
resultType = leftType.getComponentType();
} else {
@@ -696,8 +704,10 @@ public class OptimizingStatementWriter extends StatementWriter {
}
resultType = boolean_TYPE;
break;
- case Types.LOGICAL_AND: case Types.LOGICAL_AND_EQUAL:
- case Types.LOGICAL_OR: case Types.LOGICAL_OR_EQUAL:
+ case Types.LOGICAL_AND:
+ case Types.LOGICAL_AND_EQUAL:
+ case Types.LOGICAL_OR:
+ case Types.LOGICAL_OR_EQUAL:
if (boolean_TYPE.equals(leftType) && boolean_TYPE.equals(rightType)) {
opt.chainShouldOptimize(true);
} else {
@@ -706,7 +716,8 @@ public class OptimizingStatementWriter extends StatementWriter {
expression.setType(boolean_TYPE);
resultType = boolean_TYPE;
break;
- case Types.DIVIDE: case Types.DIVIDE_EQUAL:
+ case Types.DIVIDE:
+ case Types.DIVIDE_EQUAL:
if (isLongCategory(leftType) && isLongCategory(rightType)) {
resultType = BigDecimal_TYPE;
opt.chainShouldOptimize(true);
@@ -718,8 +729,9 @@ public class OptimizingStatementWriter extends StatementWriter {
opt.chainShouldOptimize(true);
}
break;
- case Types.POWER: case Types.POWER_EQUAL:
- //TODO: implement
+ case Types.POWER:
+ case Types.POWER_EQUAL:
+ // TODO: implement
break;
case Types.ASSIGN:
resultType = optimizeDivWithIntOrLongTarget(expression.getRightExpression(), leftType);
@@ -742,166 +754,189 @@ public class OptimizingStatementWriter extends StatementWriter {
}
}
- if (resultType!=null) {
- StatementMeta meta = addMeta(expression);
- meta.type = resultType;
+ if (resultType != null) {
+ addMeta(expression).type = resultType;
opt.chainInvolvedType(resultType);
opt.chainInvolvedType(leftType);
opt.chainInvolvedType(rightType);
}
}
- /**
- * method to optimize Z = X/Y with Z being int or long style
- * @returns null if the optimization cannot be applied, otherwise it
- * will return the new target type
- */
- private ClassNode optimizeDivWithIntOrLongTarget(Expression rhs, ClassNode assignmentTartgetType) {
- if (!(rhs instanceof BinaryExpression)) return null;
- BinaryExpression binExp = (BinaryExpression) rhs;
- int op = binExp.getOperation().getType();
- if (op!=Types.DIVIDE && op!=Types.DIVIDE_EQUAL) return null;
-
- ClassNode originalResultType = typeChooser.resolveType(binExp, node);
- if ( !originalResultType.equals(BigDecimal_TYPE) ||
- !(isLongCategory(assignmentTartgetType) || isFloatingCategory(assignmentTartgetType))
- ) {
- return null;
- }
+ @Override
+ public void visitBitwiseNegationExpression(final BitwiseNegationExpression expression) {
+ // TODO: implement int operations for this
+ super.visitBitwiseNegationExpression(expression);
+ addMeta(expression).type = OBJECT_TYPE;
+ }
- ClassNode leftType = typeChooser.resolveType(binExp.getLeftExpression(), node);
- if (!isLongCategory(leftType)) return null;
- ClassNode rightType = typeChooser.resolveType(binExp.getRightExpression(), node);
- if (!isLongCategory(rightType)) return null;
+ @Override
+ public void visitClosureExpression(final ClosureExpression expression) {
+ }
- ClassNode target;
- if (isIntCategory(leftType) && isIntCategory(rightType)) {
- target = int_TYPE;
- } else if (isLongCategory(leftType) && isLongCategory(rightType)) {
- target = long_TYPE;
- } else if (isDoubleCategory(leftType) && isDoubleCategory(rightType)) {
- target = double_TYPE;
- } else {
- return null;
- }
- StatementMeta meta = addMeta(rhs);
- meta.type = target;
- opt.chainInvolvedType(target);
- return target;
+ @Override
+ public void visitConstructorCallExpression(final ConstructorCallExpression expression) {
+ if (expression.getNodeMetaData(StatementMeta.class) != null) return;
+ super.visitConstructorCallExpression(expression);
+ // we cannot set a target for the constructor call, since we cannot easily check the meta class of the other class
+ //setMethodTarget(call, "<init>", call.getArguments(), false);
}
@Override
- public void visitExpressionStatement(ExpressionStatement statement) {
- if (statement.getNodeMetaData(StatementMeta.class)!=null) return;
- opt.push();
- super.visitExpressionStatement(statement);
- if (opt.shouldOptimize()) addMeta(statement,opt);
- opt.pop(opt.shouldOptimize());
+ public void visitDeclarationExpression(final DeclarationExpression expression) {
+ Expression right = expression.getRightExpression();
+ right.visit(this);
+
+ ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
+ Expression rightExpression = expression.getRightExpression();
+ ClassNode rightType = optimizeDivWithIntOrLongTarget(rightExpression, leftType);
+ if (rightType == null) rightType = typeChooser.resolveType(expression.getRightExpression(), node);
+ if (isPrimitiveType(leftType) && isPrimitiveType(rightType)) {
+ // if right is a constant, then we optimize only if it makes a block complete, so we set a maybe
+ if (right instanceof ConstantExpression) {
+ opt.chainCanOptimize(true);
+ } else {
+ opt.chainShouldOptimize(true);
+ }
+ addMeta(expression).type = Optional.ofNullable(typeChooser.resolveType(expression, node)).orElse(leftType);
+ opt.chainInvolvedType(leftType);
+ opt.chainInvolvedType(rightType);
+ }
}
@Override
- public void visitBlockStatement(BlockStatement block) {
- opt.push();
- boolean optAll = true;
- for (Statement statement : block.getStatements()) {
- opt.push();
- statement.visit(this);
- optAll = optAll && opt.canOptimize();
- opt.pop(true);
+ public void visitMethodCallExpression(final MethodCallExpression expression) {
+ if (expression.getNodeMetaData(StatementMeta.class) != null) return;
+ super.visitMethodCallExpression(expression);
+
+ Expression objectExpression = expression.getObjectExpression();
+ boolean setTarget = AsmClassGenerator.isThisExpression(objectExpression);
+ if (!setTarget) {
+ if (!(objectExpression instanceof ClassExpression)) return;
+ setTarget = objectExpression.equals(node); // XXX: comparing Expression and ClassNode
}
- if (block.isEmpty()) {
- opt.chainCanOptimize(true);
- opt.pop(true);
- } else {
- opt.chainShouldOptimize(optAll);
- if (optAll) addMeta(block,opt);
- opt.pop(optAll);
+ if (setTarget) {
+ setMethodTarget(expression, expression.getMethodAsString(), expression.getArguments(), true);
}
}
@Override
- public void visitIfElse(IfStatement statement) {
- opt.push();
- super.visitIfElse(statement);
- if (opt.shouldOptimize()) addMeta(statement,opt);
- opt.pop(opt.shouldOptimize());
+ public void visitPostfixExpression(final PostfixExpression expression) {
+ super.visitPostfixExpression(expression);
+ addTypeInformation(expression.getExpression(), expression);
}
- /*@Override
- public void visitConstantExpression(ConstantExpression expression) {
- super.visitConstantExpression(expression);
- opt.chainShouldOptimize(true);
- }*/
+ @Override
+ public void visitPrefixExpression(final PrefixExpression expression) {
+ super.visitPrefixExpression(expression);
+ addTypeInformation(expression.getExpression(), expression);
+ }
@Override
- public void visitStaticMethodCallExpression(StaticMethodCallExpression expression) {
- if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
+ public void visitStaticMethodCallExpression(final StaticMethodCallExpression expression) {
+ if (expression.getNodeMetaData(StatementMeta.class) != null) return;
super.visitStaticMethodCallExpression(expression);
+ setMethodTarget(expression, expression.getMethod(), expression.getArguments(), true);
+ }
- setMethodTarget(expression,expression.getMethod(), expression.getArguments(), true);
+ @Override
+ public void visitUnaryMinusExpression(final UnaryMinusExpression expression) {
+ // TODO: implement int operations for this
+ super.visitUnaryMinusExpression(expression);
+ addMeta(expression).type = OBJECT_TYPE;
}
@Override
- public void visitMethodCallExpression(MethodCallExpression expression) {
- if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
- super.visitMethodCallExpression(expression);
+ public void visitUnaryPlusExpression(final UnaryPlusExpression expression) {
+ // TODO: implement int operations for this
+ super.visitUnaryPlusExpression(expression);
+ addMeta(expression).type = OBJECT_TYPE;
+ }
- Expression object = expression.getObjectExpression();
- boolean setTarget = AsmClassGenerator.isThisExpression(object);
- if (!setTarget) {
- if (!(object instanceof ClassExpression)) return;
- setTarget = object.equals(node);
- }
+ //
- if (!setTarget) return;
- setMethodTarget(expression, expression.getMethodAsString(), expression.getArguments(), true);
+ private void addTypeInformation(final Expression expression, final Expression orig) {
+ ClassNode type = typeChooser.resolveType(expression, node);
+ if (isPrimitiveType(type)) {
+ addMeta(orig).type = type;
+ opt.chainShouldOptimize(true);
+ opt.chainInvolvedType(type);
+ }
}
- @Override
- public void visitConstructorCallExpression(ConstructorCallExpression call) {
- if (call.getNodeMetaData(StatementMeta.class)!=null) return;
- super.visitConstructorCallExpression(call);
+ /**
+ * Optimizes "Z = X/Y" with Z being int or long style.
+ *
+ * @returns null if the optimization cannot be applied, otherwise it will return the new target type
+ */
+ private ClassNode optimizeDivWithIntOrLongTarget(final Expression rhs, final ClassNode assignmentTartgetType) {
+ if (!(rhs instanceof BinaryExpression)) return null;
+ BinaryExpression binExp = (BinaryExpression) rhs;
+ int op = binExp.getOperation().getType();
+ if (op != Types.DIVIDE && op != Types.DIVIDE_EQUAL) return null;
- // we cannot a target for the constructor call, since we cannot easily
- // check the meta class of the other class
- // setMethodTarget(call, "<init>", call.getArguments(), false);
+ ClassNode originalResultType = typeChooser.resolveType(binExp, node);
+ if (!originalResultType.equals(BigDecimal_TYPE)
+ || !(isLongCategory(assignmentTartgetType)
+ || isFloatingCategory(assignmentTartgetType))) {
+ return null;
+ }
+
+ ClassNode leftType = typeChooser.resolveType(binExp.getLeftExpression(), node);
+ if (!isLongCategory(leftType)) return null;
+ ClassNode rightType = typeChooser.resolveType(binExp.getRightExpression(), node);
+ if (!isLongCategory(rightType)) return null;
+
+ ClassNode target;
+ if (isIntCategory(leftType) && isIntCategory(rightType)) {
+ target = int_TYPE;
+ } else if (isLongCategory(leftType) && isLongCategory(rightType)) {
+ target = long_TYPE;
+ } else if (isDoubleCategory(leftType) && isDoubleCategory(rightType)) {
+ target = double_TYPE;
+ } else {
+ return null;
+ }
+ addMeta(rhs).type = target;
+ opt.chainInvolvedType(target);
+ return target;
}
- private void setMethodTarget(Expression expression, String name, Expression callArgs, boolean isMethod) {
- if (name==null) return;
+ private void setMethodTarget(final Expression expression, final String name, final Expression callArgs, final boolean isMethod) {
+ if (name == null) return;
if (!optimizeMethodCall) return;
if (AsmClassGenerator.containsSpreadExpression(callArgs)) return;
+
// find method call target
Parameter[] paraTypes = null;
if (callArgs instanceof ArgumentListExpression) {
ArgumentListExpression args = (ArgumentListExpression) callArgs;
int size = args.getExpressions().size();
paraTypes = new Parameter[size];
- int i=0;
- for (Expression exp: args.getExpressions()) {
+ int i = 0;
+ for (Expression exp : args.getExpressions()) {
ClassNode type = typeChooser.resolveType(exp, node);
if (!validTypeForCall(type)) return;
- paraTypes[i] = new Parameter(type,"");
- i++;
+ paraTypes[i] = new Parameter(type, "");
+ i += 1;
}
} else {
ClassNode type = typeChooser.resolveType(callArgs, node);
if (!validTypeForCall(type)) return;
- paraTypes = new Parameter[]{new Parameter(type,"")};
+ paraTypes = new Parameter[]{new Parameter(type, "")};
}
MethodNode target;
ClassNode type;
if (isMethod) {
target = node.getMethod(name, paraTypes);
- if (target==null) return;
+ if (target == null) return;
if (!target.getDeclaringClass().equals(node)) return;
if (scope.isInStaticContext() && !target.isStatic()) return;
type = target.getReturnType().redirect();
} else {
type = expression.getType();
target = selectConstructor(type, paraTypes);
- if (target==null) return;
+ if (target == null) return;
}
StatementMeta meta = addMeta(expression);
@@ -910,37 +945,21 @@ public class OptimizingStatementWriter extends StatementWriter {
opt.chainShouldOptimize(true);
}
- private static MethodNode selectConstructor(ClassNode node, Parameter[] paraTypes) {
- List<ConstructorNode> cl = node.getDeclaredConstructors();
- MethodNode res = null;
- for (ConstructorNode cn : cl) {
- if (ParameterUtils.parametersEqual(cn.getParameters(), paraTypes)) {
- res = cn;
+ private static MethodNode selectConstructor(final ClassNode node, final Parameter[] parameters) {
+ List<ConstructorNode> ctors = node.getDeclaredConstructors();
+ MethodNode result = null;
+ for (ConstructorNode ctor : ctors) {
+ if (ParameterUtils.parametersEqual(ctor.getParameters(), parameters)) {
+ result = ctor;
break;
}
}
- if (res !=null && res.isPublic()) return res;
- return null;
+ return (result != null && result.isPublic() ? result : null);
}
- private static boolean validTypeForCall(ClassNode type) {
+ private static boolean validTypeForCall(final ClassNode type) {
// do call only for final classes and primitive types
- if (isPrimitiveType(type)) return true;
- return (type.getModifiers() & ACC_FINAL) > 0;
- }
-
- @Override
- public void visitClosureExpression(ClosureExpression expression) {
- }
-
- @Override
- public void visitForLoop(ForStatement statement) {
- opt.push();
- super.visitForLoop(statement);
- if (opt.shouldOptimize()) addMeta(statement,opt);
- opt.pop(opt.shouldOptimize());
+ return isPrimitiveType(type) || (type.getModifiers() & ACC_FINAL) > 0;
}
}
-
-
}
[groovy] 07/20: GROOVY-8648: fix ASM error for this or super
attribute expression on LHS
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 285e82e948e1baabed273a0432b5a0540913bad9
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 25 11:55:01 2019 -0600
GROOVY-8648: fix ASM error for this or super attribute expression on LHS
java.lang.NegativeArraySizeException
at groovyjarjarasm.asm.Frame.merge(Frame.java:1222)
at groovyjarjarasm.asm.MethodWriter.computeAllFrames(MethodWriter.java:1610)
at groovyjarjarasm.asm.MethodWriter.visitMaxs(MethodWriter.java:1546)
at org.codehaus.groovy.classgen.AsmClassGenerator.visitConstructorOrMethod(AsmClassGenerator.java:410)
(cherry picked from commit 06da9561f0a274ce71c7a1b669841bc70c61766d)
---
.../groovy/classgen/AsmClassGenerator.java | 36 ++++++++--------
src/test/groovy/bugs/Groovy8648.groovy | 49 ++++++++++++++++++++++
2 files changed, 68 insertions(+), 17 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 2c688f8..3e50687 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -1147,36 +1147,41 @@ public class AsmClassGenerator extends ClassGenerator {
@Override
public void visitAttributeExpression(AttributeExpression expression) {
Expression objectExpression = expression.getObjectExpression();
- ClassNode classNode = controller.getClassNode();
+ OperandStack operandStack = controller.getOperandStack();
+ int mark = operandStack.getStackLength() - 1;
+ boolean visited = false;
+
// TODO: checking for isThisOrSuper is enough for AttributeExpression, but if this is moved into
// visitAttributeOrProperty to handle attributes and properties equally, then the extended check should be done
if (isThisOrSuper(objectExpression)) {
// let's use the field expression if it's available
String name = expression.getPropertyAsString();
if (name != null) {
+ ClassNode classNode = controller.getClassNode();
FieldNode field = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
if (field != null) {
FieldExpression fldExp = new FieldExpression(field);
fldExp.setSourcePosition(expression.getProperty());
visitFieldExpression(fldExp);
- return;
+ visited = true;
}
}
}
- MethodCallerMultiAdapter adapter;
- OperandStack operandStack = controller.getOperandStack();
- int mark = operandStack.getStackLength() - 1;
- if (controller.getCompileStack().isLHS()) {
- adapter = setField;
- if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
- if (usesSuper(expression)) adapter = setFieldOnSuper;
- } else {
- adapter = getField;
- if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
- if (usesSuper(expression)) adapter = getFieldOnSuper;
+ if (!visited) {
+ MethodCallerMultiAdapter adapter;
+ if (controller.getCompileStack().isLHS()) {
+ adapter = setField;
+ if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
+ if (usesSuper(expression)) adapter = setFieldOnSuper;
+ } else {
+ adapter = getField;
+ if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
+ if (usesSuper(expression)) adapter = getFieldOnSuper;
+ }
+ visitAttributeOrProperty(expression, adapter);
}
- visitAttributeOrProperty(expression, adapter);
+
if (controller.getCompileStack().isLHS()) {
operandStack.remove(operandStack.getStackLength() - mark);
} else {
@@ -1210,9 +1215,6 @@ public class AsmClassGenerator extends ClassGenerator {
loadInstanceField(expression);
}
}
- if (!controller.getCompileStack().isLHS()) {
- controller.getAssertionWriter().record(expression);
- }
}
public void loadStaticField(FieldExpression fldExp) {
diff --git a/src/test/groovy/bugs/Groovy8648.groovy b/src/test/groovy/bugs/Groovy8648.groovy
new file mode 100644
index 0000000..2687c2a
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8648.groovy
@@ -0,0 +1,49 @@
+/*
+ * 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.bugs
+
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+@CompileStatic
+final class Groovy8648 {
+
+ @Test
+ void testDirectFieldAccessOnLHS() {
+ assertScript '''
+ class Account {
+ private int balance = 0
+ int getBalance() {
+ return balance
+ }
+ void deposit(int amount) {
+ assert amount > 0
+ this.@balance += amount // ASM error for LHS attribute expression
+ }
+ }
+
+ new Account().with {
+ deposit(42)
+ assert balance == 42
+ }
+ '''
+ }
+}
[groovy] 19/20: track "in closure" and other minor edits
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit e15269a1f56cc3b8374a6d746fc4e7868de69bcb
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Nov 30 13:55:13 2019 -0600
track "in closure" and other minor edits
(cherry picked from commit 0971d34d7d4ff6b954d0e5c66e69952e3c9bea75)
---
.../groovy/classgen/VariableScopeVisitor.java | 125 +++++++++++----------
1 file changed, 65 insertions(+), 60 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index be90e1e..3efd775 100644
--- a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -53,6 +53,7 @@ import org.codehaus.groovy.syntax.Types;
import java.util.Deque;
import java.util.Iterator;
import java.util.LinkedList;
+import java.util.function.BiConsumer;
import static java.lang.reflect.Modifier.isFinal;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
@@ -64,7 +65,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
private ClassNode currentClass;
private VariableScope currentScope;
- private boolean inConstructor, isSpecialConstructorCall;
+ private boolean inClosure, inConstructor, inSpecialConstructorCall;
private final SourceUnit source;
private final boolean recurseInnerClasses;
@@ -73,11 +74,12 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
private static class StateStackElement {
final ClassNode clazz;
final VariableScope scope;
- final boolean inConstructor;
+ final boolean inClosure, inConstructor;
- StateStackElement(final ClassNode currentClass, final VariableScope currentScope, final boolean inConstructor) {
+ StateStackElement(final ClassNode currentClass, final VariableScope currentScope, final boolean inClosure, final boolean inConstructor) {
clazz = currentClass;
scope = currentScope;
+ this.inClosure = inClosure;
this.inConstructor = inConstructor;
}
}
@@ -97,12 +99,12 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
return source;
}
- // ------------------------------
+ //----------------------------------
// helper methods
- //------------------------------
+ //----------------------------------
private void pushState(final boolean isStatic) {
- stateStack.push(new StateStackElement(currentClass, currentScope, inConstructor));
+ stateStack.push(new StateStackElement(currentClass, currentScope, inClosure, inConstructor));
currentScope = new VariableScope(currentScope);
currentScope.setInStaticContext(isStatic);
}
@@ -113,9 +115,10 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
private void popState() {
StateStackElement state = stateStack.pop();
- currentClass = state.clazz;
- currentScope = state.scope;
- inConstructor = state.inConstructor;
+ this.currentClass = state.clazz;
+ this.currentScope = state.scope;
+ this.inClosure = state.inClosure;
+ this.inConstructor = state.inConstructor;
}
private void declare(final VariableExpression variable) {
@@ -124,14 +127,14 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
variable.setAccessedVariable(variable);
}
- private void declare(final Variable variable, final ASTNode expression) {
+ private void declare(final Variable variable, final ASTNode context) {
String scopeType = "scope";
String variableType = "variable";
- if (expression.getClass() == FieldNode.class) {
+ if (context.getClass() == FieldNode.class) {
scopeType = "class";
variableType = "field";
- } else if (expression.getClass() == PropertyNode.class) {
+ } else if (context.getClass() == PropertyNode.class) {
scopeType = "class";
variableType = "property";
}
@@ -142,7 +145,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
msg.append(" of the name ").append(variable.getName());
if (currentScope.getDeclaredVariable(variable.getName()) != null) {
- addError(msg.toString(), expression);
+ addError(msg.toString(), context);
return;
}
@@ -154,7 +157,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
if (scope.getDeclaredVariable(variable.getName()) != null) {
// variable already declared
- addError(msg.toString(), expression);
+ addError(msg.toString(), context);
break;
}
}
@@ -194,44 +197,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
return findClassMember(cn.getOuterClass(), name);
}
- private static boolean isAnonymous(final ClassNode node) {
- return (!node.isEnum() && node instanceof InnerClassNode && ((InnerClassNode) node).isAnonymous());
- }
-
- private void markClosureSharedVariables() {
- for (Iterator<Variable> it = currentScope.getReferencedLocalVariablesIterator(); it.hasNext(); ) {
- Variable variable = it.next();
- variable.setClosureSharedVariable(true);
- }
- }
-
- // -------------------------------
- // different Variable-based checks
- // -------------------------------
-
- private void checkFinalFieldAccess(final Expression expression) {
- if (expression instanceof TupleExpression) {
- TupleExpression tuple = (TupleExpression) expression;
- for (Expression tupleExpression : tuple.getExpressions()) {
- checkForFinal(expression, (VariableExpression) tupleExpression);
- }
- } else if (expression instanceof VariableExpression) {
- checkForFinal(expression, (VariableExpression) expression);
- }
- // currently not looking for PropertyExpression: dealt with at runtime using ReadOnlyPropertyException
- }
-
- private void checkForFinal(final Expression context, final VariableExpression expression) {
- Variable variable = expression.getAccessedVariable();
- if (variable != null) {
- if (isFinal(variable.getModifiers()) && variable instanceof Parameter) {
- addError("Cannot assign a value to final variable '" + variable.getName() + "'", context);
- }
- // TODO: handle local variables
- }
- }
-
- private Variable checkVariableNameForDeclaration(final String name, final Expression expression) {
+ private Variable findVariableDeclaration(final String name) {
if ("super".equals(name) || "this".equals(name)) return null;
Variable variable = null;
@@ -263,7 +229,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
if (classScope != null) {
Variable member = findClassMember(classScope, name);
if (member != null) {
- boolean staticScope = (crossingStaticContext || isSpecialConstructorCall), staticMember = member.isInStaticContext();
+ boolean staticScope = (crossingStaticContext || inSpecialConstructorCall), staticMember = member.isInStaticContext();
// prevent a static context (e.g. a static method) from accessing a non-static variable (e.g. a non-static field)
if (!(staticScope && !staticMember)) {
variable = member;
@@ -292,6 +258,43 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
return variable;
}
+ private static boolean isAnonymous(final ClassNode node) {
+ return (!node.isEnum() && node instanceof InnerClassNode && ((InnerClassNode) node).isAnonymous());
+ }
+
+ private void markClosureSharedVariables() {
+ for (Iterator<Variable> it = currentScope.getReferencedLocalVariablesIterator(); it.hasNext(); ) {
+ Variable variable = it.next();
+ variable.setClosureSharedVariable(true);
+ }
+ }
+
+ //----------------------------------
+ // variable checks
+ //----------------------------------
+
+ private void checkFinalFieldAccess(final Expression expression) {
+ BiConsumer<VariableExpression, ASTNode> checkForFinal = (expr, node) -> {
+ Variable variable = expr.getAccessedVariable();
+ if (variable != null) {
+ if (isFinal(variable.getModifiers()) && variable instanceof Parameter) {
+ addError("Cannot assign a value to final variable '" + variable.getName() + "'", node);
+ }
+ // TODO: handle local variables
+ }
+ };
+
+ if (expression instanceof VariableExpression) {
+ checkForFinal.accept((VariableExpression) expression, expression);
+ } else if (expression instanceof TupleExpression) {
+ TupleExpression tuple = (TupleExpression) expression;
+ for (Expression tupleExpression : tuple.getExpressions()) {
+ checkForFinal.accept((VariableExpression) tupleExpression, expression);
+ }
+ }
+ // currently not looking for PropertyExpression: dealt with at runtime using ReadOnlyPropertyException
+ }
+
/**
* A property on "this", like this.x is transformed to a direct field access,
* so we need to check the static context here.
@@ -326,6 +329,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
if (isAnonymous(node)) return;
pushState();
+ inClosure = false;
currentClass = node;
currentScope.setClassScope(node);
@@ -431,7 +435,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
@Override
public void visitClosureExpression(final ClosureExpression expression) {
pushState();
-
+ inClosure = true;
expression.setVariableScope(currentScope);
if (expression.isParameterSpecified()) {
@@ -456,9 +460,11 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
@Override
public void visitConstructorCallExpression(final ConstructorCallExpression expression) {
- isSpecialConstructorCall = expression.isSpecialCall();
+ boolean oldInSpecialCtorFlag = inSpecialConstructorCall;
+ inSpecialConstructorCall |= expression.isSpecialCall();
super.visitConstructorCallExpression(expression);
- isSpecialConstructorCall = false;
+ inSpecialConstructorCall = oldInSpecialCtorFlag;
+
if (!expression.isUsingAnonymousInnerClass()) return;
pushState();
@@ -516,7 +522,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
public void visitFieldExpression(final FieldExpression expression) {
String name = expression.getFieldName();
//TODO: change that to get the correct scope
- Variable variable = checkVariableNameForDeclaration(name, expression);
+ Variable variable = findVariableDeclaration(name);
checkVariableContextAccess(variable, expression);
}
@@ -530,7 +536,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
throw new GroovyBugError("method name is null");
}
- Variable variable = checkVariableNameForDeclaration(methodName, expression);
+ Variable variable = findVariableDeclaration(methodName);
if (variable != null && !(variable instanceof DynamicVariable)) {
checkVariableContextAccess(variable, expression);
}
@@ -557,8 +563,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
@Override
public void visitVariableExpression(final VariableExpression expression) {
- String name = expression.getName();
- Variable variable = checkVariableNameForDeclaration(name, expression);
+ Variable variable = findVariableDeclaration(expression.getName());
if (variable == null) return;
expression.setAccessedVariable(variable);
checkVariableContextAccess(variable, expression);
[groovy] 16/20: GROOVY-8361: add test case
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 2731e714b8827440063a8182d3df297351ab194b
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Nov 27 09:23:41 2019 -0600
GROOVY-8361: add test case
(cherry picked from commit 73eb7200cd3cb64f4ac4c829bc56daab30aa9b79)
---
src/test/groovy/bugs/Groovy8361.groovy | 115 +++++++++++++++++++++++++++++++++
1 file changed, 115 insertions(+)
diff --git a/src/test/groovy/bugs/Groovy8361.groovy b/src/test/groovy/bugs/Groovy8361.groovy
new file mode 100644
index 0000000..c3bee1d
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8361.groovy
@@ -0,0 +1,115 @@
+/*
+ * 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.bugs
+
+import org.codehaus.groovy.control.CompilationUnit
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.shouldFail
+
+final class Groovy8361 {
+
+ @Test
+ void testAliasedImportNotUsed1() {
+ def config = new CompilerConfiguration(
+ targetDirectory: File.createTempDir(),
+ jointCompilationOptions: [memStub: true]
+ )
+
+ def parentDir = File.createTempDir()
+ new File(parentDir, 'p').mkdir()
+ new File(parentDir, 'q').mkdir()
+ try {
+ def a = new File(parentDir, 'p/A.java')
+ a.write '''
+ package p;
+
+ public class A {
+ public static class AA {
+ }
+ }
+ '''
+ def b = new File(parentDir, 'q/B.groovy')
+ b.write '''
+ package q
+
+ import p.A.AA as AAA
+
+ println new AA() // should be unresolved
+ '''
+
+ def loader = new GroovyClassLoader(this.class.classLoader)
+ def cu = new JavaAwareCompilationUnit(config, loader)
+ cu.addSources(a, b)
+
+ def err = shouldFail {
+ cu.compile()
+ }
+ assert err =~ /unable to resolve class AA/
+ } finally {
+ parentDir.deleteDir()
+ config.targetDirectory.deleteDir()
+ }
+ }
+
+ @Test
+ void testAliasedImportNotUsed2() {
+ def config = new CompilerConfiguration(
+ targetDirectory: File.createTempDir(),
+ jointCompilationOptions: [memStub: true]
+ )
+
+ def parentDir = File.createTempDir()
+ new File(parentDir, 'p').mkdir()
+ new File(parentDir, 'q').mkdir()
+ try {
+ def a = new File(parentDir, 'p/A.groovy')
+ a.write '''
+ package p
+
+ class A {
+ static class AA {
+ }
+ }
+ '''
+ def b = new File(parentDir, 'q/B.groovy')
+ b.write '''
+ package q
+
+ import p.A.AA as AAA
+
+ println new AA() // should be unresolved
+ '''
+
+ def loader = new GroovyClassLoader(this.class.classLoader)
+ def cu = new CompilationUnit(config, null, loader)
+ cu.addSources(a, b)
+
+ def err = shouldFail {
+ cu.compile()
+ }
+ assert err =~ /unable to resolve class AA/
+ } finally {
+ parentDir.deleteDir()
+ config.targetDirectory.deleteDir()
+ }
+ }
+}
[groovy] 13/20: make parameters final and remove dead path in
visitAttributeExpression
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 4dfdb1a1b3ee5448b8a47dd995601d3ade1c3387
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Nov 26 13:33:09 2019 -0600
make parameters final and remove dead path in visitAttributeExpression
(cherry picked from commit c9adec3438cdd14021362885538d336a3ddea738)
---
.../groovy/classgen/AsmClassGenerator.java | 297 ++++++++++-----------
1 file changed, 142 insertions(+), 155 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 825a278..717c735 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -131,33 +131,32 @@ import static org.apache.groovy.util.BeanUtils.capitalize;
*/
public class AsmClassGenerator extends ClassGenerator {
- // fields and properties
- public static final MethodCallerMultiAdapter setField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setField", false, false);
- public static final MethodCallerMultiAdapter getField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getField", false, false);
- public static final MethodCallerMultiAdapter setGroovyObjectField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setGroovyObjectField", false, false);
- public static final MethodCallerMultiAdapter getGroovyObjectField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getGroovyObjectField", false, false);
- static final MethodCallerMultiAdapter setFieldOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setFieldOnSuper", false, false);
- static final MethodCallerMultiAdapter getFieldOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getFieldOnSuper", false, false);
-
- public static final MethodCallerMultiAdapter setProperty = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setProperty", false, false);
- static final MethodCallerMultiAdapter getProperty = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getProperty", false, false);
- static final MethodCallerMultiAdapter setGroovyObjectProperty = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setGroovyObjectProperty", false, false);
- static final MethodCallerMultiAdapter getGroovyObjectProperty = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getGroovyObjectProperty", false, false);
- static final MethodCallerMultiAdapter setPropertyOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setPropertyOnSuper", false, false);
- static final MethodCallerMultiAdapter getPropertyOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getPropertyOnSuper", false, false);
-
- // spread expressions
- static final MethodCaller spreadMap = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "spreadMap");
- static final MethodCaller despreadList = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "despreadList");
+ // fields
+ public static final MethodCallerMultiAdapter setField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setField", false, false);
+ public static final MethodCallerMultiAdapter getField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getField", false, false);
+ //private static final MethodCallerMultiAdapter setFieldOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setFieldOnSuper", false, false);
+ //private static final MethodCallerMultiAdapter getFieldOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getFieldOnSuper", false, false);
+ public static final MethodCallerMultiAdapter setGroovyObjectField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setGroovyObjectField", false, false);
+ public static final MethodCallerMultiAdapter getGroovyObjectField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getGroovyObjectField", false, false);
+
+ // properties
+ public static final MethodCallerMultiAdapter setProperty = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setProperty", false, false);
+ private static final MethodCallerMultiAdapter getProperty = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getProperty", false, false);
+ //private static final MethodCallerMultiAdapter setPropertyOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setPropertyOnSuper", false, false);
+ //private static final MethodCallerMultiAdapter getPropertyOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getPropertyOnSuper", false, false);
+ private static final MethodCallerMultiAdapter setGroovyObjectProperty = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setGroovyObjectProperty", false, false);
+ private static final MethodCallerMultiAdapter getGroovyObjectProperty = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getGroovyObjectProperty", false, false);
+
+ // spread expressions
+ private static final MethodCaller spreadMap = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "spreadMap");
+ private static final MethodCaller despreadList = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "despreadList");
// type conversions
- static final MethodCaller createListMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createList");
- static final MethodCaller createMapMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createMap");
- static final MethodCaller createRangeMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createRange");
-
- // wrapper creation methods
- static final MethodCaller createPojoWrapperMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createPojoWrapper");
- static final MethodCaller createGroovyObjectWrapperMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createGroovyObjectWrapper");
+ private static final MethodCaller createMapMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createMap");
+ private static final MethodCaller createListMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createList");
+ private static final MethodCaller createRangeMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createRange");
+ private static final MethodCaller createPojoWrapperMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createPojoWrapper");
+ private static final MethodCaller createGroovyObjectWrapperMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "createGroovyObjectWrapper");
private final Map<String, GenericsType> genericParameterNames = new HashMap<>();
private final Map<String,ClassNode> referencedClasses = new HashMap<>();
@@ -176,7 +175,7 @@ public class AsmClassGenerator extends ClassGenerator {
private ClassVisitor classVisitor;
private final String sourceFile;
- public AsmClassGenerator(SourceUnit source, GeneratorContext context, ClassVisitor classVisitor, String sourceFile) {
+ public AsmClassGenerator(final SourceUnit source, final GeneratorContext context, final ClassVisitor classVisitor, final String sourceFile) {
this.source = source;
this.context = context;
this.classVisitor = classVisitor;
@@ -196,7 +195,7 @@ public class AsmClassGenerator extends ClassGenerator {
//--------------------------------------------------------------------------
@Override
- public void visitClass(ClassNode classNode) {
+ public void visitClass(final ClassNode classNode) {
referencedClasses.clear();
WriterControllerFactory factory = classNode.getNodeMetaData(WriterControllerFactory.class);
WriterController normalController = new WriterController();
@@ -293,7 +292,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void makeInnerClassEntry(ClassNode cn) {
+ private void makeInnerClassEntry(final ClassNode cn) {
if (!(cn instanceof InnerClassNode)) return;
InnerClassNode innerClass = (InnerClassNode) cn;
String innerClassName = innerClass.getName();
@@ -318,14 +317,14 @@ public class AsmClassGenerator extends ClassGenerator {
* for what flags are allowed depending on the fact we are writing the inner class table
* or the class itself
*/
- private static int adjustedClassModifiersForInnerClassTable(ClassNode classNode) {
+ private static int adjustedClassModifiersForInnerClassTable(final ClassNode classNode) {
int modifiers = classNode.getModifiers();
modifiers = modifiers & ~ACC_SUPER;
modifiers = fixInterfaceModifiers(classNode, modifiers);
return modifiers;
}
- private static int fixInterfaceModifiers(ClassNode classNode, int modifiers) {
+ private static int fixInterfaceModifiers(final ClassNode classNode, int modifiers) {
// (JLS §9.1.1.1). Such a class file must not have its ACC_FINAL, ACC_SUPER or ACC_ENUM flags set.
if (classNode.isInterface()) {
modifiers = modifiers & ~ACC_ENUM;
@@ -334,7 +333,7 @@ public class AsmClassGenerator extends ClassGenerator {
return modifiers;
}
- private static int fixInnerClassModifiers(ClassNode classNode, int modifiers) {
+ private static int fixInnerClassModifiers(final ClassNode classNode, int modifiers) {
// on the inner class node itself, private/protected are not allowed
if (classNode.getOuterClass() != null) {
if ((modifiers & ACC_PRIVATE) != 0) {
@@ -355,7 +354,7 @@ public class AsmClassGenerator extends ClassGenerator {
* for what flags are allowed depending on the fact we are writing the inner class table
* or the class itself
*/
- private static int adjustedClassModifiersForClassWriting(ClassNode classNode) {
+ private static int adjustedClassModifiersForClassWriting(final ClassNode classNode) {
int modifiers = classNode.getModifiers();
boolean needsSuper = !classNode.isInterface();
modifiers = needsSuper ? modifiers | ACC_SUPER : modifiers & ~ACC_SUPER;
@@ -366,13 +365,13 @@ public class AsmClassGenerator extends ClassGenerator {
return modifiers;
}
- public void visitGenericType(GenericsType genericsType) {
+ public void visitGenericType(final GenericsType genericsType) {
ClassNode type = genericsType.getType();
genericParameterNames.put(type.getName(), genericsType);
}
@Override
- protected void visitConstructorOrMethod(MethodNode node, boolean isConstructor) {
+ protected void visitConstructorOrMethod(final MethodNode node, final boolean isConstructor) {
controller.resetLineNumber();
Parameter[] parameters = node.getParameters();
String methodType = BytecodeHelper.getMethodDescriptor(node.getReturnType(), parameters);
@@ -432,7 +431,7 @@ public class AsmClassGenerator extends ClassGenerator {
mv.visitEnd();
}
- private void visitStdMethod(MethodNode node, boolean isConstructor, Parameter[] parameters, Statement code) {
+ private void visitStdMethod(final MethodNode node, final boolean isConstructor, final Parameter[] parameters, final Statement code) {
controller.getCompileStack().init(node.getVariableScope(), parameters);
controller.getCallSiteWriter().makeSiteEntry();
@@ -487,7 +486,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void visitAnnotationDefaultExpression(AnnotationVisitor av, ClassNode type, Expression exp) {
+ private void visitAnnotationDefaultExpression(final AnnotationVisitor av, final ClassNode type, final Expression exp) {
if (exp instanceof ClosureExpression) {
ClassNode closureClass = controller.getClosureWriter().getOrAddClosureClass((ClosureExpression) exp, ACC_PUBLIC);
Type t = Type.getType(BytecodeHelper.getTypeDescription(closureClass));
@@ -527,7 +526,7 @@ public class AsmClassGenerator extends ClassGenerator {
av.visitEnd();
}
- private void visitAnnotationDefault(MethodNode node, MethodVisitor mv) {
+ private void visitAnnotationDefault(final MethodNode node, final MethodVisitor mv) {
if (!node.hasAnnotationDefault()) return;
Expression exp = ((ReturnStatement) node.getCode()).getExpression();
AnnotationVisitor av = mv.visitAnnotationDefault();
@@ -535,19 +534,19 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitConstructor(ConstructorNode node) {
+ public void visitConstructor(final ConstructorNode node) {
controller.setConstructorNode(node);
super.visitConstructor(node);
}
@Override
- public void visitMethod(MethodNode node) {
+ public void visitMethod(final MethodNode node) {
controller.setMethodNode(node);
super.visitMethod(node);
}
@Override
- public void visitField(FieldNode fieldNode) {
+ public void visitField(final FieldNode fieldNode) {
onLineNumber(fieldNode, "visitField: " + fieldNode.getName());
ClassNode t = fieldNode.getType();
String signature = BytecodeHelper.getGenericsBounds(t);
@@ -579,7 +578,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitProperty(PropertyNode statement) {
+ public void visitProperty(final PropertyNode statement) {
// the verifier created the field and the setter/getter methods, so here is
// not really something to do
onLineNumber(statement, "visitProperty:" + statement.getField().getName());
@@ -593,86 +592,86 @@ public class AsmClassGenerator extends ClassGenerator {
//-------------------------------------------------------------------------
@Override
- protected void visitStatement(Statement statement) {
+ protected void visitStatement(final Statement statement) {
throw new GroovyBugError("visitStatement should not be visited here.");
}
@Override
- public void visitCatchStatement(CatchStatement statement) {
+ public void visitCatchStatement(final CatchStatement statement) {
statement.getCode().visit(this);
}
@Override
- public void visitBlockStatement(BlockStatement statement) {
+ public void visitBlockStatement(final BlockStatement statement) {
controller.getStatementWriter().writeBlockStatement(statement);
}
@Override
- public void visitForLoop(ForStatement statement) {
+ public void visitForLoop(final ForStatement statement) {
controller.getStatementWriter().writeForStatement(statement);
}
@Override
- public void visitWhileLoop( WhileStatement statement) {
+ public void visitWhileLoop(final WhileStatement statement) {
controller.getStatementWriter().writeWhileLoop(statement);
}
@Override
- public void visitDoWhileLoop(DoWhileStatement statement) {
+ public void visitDoWhileLoop(final DoWhileStatement statement) {
controller.getStatementWriter().writeDoWhileLoop(statement);
}
@Override
- public void visitIfElse(IfStatement statement) {
+ public void visitIfElse(final IfStatement statement) {
controller.getStatementWriter().writeIfElse(statement);
}
@Override
- public void visitAssertStatement(AssertStatement statement) {
+ public void visitAssertStatement(final AssertStatement statement) {
controller.getStatementWriter().writeAssert(statement);
}
@Override
- public void visitTryCatchFinally(TryCatchStatement statement) {
+ public void visitTryCatchFinally(final TryCatchStatement statement) {
controller.getStatementWriter().writeTryCatchFinally(statement);
}
@Override
- public void visitSwitch(SwitchStatement statement) {
+ public void visitSwitch(final SwitchStatement statement) {
controller.getStatementWriter().writeSwitch(statement);
}
@Override
- public void visitCaseStatement(CaseStatement statement) {
+ public void visitCaseStatement(final CaseStatement statement) {
}
@Override
- public void visitBreakStatement(BreakStatement statement) {
+ public void visitBreakStatement(final BreakStatement statement) {
controller.getStatementWriter().writeBreak(statement);
}
@Override
- public void visitContinueStatement(ContinueStatement statement) {
+ public void visitContinueStatement(final ContinueStatement statement) {
controller.getStatementWriter().writeContinue(statement);
}
@Override
- public void visitSynchronizedStatement(SynchronizedStatement statement) {
+ public void visitSynchronizedStatement(final SynchronizedStatement statement) {
controller.getStatementWriter().writeSynchronized(statement);
}
@Override
- public void visitThrowStatement(ThrowStatement statement) {
+ public void visitThrowStatement(final ThrowStatement statement) {
controller.getStatementWriter().writeThrow(statement);
}
@Override
- public void visitReturnStatement(ReturnStatement statement) {
+ public void visitReturnStatement(final ReturnStatement statement) {
controller.getStatementWriter().writeReturn(statement);
}
@Override
- public void visitExpressionStatement(ExpressionStatement statement) {
+ public void visitExpressionStatement(final ExpressionStatement statement) {
controller.getStatementWriter().writeExpressionStatement(statement);
}
@@ -680,43 +679,43 @@ public class AsmClassGenerator extends ClassGenerator {
//-------------------------------------------------------------------------
@Override
- public void visitTernaryExpression(TernaryExpression expression) {
+ public void visitTernaryExpression(final TernaryExpression expression) {
onLineNumber(expression, "visitTernaryExpression");
controller.getBinaryExpressionHelper().evaluateTernary(expression);
}
@Override
- public void visitDeclarationExpression(DeclarationExpression expression) {
+ public void visitDeclarationExpression(final DeclarationExpression expression) {
onLineNumber(expression, "visitDeclarationExpression: \"" + expression.getText() + "\"");
controller.getBinaryExpressionHelper().evaluateEqual(expression,true);
}
@Override
- public void visitBinaryExpression(BinaryExpression expression) {
+ public void visitBinaryExpression(final BinaryExpression expression) {
onLineNumber(expression, "visitBinaryExpression: \"" + expression.getOperation().getText() + "\" ");
controller.getBinaryExpressionHelper().eval(expression);
controller.getAssertionWriter().record(expression.getOperation());
}
@Override
- public void visitPostfixExpression(PostfixExpression expression) {
+ public void visitPostfixExpression(final PostfixExpression expression) {
controller.getBinaryExpressionHelper().evaluatePostfixMethod(expression);
controller.getAssertionWriter().record(expression);
}
@Override
- public void visitPrefixExpression(PrefixExpression expression) {
+ public void visitPrefixExpression(final PrefixExpression expression) {
controller.getBinaryExpressionHelper().evaluatePrefixMethod(expression);
controller.getAssertionWriter().record(expression);
}
@Override
- public void visitClosureExpression(ClosureExpression expression) {
+ public void visitClosureExpression(final ClosureExpression expression) {
controller.getClosureWriter().writeClosure(expression);
}
@Override
- public void visitLambdaExpression(LambdaExpression expression) {
+ public void visitLambdaExpression(final LambdaExpression expression) {
controller.getLambdaWriter().writeLambda(expression);
}
@@ -737,7 +736,7 @@ public class AsmClassGenerator extends ClassGenerator {
* @see <a href="http://java.sun.com/docs/books/vmspec/2nd-edition/html/ClassFile.doc.html#14152">Class field types</a>
*/
@Override
- public void visitConstantExpression(ConstantExpression expression) {
+ public void visitConstantExpression(final ConstantExpression expression) {
final String constantName = expression.getConstantName();
if (controller.isStaticConstructor() || constantName == null) {
controller.getOperandStack().pushConstant(expression);
@@ -748,12 +747,12 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitSpreadExpression(SpreadExpression expression) {
+ public void visitSpreadExpression(final SpreadExpression expression) {
throw new GroovyBugError("SpreadExpression should not be visited here");
}
@Override
- public void visitSpreadMapExpression(SpreadMapExpression expression) {
+ public void visitSpreadMapExpression(final SpreadMapExpression expression) {
Expression subExpression = expression.getExpression();
// to not record the underlying MapExpression twice,
// we disable the assertion tracker
@@ -767,32 +766,32 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitMethodPointerExpression(MethodPointerExpression expression) {
+ public void visitMethodPointerExpression(final MethodPointerExpression expression) {
controller.getMethodPointerExpressionWriter().writeMethodPointerExpression(expression);
}
@Override
- public void visitMethodReferenceExpression(MethodReferenceExpression expression) {
+ public void visitMethodReferenceExpression(final MethodReferenceExpression expression) {
controller.getMethodReferenceExpressionWriter().writeMethodReferenceExpression(expression);
}
@Override
- public void visitUnaryMinusExpression(UnaryMinusExpression expression) {
+ public void visitUnaryMinusExpression(final UnaryMinusExpression expression) {
controller.getUnaryExpressionHelper().writeUnaryMinus(expression);
}
@Override
- public void visitUnaryPlusExpression(UnaryPlusExpression expression) {
+ public void visitUnaryPlusExpression(final UnaryPlusExpression expression) {
controller.getUnaryExpressionHelper().writeUnaryPlus(expression);
}
@Override
- public void visitBitwiseNegationExpression(BitwiseNegationExpression expression) {
+ public void visitBitwiseNegationExpression(final BitwiseNegationExpression expression) {
controller.getUnaryExpressionHelper().writeBitwiseNegate(expression);
}
@Override
- public void visitCastExpression(CastExpression castExpression) {
+ public void visitCastExpression(final CastExpression castExpression) {
ClassNode type = castExpression.getType();
Expression subExpression = castExpression.getExpression();
subExpression.visit(this);
@@ -816,12 +815,12 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitNotExpression(NotExpression expression) {
+ public void visitNotExpression(final NotExpression expression) {
controller.getUnaryExpressionHelper().writeNotExpression(expression);
}
@Override
- public void visitBooleanExpression(BooleanExpression expression) {
+ public void visitBooleanExpression(final BooleanExpression expression) {
controller.getCompileStack().pushBooleanExpression();
int mark = controller.getOperandStack().getStackLength();
Expression inner = expression.getExpression();
@@ -831,21 +830,21 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitMethodCallExpression(MethodCallExpression call) {
+ public void visitMethodCallExpression(final MethodCallExpression call) {
onLineNumber(call, "visitMethodCallExpression: \"" + call.getMethod() + "\":");
controller.getInvocationWriter().writeInvokeMethod(call);
controller.getAssertionWriter().record(call.getMethod());
}
@Override
- public void visitStaticMethodCallExpression(StaticMethodCallExpression call) {
+ public void visitStaticMethodCallExpression(final StaticMethodCallExpression call) {
onLineNumber(call, "visitStaticMethodCallExpression: \"" + call.getMethod() + "\":");
controller.getInvocationWriter().writeInvokeStaticMethod(call);
controller.getAssertionWriter().record(call);
}
@Override
- public void visitConstructorCallExpression(ConstructorCallExpression call) {
+ public void visitConstructorCallExpression(final ConstructorCallExpression call) {
onLineNumber(call, "visitConstructorCallExpression: \"" + call.getType().getName() + "\":");
if (call.isSpecialCall()) {
@@ -856,7 +855,7 @@ public class AsmClassGenerator extends ClassGenerator {
controller.getAssertionWriter().record(call);
}
- private static String makeFieldClassName(ClassNode type) {
+ private static String makeFieldClassName(final ClassNode type) {
String internalName = BytecodeHelper.getClassInternalName(type);
StringBuilder ret = new StringBuilder(internalName.length());
for (int i = 0, n = internalName.length(); i < n; i += 1) {
@@ -872,7 +871,7 @@ public class AsmClassGenerator extends ClassGenerator {
return ret.toString();
}
- private static String getStaticFieldName(ClassNode type) {
+ private static String getStaticFieldName(final ClassNode type) {
ClassNode componentType = type;
StringBuilder prefix = new StringBuilder();
for (; componentType.isArray(); componentType = componentType.getComponentType()) {
@@ -883,12 +882,12 @@ public class AsmClassGenerator extends ClassGenerator {
return name;
}
- private static boolean isValidFieldNodeForByteCodeAccess(FieldNode fn, ClassNode accessingNode) {
+ private static boolean isValidFieldNodeForByteCodeAccess(final FieldNode fn, final ClassNode accessingNode) {
if (fn == null) return false;
ClassNode declaringClass = fn.getDeclaringClass();
// same class is always allowed access
if (fn.isPublic() || declaringClass.equals(accessingNode)) return true;
- boolean samePackages = samePackages(declaringClass.getPackageName(), accessingNode.getPackageName());
+ boolean samePackages = Objects.equals(declaringClass.getPackageName(), accessingNode.getPackageName());
// protected means same class or same package, or subclass
if (fn.isProtected() && (samePackages || accessingNode.isDerivedFrom(declaringClass))) {
return true;
@@ -900,7 +899,7 @@ public class AsmClassGenerator extends ClassGenerator {
return false;
}
- public static FieldNode getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(ClassNode accessingNode, ClassNode current, String name, boolean skipCurrent) {
+ public static FieldNode getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(final ClassNode accessingNode, final ClassNode current, final String name, final boolean skipCurrent) {
if (!skipCurrent) {
FieldNode currentClassField = current.getDeclaredField(name);
if (isValidFieldNodeForByteCodeAccess(currentClassField, accessingNode)) return currentClassField;
@@ -912,7 +911,7 @@ public class AsmClassGenerator extends ClassGenerator {
return null;
}
- private void visitAttributeOrProperty(PropertyExpression expression, MethodCallerMultiAdapter adapter) {
+ private void visitAttributeOrProperty(final PropertyExpression expression, final MethodCallerMultiAdapter adapter) {
ClassNode classNode = controller.getClassNode();
String propertyName = expression.getPropertyAsString();
Expression objectExpression = expression.getObjectExpression();
@@ -977,7 +976,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void setPropertyOfSuperClass(ClassNode classNode, PropertyExpression expression, MethodVisitor mv) {
+ private void setPropertyOfSuperClass(final ClassNode classNode, final PropertyExpression expression, final MethodVisitor mv) {
String fieldName = expression.getPropertyAsString();
FieldNode fieldNode = classNode.getSuperClass().getField(fieldName);
@@ -1013,21 +1012,21 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private static boolean getterAndSetterExists(MethodNode setter, MethodNode getter) {
- return null != setter && null != getter && setter.getDeclaringClass().equals(getter.getDeclaringClass());
+ private static boolean getterAndSetterExists(final MethodNode setter, final MethodNode getter) {
+ return setter != null && getter != null && setter.getDeclaringClass().equals(getter.getDeclaringClass());
}
- private static MethodNode findSetterOfSuperClass(ClassNode classNode, FieldNode fieldNode) {
+ private static MethodNode findSetterOfSuperClass(final ClassNode classNode, final FieldNode fieldNode) {
String setterMethodName = "set" + capitalize(fieldNode.getName());
return classNode.getSuperClass().getSetterMethod(setterMethodName);
}
- private static MethodNode findGetterOfSuperClass(ClassNode classNode, FieldNode fieldNode) {
+ private static MethodNode findGetterOfSuperClass(final ClassNode classNode, final FieldNode fieldNode) {
String getterMethodName = "get" + capitalize(fieldNode.getName());
return classNode.getSuperClass().getGetterMethod(getterMethodName);
}
- private boolean isGroovyObject(Expression objectExpression) {
+ private boolean isGroovyObject(final Expression objectExpression) {
if (isThisExpression(objectExpression)) return true;
if (objectExpression instanceof ClassExpression) return false;
@@ -1037,7 +1036,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitPropertyExpression(PropertyExpression expression) {
+ public void visitPropertyExpression(final PropertyExpression expression) {
Expression objectExpression = expression.getObjectExpression();
OperandStack operandStack = controller.getOperandStack();
int mark = operandStack.getStackLength() - 1;
@@ -1109,7 +1108,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitAttributeExpression(AttributeExpression expression) {
+ public void visitAttributeExpression(final AttributeExpression expression) {
Expression objectExpression = expression.getObjectExpression();
OperandStack operandStack = controller.getOperandStack();
int mark = operandStack.getStackLength() - 1;
@@ -1137,13 +1136,9 @@ public class AsmClassGenerator extends ClassGenerator {
if (!visited) {
MethodCallerMultiAdapter adapter;
if (controller.getCompileStack().isLHS()) {
- adapter = setField;
- if (isSuperExpression(objectExpression)) adapter = setFieldOnSuper;
- else if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
+ adapter = isGroovyObject(objectExpression) ? setGroovyObjectField : setField;
} else {
- adapter = getField;
- if (isSuperExpression(objectExpression)) adapter = getFieldOnSuper;
- else if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
+ adapter = isGroovyObject(objectExpression) ? getGroovyObjectField : getField;
}
visitAttributeOrProperty(expression, adapter);
}
@@ -1156,7 +1151,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitFieldExpression(FieldExpression expression) {
+ public void visitFieldExpression(final FieldExpression expression) {
FieldNode field = expression.getField();
if (field.isStatic()) {
if (controller.getCompileStack().isLHS()) {
@@ -1173,7 +1168,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- public void loadStaticField(FieldExpression fldExp) {
+ public void loadStaticField(final FieldExpression fldExp) {
MethodVisitor mv = controller.getMethodVisitor();
FieldNode field = fldExp.getField();
boolean holder = field.isHolder() && !controller.isInClosureConstructor();
@@ -1195,7 +1190,7 @@ public class AsmClassGenerator extends ClassGenerator {
/**
* RHS instance field. should move most of the code in the BytecodeHelper
*/
- public void loadInstanceField(FieldExpression fldExp) {
+ public void loadInstanceField(final FieldExpression fldExp) {
MethodVisitor mv = controller.getMethodVisitor();
FieldNode field = fldExp.getField();
boolean holder = field.isHolder() && !controller.isInClosureConstructor();
@@ -1215,7 +1210,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void storeThisInstanceField(FieldExpression expression) {
+ private void storeThisInstanceField(final FieldExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
FieldNode field = expression.getField();
@@ -1248,7 +1243,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void storeStaticField(FieldExpression expression) {
+ private void storeStaticField(final FieldExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
FieldNode field = expression.getField();
@@ -1268,11 +1263,8 @@ public class AsmClassGenerator extends ClassGenerator {
controller.getOperandStack().remove(1);
}
- /**
- * Visits a bare (unqualified) variable expression.
- */
@Override
- public void visitVariableExpression(VariableExpression expression) {
+ public void visitVariableExpression(final VariableExpression expression) {
String variableName = expression.getName();
//-----------------------------------------------------------------------
@@ -1312,7 +1304,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void loadThis(VariableExpression thisExpression) {
+ private void loadThis(final VariableExpression thisExpression) {
MethodVisitor mv = controller.getMethodVisitor();
mv.visitVarInsn(ALOAD, 0);
if (controller.isInClosure() && !controller.getCompileStack().isImplicitThis()) {
@@ -1329,7 +1321,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void processClassVariable(VariableExpression expression) {
+ private void processClassVariable(final VariableExpression expression) {
if (passingParams && controller.isInScriptBody()) {
//TODO: check if this part is actually used
MethodVisitor mv = controller.getMethodVisitor();
@@ -1438,7 +1430,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitClassExpression(ClassExpression expression) {
+ public void visitClassExpression(final ClassExpression expression) {
ClassNode type = expression.getType();
MethodVisitor mv = controller.getMethodVisitor();
if (BytecodeHelper.isClassLiteralPossible(type) || BytecodeHelper.isSameCompilationUnit(controller.getClassNode(), type)) {
@@ -1469,7 +1461,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitRangeExpression(RangeExpression expression) {
+ public void visitRangeExpression(final RangeExpression expression) {
OperandStack operandStack = controller.getOperandStack();
expression.getFrom().visit(this);
operandStack.box();
@@ -1482,12 +1474,12 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitMapEntryExpression(MapEntryExpression expression) {
+ public void visitMapEntryExpression(final MapEntryExpression expression) {
throw new GroovyBugError("MapEntryExpression should not be visited here");
}
@Override
- public void visitMapExpression(MapExpression expression) {
+ public void visitMapExpression(final MapExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
List<MapEntryExpression> entries = expression.getMapEntryExpressions();
@@ -1519,7 +1511,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitArgumentlistExpression(ArgumentListExpression ale) {
+ public void visitArgumentlistExpression(final ArgumentListExpression ale) {
if (containsSpreadExpression(ale)) {
despreadList(ale.getExpressions(), true);
} else {
@@ -1527,7 +1519,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- public void despreadList(List<Expression> expressions, boolean wrap) {
+ public void despreadList(final List<Expression> expressions, final boolean wrap) {
List<Expression> spreadIndexes = new ArrayList<>();
List<Expression> spreadExpressions = new ArrayList<>();
List<Expression> normalArguments = new ArrayList<>();
@@ -1553,11 +1545,11 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitTupleExpression(TupleExpression expression) {
+ public void visitTupleExpression(final TupleExpression expression) {
visitTupleExpression(expression, false);
}
- void visitTupleExpression(TupleExpression expression, boolean useWrapper) {
+ void visitTupleExpression(final TupleExpression expression, final boolean useWrapper) {
MethodVisitor mv = controller.getMethodVisitor();
int size = expression.getExpressions().size();
@@ -1577,7 +1569,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- public void loadWrapper(Expression argument) {
+ public void loadWrapper(final Expression argument) {
MethodVisitor mv = controller.getMethodVisitor();
ClassNode goalClass = argument.getType();
visitClassExpression(new ClassExpression(goalClass));
@@ -1590,7 +1582,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitArrayExpression(ArrayExpression expression) {
+ public void visitArrayExpression(final ArrayExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
ClassNode elementType = expression.getElementType();
String arrayTypeName = BytecodeHelper.getClassInternalName(elementType);
@@ -1666,7 +1658,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitClosureListExpression(ClosureListExpression expression) {
+ public void visitClosureListExpression(final ClosureListExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
controller.getCompileStack().pushVariableScope(expression.getVariableScope());
@@ -1806,13 +1798,13 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitBytecodeExpression(BytecodeExpression expression) {
+ public void visitBytecodeExpression(final BytecodeExpression expression) {
expression.visit(controller.getMethodVisitor());
controller.getOperandStack().push(expression.getType());
}
@Override
- public void visitBytecodeSequence(BytecodeSequence bytecodeSequence) {
+ public void visitBytecodeSequence(final BytecodeSequence bytecodeSequence) {
MethodVisitor mv = controller.getMethodVisitor();
List instructions = bytecodeSequence.getInstructions();
int mark = controller.getOperandStack().getStackLength();
@@ -1834,7 +1826,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitListExpression(ListExpression expression) {
+ public void visitListExpression(final ListExpression expression) {
onLineNumber(expression, "ListExpression");
int size = expression.getExpressions().size();
@@ -1899,7 +1891,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
@Override
- public void visitGStringExpression(GStringExpression expression) {
+ public void visitGStringExpression(final GStringExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
mv.visitTypeInsn(NEW, "org/codehaus/groovy/runtime/GStringImpl");
@@ -1936,18 +1928,16 @@ public class AsmClassGenerator extends ClassGenerator {
controller.getOperandStack().push(ClassHelper.GSTRING_TYPE);
}
- /**
- * Note: ignore it. Annotation generation needs the current visitor.
- */
@Override
- public void visitAnnotations(AnnotatedNode node) {
+ public void visitAnnotations(final AnnotatedNode node) {
+ // ignore it; annotation generation needs the current visitor
}
- private void visitAnnotations(AnnotatedNode targetNode, Object visitor) {
+ private void visitAnnotations(final AnnotatedNode targetNode, final Object visitor) {
visitAnnotations(targetNode, targetNode, visitor);
}
- private void visitAnnotations(AnnotatedNode targetNode, AnnotatedNode sourceNode, Object visitor) {
+ private void visitAnnotations(final AnnotatedNode targetNode, final AnnotatedNode sourceNode, final Object visitor) {
for (AnnotationNode an : sourceNode.getAnnotations()) {
// skip built-in properties
if (an.isBuiltIn()) continue;
@@ -1959,7 +1949,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void visitParameterAnnotations(Parameter parameter, int paramNumber, MethodVisitor mv) {
+ private void visitParameterAnnotations(final Parameter parameter, final int paramNumber, final MethodVisitor mv) {
for (AnnotationNode an : parameter.getAnnotations()) {
// skip built-in properties
if (an.isBuiltIn()) continue;
@@ -1972,7 +1962,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private AnnotationVisitor getAnnotationVisitor(AnnotatedNode targetNode, AnnotationNode an, Object visitor) {
+ private AnnotationVisitor getAnnotationVisitor(final AnnotatedNode targetNode, final AnnotationNode an, final Object visitor) {
final String annotationDescriptor = BytecodeHelper.getTypeDescription(an.getClassNode());
if (targetNode instanceof MethodNode) {
return ((MethodVisitor) visitor).visitAnnotation(annotationDescriptor, an.hasRuntimeRetention());
@@ -1986,11 +1976,12 @@ public class AsmClassGenerator extends ClassGenerator {
}
/**
- * Generate the annotation attributes.
+ * Generates the annotation attributes.
+ *
* @param an the node with an annotation
* @param av the visitor to use
*/
- private void visitAnnotationAttributes(AnnotationNode an, AnnotationVisitor av) {
+ private void visitAnnotationAttributes(final AnnotationNode an, final AnnotationVisitor av) {
Map<String, Object> constantAttrs = new HashMap<>();
Map<String, PropertyExpression> enumAttrs = new HashMap<>();
Map<String, Object> atAttrs = new HashMap<>();
@@ -2034,7 +2025,7 @@ public class AsmClassGenerator extends ClassGenerator {
visitArrayAttributes(an, arrayAttrs, av);
}
- private void visitArrayAttributes(AnnotationNode an, Map<String, ListExpression> arrayAttr, AnnotationVisitor av) {
+ private void visitArrayAttributes(final AnnotationNode an, final Map<String, ListExpression> arrayAttr, final AnnotationVisitor av) {
if (arrayAttr.isEmpty()) return;
for (Map.Entry<String, ListExpression> entry : arrayAttr.entrySet()) {
AnnotationVisitor av2 = av.visitArray(entry.getKey());
@@ -2049,7 +2040,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private static int determineCommonArrayType(List<Expression> values) {
+ private static int determineCommonArrayType(final List<Expression> values) {
Expression expr = values.get(0);
int arrayElementType = -1;
if (expr instanceof AnnotationConstantExpression) {
@@ -2064,7 +2055,7 @@ public class AsmClassGenerator extends ClassGenerator {
return arrayElementType;
}
- private void visitAnnotationArrayElement(Expression expr, int arrayElementType, AnnotationVisitor av) {
+ private void visitAnnotationArrayElement(final Expression expr, final int arrayElementType, final AnnotationVisitor av) {
switch (arrayElementType) {
case 1:
AnnotationNode atAttr = (AnnotationNode) ((AnnotationConstantExpression) expr).getValue();
@@ -2090,7 +2081,7 @@ public class AsmClassGenerator extends ClassGenerator {
// Implementation methods
//-------------------------------------------------------------------------
- public static int argumentSize(Expression arguments) {
+ public static int argumentSize(final Expression arguments) {
if (arguments instanceof TupleExpression) {
TupleExpression tupleExpression = (TupleExpression) arguments;
int size = tupleExpression.getExpressions().size();
@@ -2099,12 +2090,12 @@ public class AsmClassGenerator extends ClassGenerator {
return 1;
}
- private static String[] buildExceptions(ClassNode[] exceptions) {
+ private static String[] buildExceptions(final ClassNode[] exceptions) {
if (exceptions == null) return null;
return Arrays.stream(exceptions).map(BytecodeHelper::getClassInternalName).toArray(String[]::new);
}
- private static boolean containsOnlyConstants(ListExpression list) {
+ private static boolean containsOnlyConstants(final ListExpression list) {
for (Expression exp : list.getExpressions()) {
if (exp instanceof ConstantExpression) continue;
return false;
@@ -2112,7 +2103,7 @@ public class AsmClassGenerator extends ClassGenerator {
return true;
}
- public static boolean containsSpreadExpression(Expression arguments) {
+ public static boolean containsSpreadExpression(final Expression arguments) {
List<Expression> args = null;
if (arguments instanceof TupleExpression) {
TupleExpression tupleExpression = (TupleExpression) arguments;
@@ -2133,30 +2124,26 @@ public class AsmClassGenerator extends ClassGenerator {
return controller.getClassNode().getOuterClass() != null;
}
- public static boolean isNullConstant(Expression expression) {
+ public static boolean isNullConstant(final Expression expression) {
return expression instanceof ConstantExpression && ((ConstantExpression) expression).isNullExpression();
}
- public static boolean isThisExpression(Expression expression) {
+ public static boolean isThisExpression(final Expression expression) {
return expression instanceof VariableExpression && ((VariableExpression) expression).isThisExpression();
}
- private static boolean isSuperExpression(Expression expression) {
+ private static boolean isSuperExpression(final Expression expression) {
return expression instanceof VariableExpression && ((VariableExpression) expression).isSuperExpression();
}
- private static boolean isThisOrSuper(Expression expression) {
+ private static boolean isThisOrSuper(final Expression expression) {
return isThisExpression(expression) || isSuperExpression(expression);
}
- private static boolean isVargs(Parameter[] params) {
+ private static boolean isVargs(final Parameter[] params) {
return (params.length > 0 && params[params.length - 1].getType().isArray());
}
- private static boolean samePackages(String pkg1, String pkg2) {
- return Objects.equals(pkg1, pkg2);
- }
-
private CompileUnit getCompileUnit() {
CompileUnit answer = controller.getClassNode().getCompileUnit();
if (answer == null) {
@@ -2165,14 +2152,14 @@ public class AsmClassGenerator extends ClassGenerator {
return answer;
}
- public boolean addInnerClass(ClassNode innerClass) {
+ public boolean addInnerClass(final ClassNode innerClass) {
ModuleNode mn = controller.getClassNode().getModule();
innerClass.setModule(mn);
mn.getUnit().addGeneratedInnerClass((InnerClassNode)innerClass);
return innerClasses.add(innerClass);
}
- public void onLineNumber(ASTNode statement, String message) {
+ public void onLineNumber(final ASTNode statement, final String message) {
if (statement == null || statement instanceof BlockStatement) return;
currentASTNode = statement;
@@ -2188,7 +2175,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- public void throwException(String s) {
- throw new RuntimeParserException(s, currentASTNode);
+ public void throwException(final String message) {
+ throw new RuntimeParserException(message, currentASTNode);
}
}
[groovy] 17/20: refactor and reorganize
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 8815125d153c77dd5bde6f1d2fc68b870b43ddbc
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Nov 30 10:10:34 2019 -0600
refactor and reorganize
(cherry picked from commit ec85facd66b0543fe79322cc8c50e68db8136cd9)
---
.../groovy/classgen/VariableScopeVisitor.java | 518 ++++++++++-----------
1 file changed, 242 insertions(+), 276 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index 8d4a738..531e5f6 100644
--- a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -50,6 +50,7 @@ import org.codehaus.groovy.ast.stmt.Statement;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.syntax.Types;
+import java.util.Deque;
import java.util.Iterator;
import java.util.LinkedList;
@@ -57,49 +58,51 @@ import static java.lang.reflect.Modifier.isFinal;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
/**
- * Goes through an AST and initializes the scopes.
+ * Initializes the variable scopes for an AST.
*/
public class VariableScopeVisitor extends ClassCodeVisitorSupport {
- private VariableScope currentScope = null;
- private final VariableScope headScope = new VariableScope();
- private ClassNode currentClass = null;
+ private ClassNode currentClass;
+ private VariableScope currentScope;
+ private boolean inConstructor, isSpecialConstructorCall;
+
private final SourceUnit source;
- private boolean isSpecialConstructorCall = false;
- private boolean inConstructor = false;
private final boolean recurseInnerClasses;
+ private final Deque<StateStackElement> stateStack = new LinkedList<>();
- private final LinkedList stateStack = new LinkedList();
-
- private class StateStackElement {
- final VariableScope scope;
+ private static class StateStackElement {
final ClassNode clazz;
+ final VariableScope scope;
final boolean inConstructor;
- StateStackElement() {
- scope = VariableScopeVisitor.this.currentScope;
- clazz = VariableScopeVisitor.this.currentClass;
- inConstructor = VariableScopeVisitor.this.inConstructor;
+ StateStackElement(final ClassNode currentClass, final VariableScope currentScope, final boolean inConstructor) {
+ clazz = currentClass;
+ scope = currentScope;
+ this.inConstructor = inConstructor;
}
}
public VariableScopeVisitor(SourceUnit source, boolean recurseInnerClasses) {
this.source = source;
- currentScope = headScope;
+ this.currentScope = new VariableScope();
this.recurseInnerClasses = recurseInnerClasses;
}
-
public VariableScopeVisitor(SourceUnit source) {
this(source, false);
}
+ @Override
+ protected SourceUnit getSourceUnit() {
+ return source;
+ }
+
// ------------------------------
// helper methods
//------------------------------
- private void pushState(boolean isStatic) {
- stateStack.add(new StateStackElement());
+ private void pushState(final boolean isStatic) {
+ stateStack.push(new StateStackElement(currentClass, currentScope, inConstructor));
currentScope = new VariableScope(currentScope);
currentScope.setInStaticContext(isStatic);
}
@@ -109,35 +112,26 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
}
private void popState() {
- StateStackElement element = (StateStackElement) stateStack.removeLast();
- currentScope = element.scope;
- currentClass = element.clazz;
- inConstructor = element.inConstructor;
+ StateStackElement state = stateStack.pop();
+ currentClass = state.clazz;
+ currentScope = state.scope;
+ inConstructor = state.inConstructor;
}
- private void declare(Parameter[] parameters, ASTNode node) {
- for (Parameter parameter : parameters) {
- if (parameter.hasInitialExpression()) {
- parameter.getInitialExpression().visit(this);
- }
- declare(parameter, node);
- }
- }
-
- private void declare(VariableExpression vex) {
- vex.setInStaticContext(currentScope.isInStaticContext());
- declare(vex, vex);
- vex.setAccessedVariable(vex);
+ private void declare(final VariableExpression variable) {
+ variable.setInStaticContext(currentScope.isInStaticContext());
+ declare(variable, variable);
+ variable.setAccessedVariable(variable);
}
- private void declare(Variable var, ASTNode expr) {
+ private void declare(final Variable variable, final ASTNode expression) {
String scopeType = "scope";
String variableType = "variable";
- if (expr.getClass() == FieldNode.class) {
+ if (expression.getClass() == FieldNode.class) {
scopeType = "class";
variableType = "field";
- } else if (expr.getClass() == PropertyNode.class) {
+ } else if (expression.getClass() == PropertyNode.class) {
scopeType = "class";
variableType = "property";
}
@@ -145,10 +139,10 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
StringBuilder msg = new StringBuilder();
msg.append("The current ").append(scopeType);
msg.append(" already contains a ").append(variableType);
- msg.append(" of the name ").append(var.getName());
+ msg.append(" of the name ").append(variable.getName());
- if (currentScope.getDeclaredVariable(var.getName()) != null) {
- addError(msg.toString(), expr);
+ if (currentScope.getDeclaredVariable(variable.getName()) != null) {
+ addError(msg.toString(), expression);
return;
}
@@ -158,34 +152,29 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
// to declare a variable of the same name as a class member
if (scope.getClassScope() != null && !isAnonymous(scope.getClassScope())) break;
- if (scope.getDeclaredVariable(var.getName()) != null) {
+ if (scope.getDeclaredVariable(variable.getName()) != null) {
// variable already declared
- addError(msg.toString(), expr);
+ addError(msg.toString(), expression);
break;
}
}
// declare the variable even if there was an error to allow more checks
- currentScope.putDeclaredVariable(var);
+ currentScope.putDeclaredVariable(variable);
}
- @Override
- protected SourceUnit getSourceUnit() {
- return source;
- }
-
- private Variable findClassMember(ClassNode cn, String name) {
+ private Variable findClassMember(final ClassNode cn, final String name) {
if (cn == null) return null;
+
if (cn.isScript()) {
return new DynamicVariable(name, false);
}
for (FieldNode fn : cn.getFields()) {
- if (fn.getName().equals(name)) return fn;
+ if (name.equals(fn.getName())) return fn;
}
for (MethodNode mn : cn.getMethods()) {
- String pName = getPropertyName(mn);
- if (name.equals(pName)) {
+ if (name.equals(getPropertyName(mn))) {
PropertyNode property = new PropertyNode(name, mn.getModifiers(), ClassHelper.OBJECT_TYPE, cn, null, null, null);
property.getField().setHasNoRealSourcePosition(true);
property.getField().setSynthetic(true);
@@ -205,15 +194,44 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
return findClassMember(cn.getOuterClass(), name);
}
- private static boolean isAnonymous(ClassNode node) {
+ private static boolean isAnonymous(final ClassNode node) {
return (!node.isEnum() && node instanceof InnerClassNode && ((InnerClassNode) node).isAnonymous());
}
+ private void markClosureSharedVariables() {
+ for (Iterator<Variable> it = currentScope.getReferencedLocalVariablesIterator(); it.hasNext(); ) {
+ Variable variable = it.next();
+ variable.setClosureSharedVariable(true);
+ }
+ }
+
// -------------------------------
- // different Variable based checks
+ // different Variable-based checks
// -------------------------------
- private Variable checkVariableNameForDeclaration(String name, Expression expression) {
+ private void checkFinalFieldAccess(final Expression expression) {
+ if (expression instanceof TupleExpression) {
+ TupleExpression tuple = (TupleExpression) expression;
+ for (Expression tupleExpression : tuple.getExpressions()) {
+ checkForFinal(expression, (VariableExpression) tupleExpression);
+ }
+ } else if (expression instanceof VariableExpression) {
+ checkForFinal(expression, (VariableExpression) expression);
+ }
+ // currently not looking for PropertyExpression: dealt with at runtime using ReadOnlyPropertyException
+ }
+
+ private void checkForFinal(final Expression context, final VariableExpression expression) {
+ Variable variable = expression.getAccessedVariable();
+ if (variable != null) {
+ if (isFinal(variable.getModifiers()) && variable instanceof Parameter) {
+ addError("Cannot assign a value to final variable '" + variable.getName() + "'", context);
+ }
+ // TODO: handle local variables
+ }
+ }
+
+ private Variable checkVariableNameForDeclaration(final String name, final Expression expression) {
if ("super".equals(name) || "this".equals(name)) return null;
VariableScope scope = currentScope;
@@ -222,7 +240,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
// try to find a declaration of a variable
boolean crossingStaticContext = false;
while (true) {
- crossingStaticContext = crossingStaticContext || scope.isInStaticContext();
+ crossingStaticContext = (crossingStaticContext || scope.isInStaticContext());
Variable var1 = scope.getDeclaredVariable(var.getName());
if (var1 != null) {
@@ -246,16 +264,14 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
if (classScope != null) {
Variable member = findClassMember(classScope, var.getName());
if (member != null) {
- boolean staticScope = crossingStaticContext || isSpecialConstructorCall;
- boolean staticMember = member.isInStaticContext();
- // We don't allow a static context (e.g. a static method) to access
- // a non-static variable (e.g. a non-static field).
- if (!(staticScope && !staticMember))
+ boolean staticScope = (crossingStaticContext || isSpecialConstructorCall), staticMember = member.isInStaticContext();
+ // prevent a static context (e.g. a static method) from accessing a non-static variable (e.g. a non-static field)
+ if (!(staticScope && !staticMember)) {
var = member;
+ }
}
// GROOVY-5961
- if (!isAnonymous(classScope))
- break;
+ if (!isAnonymous(classScope)) break;
}
scope = scope.getParent();
}
@@ -264,11 +280,9 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
}
VariableScope end = scope;
-
scope = currentScope;
while (scope != end) {
- if (end.isClassScope() ||
- (end.isReferencedClassVariable(name) && end.getDeclaredVariable(name) == null)) {
+ if (end.isClassScope() || (end.isReferencedClassVariable(name) && end.getDeclaredVariable(name) == null)) {
scope.putReferencedClassVariable(var);
} else {
scope.putReferencedLocalVariable(var);
@@ -280,141 +294,143 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
}
/**
- * a property on "this", like this.x is transformed to a
- * direct field access, so we need to check the
- * static context here
- *
- * @param pe the property expression to check
+ * A property on "this", like this.x is transformed to a direct field access,
+ * so we need to check the static context here.
*/
- private void checkPropertyOnExplicitThis(PropertyExpression pe) {
+ private void checkPropertyOnExplicitThis(final PropertyExpression expression) {
if (!currentScope.isInStaticContext()) return;
- Expression object = pe.getObjectExpression();
+ Expression object = expression.getObjectExpression();
if (!(object instanceof VariableExpression)) return;
VariableExpression ve = (VariableExpression) object;
if (!ve.getName().equals("this")) return;
- String name = pe.getPropertyAsString();
+ String name = expression.getPropertyAsString();
if (name == null || name.equals("class")) return;
Variable member = findClassMember(currentClass, name);
if (member == null) return;
- checkVariableContextAccess(member, pe);
+ checkVariableContextAccess(member, expression);
}
- private void checkVariableContextAccess(Variable v, Expression expr) {
- if (v.isInStaticContext() || !currentScope.isInStaticContext()) return;
+ private void checkVariableContextAccess(final Variable variable, final Expression expression) {
+ if (variable.isInStaticContext() || !currentScope.isInStaticContext()) return;
- String msg = v.getName() +
- " is declared in a dynamic context, but you tried to" +
- " access it from a static context.";
- addError(msg, expr);
+ addError(variable.getName() + " is declared in a dynamic context, but you tried to access it from a static context.", expression);
// declare a static variable to be able to continue the check
- DynamicVariable v2 = new DynamicVariable(v.getName(), currentScope.isInStaticContext());
- currentScope.putDeclaredVariable(v2);
+ currentScope.putDeclaredVariable(new DynamicVariable(variable.getName(), currentScope.isInStaticContext()));
}
- // ------------------------------
- // code visit
- // ------------------------------
+ //--------------------------------------------------------------------------
@Override
- public void visitBlockStatement(BlockStatement block) {
+ public void visitClass(final ClassNode node) {
+ // AIC are already done, doing them here again will lead to wrong scopes
+ if (isAnonymous(node)) return;
+
pushState();
- block.setVariableScope(currentScope);
- super.visitBlockStatement(block);
+ currentClass = node;
+ currentScope.setClassScope(node);
+
+ super.visitClass(node);
+ if (recurseInnerClasses) {
+ for (Iterator<InnerClassNode> innerClasses = node.getInnerClasses(); innerClasses.hasNext(); ) {
+ visitClass(innerClasses.next());
+ }
+ }
popState();
}
@Override
- public void visitForLoop(ForStatement forLoop) {
- pushState();
- forLoop.setVariableScope(currentScope);
- Parameter p = forLoop.getVariable();
- p.setInStaticContext(currentScope.isInStaticContext());
- if (p != ForStatement.FOR_LOOP_DUMMY) declare(p, forLoop);
- super.visitForLoop(forLoop);
+ public void visitField(final FieldNode node) {
+ pushState(node.isStatic());
+ super.visitField(node);
popState();
}
@Override
- public void visitIfElse(IfStatement ifElse) {
- ifElse.getBooleanExpression().visit(this);
- pushState();
- ifElse.getIfBlock().visit(this);
- popState();
- pushState();
- ifElse.getElseBlock().visit(this);
+ public void visitProperty(final PropertyNode node) {
+ pushState(node.isStatic());
+ super.visitProperty(node);
popState();
}
@Override
- public void visitDeclarationExpression(DeclarationExpression expression) {
- visitAnnotations(expression);
- // visit right side first to avoid the usage of a
- // variable before its declaration
- expression.getRightExpression().visit(this);
+ protected void visitConstructorOrMethod(final MethodNode node, final boolean isConstructor) {
+ pushState(node.isStatic());
+ inConstructor = isConstructor;
+ node.setVariableScope(currentScope);
- if (expression.isMultipleAssignmentDeclaration()) {
- TupleExpression list = expression.getTupleExpression();
- for (Expression e : list.getExpressions()) {
- declare((VariableExpression) e);
+ visitAnnotations(node);
+ for (Parameter parameter : node.getParameters()) {
+ visitAnnotations(parameter);
+ }
+ // add parameters to scope and visit init expressions after annotations
+ // to prevent the use of parameters in annotation attributes
+ for (Parameter parameter : node.getParameters()) {
+ if (parameter.hasInitialExpression()) {
+ parameter.getInitialExpression().visit(this);
}
- } else {
- declare(expression.getVariableExpression());
+ declare(parameter, node);
}
+ visitClassCodeContainer(node.getCode());
+
+ popState();
}
- @Override
- public void visitBinaryExpression(BinaryExpression be) {
- super.visitBinaryExpression(be);
+ // statements:
- if (Types.isAssignment(be.getOperation().getType())) {
- checkFinalFieldAccess(be.getLeftExpression());
- }
+ @Override
+ public void visitBlockStatement(final BlockStatement statement) {
+ pushState();
+ statement.setVariableScope(currentScope);
+ super.visitBlockStatement(statement);
+ popState();
}
- private void checkFinalFieldAccess(Expression expression) {
- // currently not looking for PropertyExpression: dealt with at runtime using ReadOnlyPropertyException
- if (!(expression instanceof VariableExpression) && !(expression instanceof TupleExpression)) return;
- if (expression instanceof TupleExpression) {
- TupleExpression list = (TupleExpression) expression;
- for (Expression e : list.getExpressions()) {
- checkForFinal(expression, (VariableExpression) e);
- }
- } else {
- checkForFinal(expression, (VariableExpression) expression);
- }
+ @Override
+ public void visitCatchStatement(final CatchStatement statement) {
+ pushState();
+ Parameter parameter = statement.getVariable();
+ parameter.setInStaticContext(currentScope.isInStaticContext());
+ declare(parameter, statement);
+ super.visitCatchStatement(statement);
+ popState();
}
- // TODO handle local variables
- private void checkForFinal(final Expression expression, VariableExpression ve) {
- Variable v = ve.getAccessedVariable();
- if (v != null) {
- boolean isFinal = isFinal(v.getModifiers());
- boolean isParameter = v instanceof Parameter;
- if (isFinal && isParameter) {
- addError("Cannot assign a value to final variable '" + v.getName() + "'", expression);
- }
- }
+ @Override
+ public void visitForLoop(final ForStatement statement) {
+ pushState();
+ statement.setVariableScope(currentScope);
+ Parameter parameter = statement.getVariable();
+ parameter.setInStaticContext(currentScope.isInStaticContext());
+ if (parameter != ForStatement.FOR_LOOP_DUMMY) declare(parameter, statement);
+ super.visitForLoop(statement);
+ popState();
}
@Override
- public void visitVariableExpression(VariableExpression expression) {
- String name = expression.getName();
- Variable v = checkVariableNameForDeclaration(name, expression);
- if (v == null) return;
- expression.setAccessedVariable(v);
- checkVariableContextAccess(v, expression);
+ public void visitIfElse(final IfStatement statement) {
+ statement.getBooleanExpression().visit(this);
+ pushState();
+ statement.getIfBlock().visit(this);
+ popState();
+ pushState();
+ statement.getElseBlock().visit(this);
+ popState();
}
+ // expressions:
+
@Override
- public void visitPropertyExpression(PropertyExpression expression) {
- expression.getObjectExpression().visit(this);
- expression.getProperty().visit(this);
- checkPropertyOnExplicitThis(expression);
+ public void visitBinaryExpression(final BinaryExpression expression) {
+ super.visitBinaryExpression(expression);
+
+ if (Types.isAssignment(expression.getOperation().getType())) {
+ checkFinalFieldAccess(expression.getLeftExpression());
+ }
}
@Override
- public void visitClosureExpression(ClosureExpression expression) {
+ public void visitClosureExpression(final ClosureExpression expression) {
pushState();
expression.setVariableScope(currentScope);
@@ -439,163 +455,113 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
popState();
}
- private void markClosureSharedVariables() {
- VariableScope scope = currentScope;
- for (Iterator<Variable> it = scope.getReferencedLocalVariablesIterator(); it.hasNext(); ) {
- it.next().setClosureSharedVariable(true);
- }
- }
-
- @Override
- public void visitCatchStatement(CatchStatement statement) {
- pushState();
- Parameter p = statement.getVariable();
- p.setInStaticContext(currentScope.isInStaticContext());
- declare(p, statement);
- super.visitCatchStatement(statement);
- popState();
- }
-
@Override
- public void visitFieldExpression(FieldExpression expression) {
- String name = expression.getFieldName();
- //TODO: change that to get the correct scope
- Variable v = checkVariableNameForDeclaration(name, expression);
- checkVariableContextAccess(v, expression);
- }
-
- // ------------------------------
- // class visit
- // ------------------------------
-
- @Override
- public void visitClass(ClassNode node) {
- // AIC are already done, doing them here again will lead to wrong scopes
- if (isAnonymous(node)) return;
+ public void visitConstructorCallExpression(final ConstructorCallExpression expression) {
+ isSpecialConstructorCall = expression.isSpecialCall();
+ super.visitConstructorCallExpression(expression);
+ isSpecialConstructorCall = false;
+ if (!expression.isUsingAnonymousInnerClass()) return;
pushState();
+ InnerClassNode innerClass = (InnerClassNode) expression.getType();
+ innerClass.setVariableScope(currentScope);
+ currentScope.setClassScope(innerClass);
+ currentScope.setInStaticContext(false);
+ for (MethodNode method : innerClass.getMethods()) {
+ Parameter[] parameters = method.getParameters();
+ if (parameters.length == 0) {
+ parameters = null; // null means no implicit "it"
+ }
+ visitClosureExpression(new ClosureExpression(parameters, method.getCode()));
+ }
- prepareVisit(node);
-
- super.visitClass(node);
- if (recurseInnerClasses) {
- Iterator<InnerClassNode> innerClasses = node.getInnerClasses();
- while (innerClasses.hasNext()) {
- visitClass(innerClasses.next());
+ for (FieldNode field : innerClass.getFields()) {
+ Expression initExpression = field.getInitialExpression();
+ pushState(field.isStatic());
+ if (initExpression != null) {
+ if (initExpression.isSynthetic() && initExpression instanceof VariableExpression
+ && ((VariableExpression) initExpression).getAccessedVariable() instanceof Parameter) {
+ // GROOVY-6834: accessing a parameter which is not yet seen in scope
+ popState();
+ continue;
+ }
+ initExpression.visit(this);
}
+ popState();
}
- popState();
- }
- /**
- * Sets the current class node context.
- */
- public void prepareVisit(ClassNode node) {
- currentClass = node;
- currentScope.setClassScope(node);
+ for (Statement initStatement : innerClass.getObjectInitializerStatements()) {
+ initStatement.visit(this);
+ }
+ markClosureSharedVariables();
+ popState();
}
@Override
- protected void visitConstructorOrMethod(MethodNode node, boolean isConstructor) {
- pushState(node.isStatic());
- inConstructor = isConstructor;
- node.setVariableScope(currentScope);
- visitAnnotations(node);
+ public void visitDeclarationExpression(final DeclarationExpression expression) {
+ visitAnnotations(expression);
+ // visit right side first to prevent the use of a variable before its declaration
+ expression.getRightExpression().visit(this);
- // GROOVY-2156
- Parameter[] parameters = node.getParameters();
- for (Parameter parameter : parameters) {
- visitAnnotations(parameter);
+ if (expression.isMultipleAssignmentDeclaration()) {
+ TupleExpression list = expression.getTupleExpression();
+ for (Expression listExpression : list.getExpressions()) {
+ declare((VariableExpression) listExpression);
+ }
+ } else {
+ declare(expression.getVariableExpression());
}
+ }
- declare(node.getParameters(), node);
- visitClassCodeContainer(node.getCode());
-
- popState();
+ @Override
+ public void visitFieldExpression(final FieldExpression expression) {
+ String name = expression.getFieldName();
+ //TODO: change that to get the correct scope
+ Variable variable = checkVariableNameForDeclaration(name, expression);
+ checkVariableContextAccess(variable, expression);
}
@Override
- public void visitMethodCallExpression(MethodCallExpression call) {
- if (call.isImplicitThis() && call.getMethod() instanceof ConstantExpression) {
- ConstantExpression methodNameConstant = (ConstantExpression) call.getMethod();
+ public void visitMethodCallExpression(final MethodCallExpression expression) {
+ if (expression.isImplicitThis() && expression.getMethod() instanceof ConstantExpression) {
+ ConstantExpression methodNameConstant = (ConstantExpression) expression.getMethod();
String methodName = methodNameConstant.getText();
if (methodName == null) {
throw new GroovyBugError("method name is null");
}
- Variable v = checkVariableNameForDeclaration(methodName, call);
- if (v != null && !(v instanceof DynamicVariable)) {
- checkVariableContextAccess(v, call);
+ Variable variable = checkVariableNameForDeclaration(methodName, expression);
+ if (variable != null && !(variable instanceof DynamicVariable)) {
+ checkVariableContextAccess(variable, expression);
}
- if (v instanceof VariableExpression || v instanceof Parameter) {
- VariableExpression object = new VariableExpression(v);
+ if (variable instanceof VariableExpression || variable instanceof Parameter) {
+ VariableExpression object = new VariableExpression(variable);
object.setSourcePosition(methodNameConstant);
- call.setObjectExpression(object);
+ expression.setObjectExpression(object);
ConstantExpression method = new ConstantExpression("call");
method.setSourcePosition(methodNameConstant); // important for GROOVY-4344
- call.setImplicitThis(false);
- call.setMethod(method);
- }
-
- }
- super.visitMethodCallExpression(call);
- }
-
- @Override
- public void visitConstructorCallExpression(ConstructorCallExpression call) {
- isSpecialConstructorCall = call.isSpecialCall();
- super.visitConstructorCallExpression(call);
- isSpecialConstructorCall = false;
- if (!call.isUsingAnonymousInnerClass()) return;
-
- pushState();
- InnerClassNode innerClass = (InnerClassNode) call.getType();
- innerClass.setVariableScope(currentScope);
- currentScope.setClassScope(innerClass);
- currentScope.setInStaticContext(false);
- for (MethodNode method : innerClass.getMethods()) {
- Parameter[] parameters = method.getParameters();
- if (parameters.length == 0)
- parameters = null; // null means no implicit "it"
- ClosureExpression cl = new ClosureExpression(parameters, method.getCode());
- visitClosureExpression(cl);
- }
-
- for (FieldNode field : innerClass.getFields()) {
- Expression expression = field.getInitialExpression();
- pushState(field.isStatic());
- if (expression != null) {
- if (expression.isSynthetic() && expression instanceof VariableExpression &&
- ((VariableExpression) expression).getAccessedVariable() instanceof Parameter) {
- // GROOVY-6834: accessing a parameter which is not yet seen in scope
- popState();
- continue;
- }
- expression.visit(this);
+ expression.setImplicitThis(false);
+ expression.setMethod(method);
}
- popState();
}
-
- for (Statement statement : innerClass.getObjectInitializerStatements()) {
- statement.visit(this);
- }
- markClosureSharedVariables();
- popState();
+ super.visitMethodCallExpression(expression);
}
@Override
- public void visitProperty(PropertyNode node) {
- pushState(node.isStatic());
- super.visitProperty(node);
- popState();
+ public void visitPropertyExpression(final PropertyExpression expression) {
+ expression.getObjectExpression().visit(this);
+ expression.getProperty().visit(this);
+ checkPropertyOnExplicitThis(expression);
}
@Override
- public void visitField(FieldNode node) {
- pushState(node.isStatic());
- super.visitField(node);
- popState();
+ public void visitVariableExpression(final VariableExpression expression) {
+ String name = expression.getName();
+ Variable variable = checkVariableNameForDeclaration(name, expression);
+ if (variable == null) return;
+ expression.setAccessedVariable(variable);
+ checkVariableContextAccess(variable, expression);
}
}
[groovy] 09/20: minor edits
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 496524cd4e74d53a715705cd90f78054e6b21b4a
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 25 17:21:08 2019 -0600
minor edits
(cherry picked from commit ceb9f95d8a7ae6a1da48160aee6e56a99f71df74)
---
.../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] 05/20: GROOVY-9318: add support for ** syntax in star
import white/black lists
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 45610c5539442da3036483ed78dcc955a191d1f1
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Nov 24 17:25:34 2019 -0600
GROOVY-9318: add support for ** syntax in star import white/black lists
This closes #1100
This closes #1101
(cherry picked from commit 4483012e08ed4115bf25280260d7b2e3c95efeed)
---
.../control/customizers/SecureASTCustomizer.java | 84 ++--
.../customizers/SecureASTCustomizerTest.groovy | 449 +++++++++++++--------
2 files changed, 315 insertions(+), 218 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java b/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
index 5f2c2c4..572d596 100644
--- a/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
+++ b/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
@@ -18,7 +18,6 @@
*/
package org.codehaus.groovy.control.customizers;
-import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.CodeVisitorSupport;
import org.codehaus.groovy.ast.GroovyCodeVisitor;
@@ -176,14 +175,14 @@ import java.util.Map;
* config.addCompilationCustomizers(imports, secure)
* GroovyClassLoader loader = new GroovyClassLoader(this.class.classLoader, config)
* </pre>
- *
+ *
* @since 1.8.0
*/
public class SecureASTCustomizer extends CompilationCustomizer {
private boolean isPackageAllowed = true;
- private boolean isMethodDefinitionAllowed = true;
private boolean isClosuresAllowed = true;
+ private boolean isMethodDefinitionAllowed = true;
// imports
private List<String> importsWhitelist;
@@ -201,7 +200,6 @@ public class SecureASTCustomizer extends CompilationCustomizer {
private List<String> staticStarImportsWhitelist;
private List<String> staticStarImportsBlacklist;
-
// indirect import checks
// if set to true, then security rules on imports will also be applied on classnodes.
// Direct instantiation of classes without imports will therefore also fail if this option is enabled
@@ -210,12 +208,12 @@ public class SecureASTCustomizer extends CompilationCustomizer {
// statements
private List<Class<? extends Statement>> statementsWhitelist;
private List<Class<? extends Statement>> statementsBlacklist;
- private final List<StatementChecker> statementCheckers = new LinkedList<StatementChecker>();
+ private final List<StatementChecker> statementCheckers = new LinkedList<>();
// expressions
private List<Class<? extends Expression>> expressionsWhitelist;
private List<Class<? extends Expression>> expressionsBlacklist;
- private final List<ExpressionChecker> expressionCheckers = new LinkedList<ExpressionChecker>();
+ private final List<ExpressionChecker> expressionCheckers = new LinkedList<>();
// tokens from Types
private List<Integer> tokensWhitelist;
@@ -303,14 +301,13 @@ public class SecureASTCustomizer extends CompilationCustomizer {
if (this.importsWhitelist == null) importsWhitelist = Collections.emptyList();
}
- /**
- * Ensures that every star import ends with .* as this is the expected syntax in import checks.
- */
private static List<String> normalizeStarImports(List<String> starImports) {
- List<String> result = new ArrayList<String>(starImports.size());
+ List<String> result = new ArrayList<>(starImports.size());
for (String starImport : starImports) {
if (starImport.endsWith(".*")) {
result.add(starImport);
+ } else if (starImport.endsWith("**")) {
+ result.add(starImport.replaceFirst("\\*+$", ""));
} else if (starImport.endsWith(".")) {
result.add(starImport + "*");
} else {
@@ -493,7 +490,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
* @param constantTypesWhiteList a list of classes.
*/
public void setConstantTypesClassesWhiteList(final List<Class> constantTypesWhiteList) {
- List<String> values = new LinkedList<String>();
+ List<String> values = new LinkedList<>();
for (Class aClass : constantTypesWhiteList) {
values.add(aClass.getName());
}
@@ -506,7 +503,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
* @param constantTypesBlackList a list of classes.
*/
public void setConstantTypesClassesBlackList(final List<Class> constantTypesBlackList) {
- List<String> values = new LinkedList<String>();
+ List<String> values = new LinkedList<>();
for (Class aClass : constantTypesBlackList) {
values.add(aClass.getName());
}
@@ -519,8 +516,8 @@ public class SecureASTCustomizer extends CompilationCustomizer {
/**
* Sets the list of classes which deny method calls.
- *
- * Please note that since Groovy is a dynamic language, and
+ *
+ * Please note that since Groovy is a dynamic language, and
* this class performs a static type check, it will be reletively
* simple to bypass any blacklist unless the receivers blacklist contains, at
* a minimum, Object, Script, GroovyShell, and Eval. Additionally,
@@ -543,7 +540,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
* @param receiversBlacklist a list of classes.
*/
public void setReceiversClassesBlackList(final List<Class> receiversBlacklist) {
- List<String> values = new LinkedList<String>();
+ List<String> values = new LinkedList<>();
for (Class aClass : receiversBlacklist) {
values.add(aClass.getName());
}
@@ -572,7 +569,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
* @param receiversWhitelist a list of classes.
*/
public void setReceiversClassesWhiteList(final List<Class> receiversWhitelist) {
- List<String> values = new LinkedList<String>();
+ List<String> values = new LinkedList<>();
for (Class aClass : receiversWhitelist) {
values.add(aClass.getName());
}
@@ -581,7 +578,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
@Override
public void call(final SourceUnit source, final GeneratorContext context, final ClassNode classNode) throws CompilationFailedException {
- final ModuleNode ast = source.getAST();
+ ModuleNode ast = source.getAST();
if (!isPackageAllowed && ast.getPackage() != null) {
throw new SecurityException("Package definitions are not allowed");
}
@@ -590,12 +587,10 @@ public class SecureASTCustomizer extends CompilationCustomizer {
// verify imports
if (importsBlacklist != null || importsWhitelist != null || starImportsBlacklist != null || starImportsWhitelist != null) {
for (ImportNode importNode : ast.getImports()) {
- final String className = importNode.getClassName();
- assertImportIsAllowed(className);
+ assertImportIsAllowed(importNode.getClassName());
}
for (ImportNode importNode : ast.getStarImports()) {
- final String className = importNode.getPackageName();
- assertStarImportIsAllowed(className + "*");
+ assertStarImportIsAllowed(importNode.getPackageName() + "*");
}
}
@@ -611,7 +606,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
}
}
- final GroovyCodeVisitor visitor = createGroovyCodeVisitor();
+ GroovyCodeVisitor visitor = createGroovyCodeVisitor();
ast.getStatementBlock().visit(visitor);
for (ClassNode clNode : ast.getClasses()) {
if (clNode!=classNode) {
@@ -627,7 +622,9 @@ public class SecureASTCustomizer extends CompilationCustomizer {
List<MethodNode> methods = filterMethods(classNode);
if (isMethodDefinitionAllowed) {
for (MethodNode method : methods) {
- if (method.getDeclaringClass()==classNode && method.getCode() != null) method.getCode().visit(visitor);
+ if (method.getDeclaringClass() == classNode && method.getCode() != null) {
+ method.getCode().visit(visitor);
+ }
}
}
}
@@ -643,7 +640,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
}
protected static List<MethodNode> filterMethods(ClassNode owner) {
- List<MethodNode> result = new LinkedList<MethodNode>();
+ List<MethodNode> result = new LinkedList<>();
List<MethodNode> methods = owner.getMethods();
for (MethodNode method : methods) {
if (method.getDeclaringClass() == owner && !method.isSynthetic()) {
@@ -655,37 +652,40 @@ public class SecureASTCustomizer extends CompilationCustomizer {
}
protected void assertStarImportIsAllowed(final String packageName) {
- if (starImportsWhitelist != null && !starImportsWhitelist.contains(packageName)) {
+ if (starImportsWhitelist != null && !(starImportsWhitelist.contains(packageName)
+ || starImportsWhitelist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith))) {
throw new SecurityException("Importing [" + packageName + "] is not allowed");
}
- if (starImportsBlacklist != null && starImportsBlacklist.contains(packageName)) {
+ if (starImportsBlacklist != null && (starImportsBlacklist.contains(packageName)
+ || starImportsBlacklist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith))) {
throw new SecurityException("Importing [" + packageName + "] is not allowed");
}
}
protected void assertImportIsAllowed(final String className) {
- if (importsWhitelist != null && !importsWhitelist.contains(className)) {
+ if (importsWhitelist != null || starImportsWhitelist != null) {
+ if (importsWhitelist != null && importsWhitelist.contains(className)) {
+ return;
+ }
if (starImportsWhitelist != null) {
- // we should now check if the import is in the star imports
- ClassNode node = ClassHelper.make(className);
- final String packageName = node.getPackageName();
- if (!starImportsWhitelist.contains(packageName + ".*")) {
- throw new SecurityException("Importing [" + className + "] is not allowed");
+ String packageName = className.substring(0, className.lastIndexOf('.') + 1) + "*";
+ if (starImportsWhitelist.contains(packageName)
+ || starImportsWhitelist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith)) {
+ return;
}
- } else {
- throw new SecurityException("Importing [" + className + "] is not allowed");
}
- }
- if (importsBlacklist != null && importsBlacklist.contains(className)) {
throw new SecurityException("Importing [" + className + "] is not allowed");
- }
- // check that there's no star import blacklist
- if (starImportsBlacklist != null) {
- ClassNode node = ClassHelper.make(className);
- final String packageName = node.getPackageName();
- if (starImportsBlacklist.contains(packageName + ".*")) {
+ } else {
+ if (importsBlacklist != null && importsBlacklist.contains(className)) {
throw new SecurityException("Importing [" + className + "] is not allowed");
}
+ if (starImportsBlacklist != null) {
+ String packageName = className.substring(0, className.lastIndexOf('.') + 1) + "*";
+ if (starImportsBlacklist.contains(packageName) ||
+ starImportsBlacklist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith)) {
+ throw new SecurityException("Importing [" + className + "] is not allowed");
+ }
+ }
}
}
diff --git a/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy b/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
index d4ea67f..2218f9b 100644
--- a/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
+++ b/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
@@ -18,48 +18,49 @@
*/
package org.codehaus.groovy.control.customizers
-import groovy.test.GroovyTestCase
import org.codehaus.groovy.ast.expr.BinaryExpression
import org.codehaus.groovy.ast.expr.ConstantExpression
import org.codehaus.groovy.ast.expr.MethodCallExpression
import org.codehaus.groovy.control.CompilerConfiguration
import org.codehaus.groovy.control.MultipleCompilationErrorsException
import org.codehaus.groovy.syntax.Types
+import org.junit.Before
+import org.junit.Test
/**
- * Tests for the {@link SecureASTCustomizer} class.
+ * Tests for {@link SecureASTCustomizer}.
*/
-class SecureASTCustomizerTest extends GroovyTestCase {
- CompilerConfiguration configuration
- SecureASTCustomizer customizer
+final class SecureASTCustomizerTest {
+ private final CompilerConfiguration configuration = new CompilerConfiguration()
+ private final SecureASTCustomizer customizer = new SecureASTCustomizer()
+
+ @Before
void setUp() {
- configuration = new CompilerConfiguration()
- customizer = new SecureASTCustomizer()
configuration.addCompilationCustomizers(customizer)
}
- private boolean hasSecurityException(Closure closure) {
- boolean result = false;
+ private static boolean hasSecurityException(Closure closure) {
+ boolean result = false
try {
closure()
} catch (SecurityException e) {
result = true
} catch (MultipleCompilationErrorsException e) {
- result = e.errorCollector.errors.any {it.cause?.class == SecurityException }
+ result = e.errorCollector.errors.any { it.cause?.class == SecurityException }
}
-
- result
+ return result
}
+ @Test
void testPackageDefinition() {
- def shell = new GroovyShell(configuration)
- String script = """
+ String script = '''
package dummy
class A {
}
new A()
- """
+ '''
+ def shell = new GroovyShell(configuration)
shell.evaluate(script)
// no error means success
customizer.packageAllowed = false
@@ -68,14 +69,15 @@ class SecureASTCustomizerTest extends GroovyTestCase {
}
}
+ @Test
void testMethodDefinition() {
- def shell = new GroovyShell(configuration)
- String script = """
+ String script = '''
def method() {
true
}
method()
- """
+ '''
+ def shell = new GroovyShell(configuration)
shell.evaluate(script)
// no error means success
customizer.methodDefinitionAllowed = false
@@ -84,16 +86,17 @@ class SecureASTCustomizerTest extends GroovyTestCase {
}
}
+ @Test
void testMethodDefinitionInClass() {
- def shell = new GroovyShell(configuration)
- String script = """
+ String script = '''
class A {
def method() {
true
}
}
new A()
- """
+ '''
+ def shell = new GroovyShell(configuration)
shell.evaluate(script)
// no error means success
customizer.methodDefinitionAllowed = false
@@ -102,328 +105,433 @@ class SecureASTCustomizerTest extends GroovyTestCase {
}
}
+ @Test
void testClassExtendingClassWithMethods() {
- def shell = new GroovyShell(configuration)
- String script = """
+ String script = '''
class A extends LinkedList {
}
new A()
- """
+ '''
+ def shell = new GroovyShell(configuration)
shell.evaluate(script)
// no error means success
customizer.methodDefinitionAllowed = false
shell.evaluate(script)
}
+ @Test
void testExpressionWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.expressionsWhitelist = [BinaryExpression, ConstantExpression]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1+1')
assert hasSecurityException {
- shell.evaluate("""
+ shell.evaluate('''
class A {}
new A()
- """)
+ ''')
}
}
+ @Test
void testExpressionBlackList() {
- def shell = new GroovyShell(configuration)
customizer.expressionsBlacklist = [MethodCallExpression]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1+1')
assert hasSecurityException {
- shell.evaluate("""
+ shell.evaluate('''
1+1
if (1+1==2) {
"test".length()
}
- """)
+ ''')
}
}
+ @Test
void testTokenWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.tokensWhitelist = [Types.PLUS, Types.MINUS]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1+1;1-1')
assert hasSecurityException {
- shell.evaluate("""
+ shell.evaluate('''
if (i==2) println 'ok'
- """)
+ ''')
}
}
+ @Test
void testTokenBlackList() {
- def shell = new GroovyShell(configuration)
customizer.tokensBlacklist = [Types.PLUS_PLUS]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1+1;1-1')
assert hasSecurityException {
- shell.evaluate("""
+ shell.evaluate('''
i++
- """)
+ ''')
}
}
+ @Test
void testImportWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.importsWhitelist = ['java.util.ArrayList']
- shell.evaluate("""
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import java.util.ArrayList
new ArrayList()
- """)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
- import java.util.LinkedList
- new LinkedList()
- """)
+ shell.evaluate('''
+ import java.util.LinkedList
+ new LinkedList()
+ ''')
}
}
- void testStarImportWhiteList() {
- def shell = new GroovyShell(configuration)
+ @Test
+ void testStarImportWhiteList1() {
customizer.starImportsWhitelist = ['java.util.*']
- shell.evaluate("""
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import java.util.ArrayList
new ArrayList()
- """)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
- import java.util.concurrent.atomic.AtomicInteger
- new AtomicInteger(0)
- """)
+ shell.evaluate('''
+ import java.util.concurrent.atomic.AtomicInteger
+ new AtomicInteger(0)
+ ''')
}
assert hasSecurityException {
- shell.evaluate("""
- import java.util.*
- import java.util.concurrent.atomic.*
- new ArrayList()
- new AtomicInteger(0)
- """)
+ shell.evaluate('''
+ import java.util.*
+ import java.util.concurrent.atomic.*
+ new ArrayList()
+ new AtomicInteger(0)
+ ''')
}
}
- void testStarImportWhiteListWithImportWhiteList() {
+ @Test
+ void testStarImportWhiteList2() {
+ customizer.starImportsWhitelist = ['java.**']
def shell = new GroovyShell(configuration)
+ shell.evaluate('''
+ import java.lang.Object
+ Object obj
+ ''')
+ assert hasSecurityException {
+ shell.evaluate('''
+ import javax.swing.Action
+ Action act
+ ''')
+ }
+ assert hasSecurityException {
+ shell.evaluate('''
+ import java.util.*
+ import javax.swing.*
+ Object obj
+ Action act
+ ''')
+ }
+ }
+
+ @Test
+ void testStarImportWhiteListWithImportWhiteList() {
customizer.importsWhitelist = ['java.util.concurrent.atomic.AtomicInteger']
customizer.starImportsWhitelist = ['java.util.*']
- shell.evaluate("""
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import java.util.ArrayList
new ArrayList()
- """)
- shell.evaluate("""
+ ''')
+ shell.evaluate('''
import java.util.concurrent.atomic.AtomicInteger
new AtomicInteger(0)
- """)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
- import java.util.concurrent.atomic.AtomicBoolean
- new AtomicBoolean(false)
- """)
+ shell.evaluate('''
+ import java.util.concurrent.atomic.AtomicBoolean
+ new AtomicBoolean(false)
+ ''')
}
}
+ @Test
void testImportBlackList() {
- def shell = new GroovyShell(configuration)
customizer.importsBlacklist = ['java.util.LinkedList']
- shell.evaluate("""
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import java.util.ArrayList
new ArrayList()
- """)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
+ shell.evaluate('''
+ import java.util.LinkedList
+ new LinkedList()
+ ''')
+ }
+ }
+
+ @Test
+ void testStarImportBlackList1() {
+ customizer.starImportsBlacklist = ['java.lang.*']
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import java.util.LinkedList
- new LinkedList()
- """)
+ import javax.swing.Action
+ LinkedList list
+ Action act
+ ''')
+ assert hasSecurityException {
+ shell.evaluate('''
+ import java.lang.Object
+ Object obj
+ ''')
}
}
- void testStarImportBlackListWithImportBlackList() {
+ @Test
+ void testStarImportBlackList2() {
+ customizer.starImportsBlacklist = ['java.**']
def shell = new GroovyShell(configuration)
+ shell.evaluate('''
+ import javax.swing.Action
+ Action act
+ ''')
+ assert hasSecurityException {
+ shell.evaluate('''
+ import java.lang.Object
+ Object obj
+ ''')
+ }
+ assert hasSecurityException {
+ shell.evaluate('''
+ import java.util.Deque
+ Deque deck
+ ''')
+ }
+ }
+
+ @Test
+ void testStarImportBlackListWithImportBlackList() {
customizer.importsBlacklist = ['java.util.concurrent.atomic.AtomicBoolean']
customizer.starImportsBlacklist = ['java.util.*']
+ def shell = new GroovyShell(configuration)
assert hasSecurityException {
- shell.evaluate("""
- import java.util.ArrayList
- new ArrayList()
- """)
+ shell.evaluate('''
+ import java.util.ArrayList
+ new ArrayList()
+ ''')
}
- shell.evaluate("""
- import java.util.concurrent.atomic.AtomicInteger
- new AtomicInteger(0)
- """)
+ shell.evaluate('''
+ import java.util.concurrent.atomic.AtomicInteger
+ new AtomicInteger(0)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
- import java.util.concurrent.atomic.AtomicBoolean
- new AtomicBoolean(false)
- """)
+ shell.evaluate('''
+ import java.util.concurrent.atomic.AtomicBoolean
+ new AtomicBoolean(false)
+ ''')
}
}
-
+ @Test
void testIndirectImportWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.importsWhitelist = ['java.util.ArrayList']
customizer.indirectImportCheckEnabled = true
- shell.evaluate("""
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import java.util.ArrayList
new ArrayList()
- """)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
- new java.util.LinkedList()
- """)
-
- assert hasSecurityException {
- shell.evaluate("""
- return java.util.LinkedList.&size
- """)
- }
+ shell.evaluate('''
+ new java.util.LinkedList()
+ ''')
+ }
+ assert hasSecurityException {
+ shell.evaluate('''
+ return java.util.LinkedList.&size
+ ''')
}
}
+ @Test
void testIndirectStarImportWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.starImportsWhitelist = ['java.util.*']
customizer.indirectImportCheckEnabled = true
- shell.evaluate("""
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import java.util.ArrayList
new ArrayList()
- """)
- shell.evaluate("""
+ ''')
+ shell.evaluate('''
new java.util.ArrayList()
- """)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
- new java.util.concurrent.atomic.AtomicBoolean(false)
- """)
-
- assert hasSecurityException {
- shell.evaluate("""
- return java.util.concurrent.atomic.AtomicBoolean.&get
- """)
- }
+ shell.evaluate('''
+ new java.util.concurrent.atomic.AtomicBoolean(false)
+ ''')
+ }
+ assert hasSecurityException {
+ shell.evaluate('''
+ return java.util.concurrent.atomic.AtomicBoolean.&get
+ ''')
}
}
+ @Test
void testStaticImportWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.staticImportsWhitelist = ['java.lang.Math.PI']
- shell.evaluate("""
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import static java.lang.Math.PI
PI
- """)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
- import static java.lang.Math.PI
- import static java.lang.Math.cos
- cos(PI)
- """)
+ shell.evaluate('''
+ import static java.lang.Math.PI
+ import static java.lang.Math.cos
+ cos(PI)
+ ''')
}
}
+ @Test
void testStaticStarImportWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.staticStarImportsWhitelist = ['java.lang.Math.*']
- shell.evaluate("""
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('''
import static java.lang.Math.PI
import static java.lang.Math.cos
cos(PI)
- """)
+ ''')
assert hasSecurityException {
- shell.evaluate("""
- import static java.util.Collections.*
- sort([5,4,2])
- """)
+ shell.evaluate('''
+ import static java.util.Collections.*
+ sort([5,4,2])
+ ''')
}
}
+ @Test
void testIndirectStaticImport() {
- def shell = new GroovyShell(configuration)
customizer.staticImportsWhitelist = ['java.lang.Math.PI']
customizer.indirectImportCheckEnabled = true
- assert hasSecurityException {shell.evaluate('java.lang.Math.cos(1)')}
+ def shell = new GroovyShell(configuration)
+ assert hasSecurityException {
+ shell.evaluate('java.lang.Math.cos(1)')
+ }
}
+ @Test
void testIndirectStaticStarImport() {
- def shell = new GroovyShell(configuration)
customizer.staticStarImportsWhitelist = ['java.lang.Math.*']
customizer.indirectImportCheckEnabled = true
+ def shell = new GroovyShell(configuration)
shell.evaluate('java.lang.Math.cos(1)')
- assert hasSecurityException {shell.evaluate('java.util.Collections.unmodifiableList([1])')}
+ assert hasSecurityException {
+ shell.evaluate('java.util.Collections.unmodifiableList([1])')
+ }
}
+ @Test
void testConstantTypeWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.constantTypesClassesWhiteList = [Integer.TYPE]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1')
- assert hasSecurityException {shell.evaluate('"string"')}
- assert hasSecurityException {shell.evaluate('2d')}
+ assert hasSecurityException {
+ shell.evaluate('"string"')
+ }
+ assert hasSecurityException {
+ shell.evaluate('2d')
+ }
}
+ @Test
void testConstantTypeBlackList() {
- def shell = new GroovyShell(configuration)
customizer.constantTypesClassesBlackList = [String]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1')
shell.evaluate('2d')
- assert hasSecurityException {shell.evaluate('"string"')}
+ assert hasSecurityException {
+ shell.evaluate('"string"')
+ }
}
+ @Test
void testReceiverWhiteList() {
- def shell = new GroovyShell(configuration)
customizer.receiversClassesWhiteList = [Integer.TYPE]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1.plus(1)')
- assert hasSecurityException {shell.evaluate('"string".toUpperCase()')}
- assert hasSecurityException {shell.evaluate('2.0.multiply(4)')}
+ assert hasSecurityException {
+ shell.evaluate('"string".toUpperCase()')
+ }
+ assert hasSecurityException {
+ shell.evaluate('2.0.multiply(4)')
+ }
}
+ @Test
void testReceiverBlackList() {
- def shell = new GroovyShell(configuration)
customizer.receiversClassesBlackList = [String]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1.plus(1)')
shell.evaluate('2.0.multiply(4)')
- assert hasSecurityException {shell.evaluate('"string".toUpperCase()')}
+ assert hasSecurityException {
+ shell.evaluate('"string".toUpperCase()')
+ }
}
+ @Test
void testReceiverWhiteListWithStaticMethod() {
- def shell = new GroovyShell(configuration)
customizer.receiversClassesWhiteList = [Integer.TYPE]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1.plus(1)')
- assert hasSecurityException {shell.evaluate('java.lang.Math.cos(2)')}
+ assert hasSecurityException {
+ shell.evaluate('java.lang.Math.cos(2)')
+ }
}
+ @Test
void testReceiverBlackListWithStaticMethod() {
- def shell = new GroovyShell(configuration)
customizer.receiversClassesBlackList = [Math]
+ def shell = new GroovyShell(configuration)
shell.evaluate('1.plus(1)')
shell.evaluate('Collections.sort([])')
- assert hasSecurityException {shell.evaluate('java.lang.Math.cos(2)')}
+ assert hasSecurityException {
+ shell.evaluate('java.lang.Math.cos(2)')
+ }
}
- // Testcase for GROOVY-4978
+ @Test // GROOVY-4978
void testVisitMethodBody() {
- def shell = new GroovyShell(configuration)
customizer.importsBlacklist = [
"java.lang.System",
"groovy.lang.GroovyShell",
"groovy.lang.GroovyClassLoader"]
customizer.indirectImportCheckEnabled = true
-
- assert hasSecurityException {shell.evaluate('System.println(1)')}
- assert hasSecurityException {shell.evaluate('def x() { System.println(1) }')}
+ def shell = new GroovyShell(configuration)
+ assert hasSecurityException {
+ shell.evaluate('System.println(1)')
+ }
+ assert hasSecurityException {
+ shell.evaluate('def x() { System.println(1) }')
+ }
}
- // GROOVY-7424
+ @Test // GROOVY-7424
void testClassWithInterfaceVisitable() {
def shell = new GroovyShell(configuration)
shell.evaluate '''
interface Foo { def baz() }
- class Bar implements Foo { def baz() { 42 }}
+ class Bar implements Foo { def baz() { 42 } }
assert new Bar().baz() == 42
'''
}
- // GROOVY-6153
+ @Test // GROOVY-6153
void testDeterministicWhitelistBehaviour() {
- def shell = new GroovyShell(configuration)
def classWhiteList = ["java.lang.Object", "test"]
customizer.with {
setIndirectImportCheckEnabled(true);
@@ -433,7 +541,7 @@ class SecureASTCustomizerTest extends GroovyTestCase {
setClosuresAllowed(true);
setMethodDefinitionAllowed(true);
}
-
+ def shell = new GroovyShell(configuration)
assert hasSecurityException {
shell.evaluate '''
java.lang.System.out.println("run ")
@@ -441,9 +549,8 @@ class SecureASTCustomizerTest extends GroovyTestCase {
}
}
- // GROOVY-6153
+ @Test // GROOVY-6153
void testDeterministicWhitelistBehaviour2() {
- def shell = new GroovyShell(configuration)
def classWhiteList = ["java.lang.Object", "test"]
customizer.with {
setIndirectImportCheckEnabled(true);
@@ -453,7 +560,7 @@ class SecureASTCustomizerTest extends GroovyTestCase {
setClosuresAllowed(true);
setMethodDefinitionAllowed(true);
}
-
+ def shell = new GroovyShell(configuration)
assert hasSecurityException {
shell.evaluate '''
java.lang.Long x = 666L
@@ -461,24 +568,14 @@ class SecureASTCustomizerTest extends GroovyTestCase {
}
}
- // GROOVY-8135
+ @Test // GROOVY-8135
void testStarImportsWhiteListWithIndirectImportCheckEnabled() {
- SecureASTCustomizer customizer = new SecureASTCustomizer()
- customizer.setIndirectImportCheckEnabled(true)
-
- List<String> starImportsWhitelist = new ArrayList<String>()
- starImportsWhitelist.add("java.lang")
- customizer.setStarImportsWhitelist(starImportsWhitelist)
-
- CompilerConfiguration cc = new CompilerConfiguration()
- cc.addCompilationCustomizers(customizer)
-
- ClassLoader parent = getClass().getClassLoader()
- GroovyClassLoader loader = new GroovyClassLoader(parent, cc)
- loader.parseClass("Object object = new Object()")
- loader.parseClass("Object object = new Object(); object.hashCode()")
- loader.parseClass("Object[] array = new Object[0]; array.size()")
- loader.parseClass("Object[][] array = new Object[0][0]; array.size()")
+ customizer.indirectImportCheckEnabled = true
+ customizer.starImportsWhitelist = ['java.lang']
+ def shell = new GroovyShell(configuration)
+ shell.evaluate('Object object = new Object()')
+ shell.evaluate('Object object = new Object(); object.hashCode()')
+ shell.evaluate('Object[] array = new Object[0]; array.size()')
+ shell.evaluate('Object[][] array = new Object[0][0]; array.size()')
}
-
}
[groovy] 14/20: refactor InvocationWriter to reduce duplication
around makeCall
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 1c9d4e5dd70afa98c594418a800ae0c15d6401a6
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Nov 26 16:36:17 2019 -0600
refactor InvocationWriter to reduce duplication around makeCall
- replace usesSuper with AsmClassGenerator#isSuperExpression
- handling of "this" and "super" was a bit spread around
(cherry picked from commit b56d1c17cc48f024d500688d4b2a8ca4a609f1cd)
---
.../groovy/classgen/AsmClassGenerator.java | 2 +-
.../groovy/classgen/asm/InvocationWriter.java | 401 ++++++++-------------
.../classgen/asm/sc/StaticInvocationWriter.java | 13 +-
3 files changed, 164 insertions(+), 252 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 717c735..8123d66 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -2132,7 +2132,7 @@ public class AsmClassGenerator extends ClassGenerator {
return expression instanceof VariableExpression && ((VariableExpression) expression).isThisExpression();
}
- private static boolean isSuperExpression(final Expression expression) {
+ public static boolean isSuperExpression(final Expression expression) {
return expression instanceof VariableExpression && ((VariableExpression) expression).isSuperExpression();
}
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 123f38b..71fef6c 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -22,7 +22,6 @@ import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.ConstructorNode;
import org.codehaus.groovy.ast.FieldNode;
-import org.codehaus.groovy.ast.InnerClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
@@ -40,7 +39,6 @@ import org.codehaus.groovy.ast.expr.TupleExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.tools.WideningCategories;
import org.codehaus.groovy.classgen.AsmClassGenerator;
-import org.codehaus.groovy.classgen.asm.OptimizingStatementWriter.StatementMeta;
import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
import org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation;
import org.codehaus.groovy.runtime.typehandling.ShortTypeHandling;
@@ -50,7 +48,6 @@ import org.objectweb.asm.MethodVisitor;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
-import java.util.Comparator;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
@@ -82,64 +79,43 @@ public class InvocationWriter {
public static final MethodCallerMultiAdapter invokeStaticMethod = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "invokeStaticMethod", true, true);
public static final MethodCaller invokeClosureMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "invokeClosure");
public static final MethodCaller castToVargsArray = MethodCaller.newStatic(DefaultTypeTransformation.class, "castToVargsArray");
- private static final MethodNode CLASS_FOR_NAME_STRING = ClassHelper.CLASS_Type.getDeclaredMethod("forName", new Parameter[]{new Parameter(ClassHelper.STRING_TYPE,"name")});
+ private static final MethodNode CLASS_FOR_NAME_STRING = ClassHelper.CLASS_Type.getDeclaredMethod("forName", new Parameter[]{new Parameter(ClassHelper.STRING_TYPE, "name")});
// type conversions
- private static final MethodCaller
- asTypeMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "asType"),
- castToTypeMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "castToType"),
- castToClassMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToClass"),
- castToStringMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToString"),
- castToEnumMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToEnum");
+ private static final MethodCaller asTypeMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "asType");
+ private static final MethodCaller castToTypeMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "castToType");
+ private static final MethodCaller castToClassMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToClass");
+ private static final MethodCaller castToStringMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToString");
+ private static final MethodCaller castToEnumMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToEnum");
// constructor calls with this() and super()
- static final MethodCaller selectConstructorAndTransformArguments = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "selectConstructorAndTransformArguments");
+ private static final MethodCaller selectConstructorAndTransformArguments = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "selectConstructorAndTransformArguments");
private final WriterController controller;
-
- public InvocationWriter(WriterController wc) {
- this.controller = wc;
+
+ public InvocationWriter(final WriterController controller) {
+ this.controller = controller;
}
- private void makeInvokeMethodCall(MethodCallExpression call, boolean useSuper, MethodCallerMultiAdapter adapter) {
- // receiver
- // we operate on GroovyObject if possible
- Expression objectExpression = call.getObjectExpression();
- // message name
- Expression messageName = new CastExpression(ClassHelper.STRING_TYPE, call.getMethod());
- if (useSuper) {
- ClassNode classNode = controller.isInClosure() ? controller.getOutermostClass() : controller.getClassNode(); // GROOVY-4035
- ClassNode superClass = classNode.getSuperClass();
- makeCall(call, new ClassExpression(superClass),
- objectExpression, messageName,
- call.getArguments(), adapter,
- call.isSafe(), call.isSpreadSafe(),
- false
- );
- } else {
- makeCall(call, objectExpression, messageName,
- call.getArguments(), adapter,
- call.isSafe(), call.isSpreadSafe(),
- call.isImplicitThis()
- );
+ public void makeCall(final Expression origin, final Expression receiver, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, boolean safe, final boolean spreadSafe, boolean implicitThis) {
+ ClassNode sender = controller.getClassNode();
+ if (AsmClassGenerator.isSuperExpression(receiver) || (AsmClassGenerator.isThisExpression(receiver) && !implicitThis)) {
+ while (sender.isDerivedFrom(ClassHelper.CLOSURE_TYPE) && sender.implementsInterface(ClassHelper.GENERATED_CLOSURE_Type)) {
+ sender = sender.getOuterClass();
+ }
+ if (AsmClassGenerator.isSuperExpression(receiver)) {
+ sender = sender.getSuperClass(); // GROOVY-4035
+ implicitThis = false; // prevent recursion
+ safe = false; // GROOVY-6045
+ }
}
+
+ makeCall(origin, new ClassExpression(sender), receiver, message, arguments, adapter, safe, spreadSafe, implicitThis);
}
-
- public void makeCall(
- Expression origin,
- Expression receiver, Expression message, Expression arguments,
- MethodCallerMultiAdapter adapter,
- boolean safe, boolean spreadSafe, boolean implicitThis
- ) {
- ClassNode cn = controller.getClassNode();
- if (controller.isInClosure() && !implicitThis && AsmClassGenerator.isThisExpression(receiver)) cn=cn.getOuterClass();
- makeCall(origin, new ClassExpression(cn), receiver, message, arguments,
- adapter, safe, spreadSafe, implicitThis);
- }
-
- protected boolean writeDirectMethodCall(MethodNode target, boolean implicitThis, Expression receiver, TupleExpression args) {
- if (target==null) return false;
-
+
+ protected boolean writeDirectMethodCall(final MethodNode target, final boolean implicitThis, final Expression receiver, final TupleExpression args) {
+ if (target == null) return false;
+
String methodName = target.getName();
CompileStack compileStack = controller.getCompileStack();
OperandStack operandStack = controller.getOperandStack();
@@ -152,20 +128,20 @@ public class InvocationWriter {
opcode = INVOKESTATIC;
} else if (declaringClass.isInterface()) {
opcode = INVOKEINTERFACE;
- } else if (target.isPrivate() || ((receiver instanceof VariableExpression && ((VariableExpression) receiver).isSuperExpression()))) {
+ } else if (target.isPrivate() || AsmClassGenerator.isSuperExpression(receiver)) {
opcode = INVOKESPECIAL;
}
// handle receiver
int argumentsToRemove = 0;
- if (opcode!=INVOKESTATIC) {
- if (receiver!=null) {
+ if (opcode != INVOKESTATIC) {
+ if (receiver != null) {
// load receiver if not static invocation
// todo: fix inner class case
if (implicitThis
+ && classNode.getOuterClass() != null
&& !classNode.isDerivedFrom(declaringClass)
- && !classNode.implementsInterface(declaringClass)
- && classNode instanceof InnerClassNode) {
+ && !classNode.implementsInterface(declaringClass)) {
// we are calling an outer class method
compileStack.pushImplicitThis(false);
if (controller.isInClosure()) {
@@ -180,18 +156,18 @@ public class InvocationWriter {
}
operandStack.doGroovyCast(declaringClass);
compileStack.popImplicitThis();
- argumentsToRemove++;
+ argumentsToRemove += 1;
} else {
mv.visitIntInsn(ALOAD,0);
operandStack.push(classNode);
- argumentsToRemove++;
+ argumentsToRemove += 1;
}
}
int stackSize = operandStack.getStackLength();
String owner = BytecodeHelper.getClassInternalName(declaringClass);
- ClassNode receiverType = receiver!=null?controller.getTypeChooser().resolveType(receiver, classNode):declaringClass;
+ ClassNode receiverType = receiver != null ? controller.getTypeChooser().resolveType(receiver, classNode) : declaringClass;
if (opcode == INVOKEVIRTUAL && ClassHelper.OBJECT_TYPE.equals(declaringClass)) {
// avoid using a narrowed type if the method is defined on object because it can interfere
// with delegate type inference in static compilation mode and trigger a ClassCastException
@@ -215,8 +191,7 @@ public class InvocationWriter {
&& !receiverType.equals(declaringClass))
&& receiverType.isDerivedFrom(declaringClass)
&& !receiverType.getPackageName().equals(classNode.getPackageName())) {
- // package private class, public method
- // see GROOVY-6962
+ // GROOVY-6962: package private class, public method
owner = BytecodeHelper.getClassInternalName(receiverType);
}
}
@@ -226,7 +201,7 @@ public class InvocationWriter {
String desc = BytecodeHelper.getMethodDescriptor(target.getReturnType(), target.getParameters());
mv.visitMethodInsn(opcode, owner, methodName, desc, declaringClass.isInterface());
ClassNode ret = target.getReturnType().redirect();
- if (ret==ClassHelper.VOID_TYPE) {
+ if (ret == ClassHelper.VOID_TYPE) {
ret = ClassHelper.OBJECT_TYPE;
mv.visitInsn(ACONST_NULL);
}
@@ -236,33 +211,32 @@ public class InvocationWriter {
return true;
}
- private boolean lastIsArray(List<Expression> argumentList, int pos) {
+ private boolean lastIsArray(final List<Expression> argumentList, final int pos) {
Expression last = argumentList.get(pos);
ClassNode type = controller.getTypeChooser().resolveType(last, controller.getClassNode());
return type.isArray();
}
-
+
// load arguments
- protected void loadArguments(List<Expression> argumentList, Parameter[] para) {
- if (para.length==0) return;
+ protected void loadArguments(final List<Expression> argumentList, final Parameter[] para) {
+ if (para.length == 0) return;
ClassNode lastParaType = para[para.length - 1].getOriginType();
AsmClassGenerator acg = controller.getAcg();
OperandStack operandStack = controller.getOperandStack();
- if (lastParaType.isArray()
- && (argumentList.size()>para.length || argumentList.size()==para.length-1 || !lastIsArray(argumentList, para.length-1))) {
- int stackLen = operandStack.getStackLength()+argumentList.size();
+ if (lastParaType.isArray() && (argumentList.size() > para.length
+ || argumentList.size() == para.length - 1 || !lastIsArray(argumentList, para.length - 1))) {
+ int stackLen = operandStack.getStackLength() + argumentList.size();
MethodVisitor mv = controller.getMethodVisitor();
- //mv = new org.objectweb.asm.util.TraceMethodVisitor(mv);
controller.setMethodVisitor(mv);
// varg call
// first parameters as usual
- for (int i = 0; i < para.length-1; i++) {
+ for (int i = 0, n = para.length - 1; i < n; i += 1) {
argumentList.get(i).visit(acg);
operandStack.doGroovyCast(para[i].getType());
}
// last parameters wrapped in an array
- List<Expression> lastParams = new LinkedList<Expression>();
- for (int i=para.length-1; i<argumentList.size();i++) {
+ List<Expression> lastParams = new LinkedList<>();
+ for (int i = para.length - 1, n = argumentList.size(); i < n; i += 1) {
lastParams.add(argumentList.get(i));
}
ArrayExpression array = new ArrayExpression(
@@ -271,31 +245,25 @@ public class InvocationWriter {
);
array.visit(acg);
// adjust stack length
- while (operandStack.getStackLength()<stackLen) {
+ while (operandStack.getStackLength() < stackLen) {
operandStack.push(ClassHelper.OBJECT_TYPE);
}
- if (argumentList.size()==para.length-1) {
+ if (argumentList.size() == para.length - 1) {
operandStack.remove(1);
}
} else {
- for (int i = 0; i < argumentList.size(); i++) {
+ for (int i = 0, n = argumentList.size(); i < n; i += 1) {
argumentList.get(i).visit(acg);
operandStack.doGroovyCast(para[i].getType());
}
}
}
- protected boolean makeDirectCall(
- Expression origin, Expression receiver,
- Expression message, Expression arguments,
- MethodCallerMultiAdapter adapter,
- boolean implicitThis, boolean containsSpreadExpression
- ) {
+ protected boolean makeDirectCall(Expression origin, Expression receiver, Expression message, Expression arguments, MethodCallerMultiAdapter adapter, boolean implicitThis, boolean containsSpreadExpression) {
if (makeClassForNameCall(origin, receiver, message, arguments)) return true;
// optimization path
- boolean fittingAdapter = adapter == invokeMethodOnCurrent ||
- adapter == invokeStaticMethod;
+ boolean fittingAdapter = adapter == invokeMethodOnCurrent || adapter == invokeStaticMethod;
if (fittingAdapter && controller.optimizeForInt && controller.isFastPath()) {
String methodName = getMethodName(message);
if (methodName != null) {
@@ -306,10 +274,10 @@ public class InvocationWriter {
args = new TupleExpression(receiver);
}
- StatementMeta meta = null;
- if (origin!=null) meta = origin.getNodeMetaData(StatementMeta.class);
+ OptimizingStatementWriter.StatementMeta meta = null;
+ if (origin != null) meta = origin.getNodeMetaData(OptimizingStatementWriter.StatementMeta.class);
MethodNode mn = null;
- if (meta!=null) mn = meta.target;
+ if (meta != null) mn = meta.target;
if (writeDirectMethodCall(mn, true, null, args)) return true;
}
@@ -324,39 +292,23 @@ public class InvocationWriter {
return false;
}
- protected boolean makeCachedCall(
- Expression origin, ClassExpression sender,
- Expression receiver, Expression message, Expression arguments,
- MethodCallerMultiAdapter adapter,
- boolean safe, boolean spreadSafe, boolean implicitThis,
- boolean containsSpreadExpression
- ) {
+ protected boolean makeCachedCall(Expression origin, ClassExpression sender, Expression receiver, Expression message, Expression arguments, MethodCallerMultiAdapter adapter, boolean safe, boolean spreadSafe, boolean implicitThis, boolean containsSpreadExpression) {
// prepare call site
if ((adapter == invokeMethod || adapter == invokeMethodOnCurrent || adapter == invokeStaticMethod) && !spreadSafe) {
String methodName = getMethodName(message);
-
if (methodName != null) {
- controller.getCallSiteWriter().makeCallSite(
- receiver, methodName, arguments, safe, implicitThis,
- adapter == invokeMethodOnCurrent,
- adapter == invokeStaticMethod);
+ controller.getCallSiteWriter().makeCallSite(receiver, methodName, arguments, safe, implicitThis, adapter == invokeMethodOnCurrent, adapter == invokeStaticMethod);
return true;
}
}
return false;
}
-
- protected void makeUncachedCall(
- Expression origin, ClassExpression sender,
- Expression receiver, Expression message, Expression arguments,
- MethodCallerMultiAdapter adapter,
- boolean safe, boolean spreadSafe, boolean implicitThis,
- boolean containsSpreadExpression
- ) {
+
+ protected void makeUncachedCall(Expression origin, ClassExpression sender, Expression receiver, Expression message, Expression arguments, MethodCallerMultiAdapter adapter, boolean safe, boolean spreadSafe, boolean implicitThis, boolean containsSpreadExpression) {
OperandStack operandStack = controller.getOperandStack();
CompileStack compileStack = controller.getCompileStack();
AsmClassGenerator acg = controller.getAcg();
-
+
// ensure VariableArguments are read, not stored
compileStack.pushLHS(false);
@@ -371,20 +323,19 @@ public class InvocationWriter {
if (adapter == invokeMethodOnSuper && methodName != null) {
controller.getSuperMethodNames().add(methodName);
}
-
+
// receiver
compileStack.pushImplicitThis(implicitThis);
receiver.visit(acg);
operandStack.box();
compileStack.popImplicitThis();
-
-
+
int operandsToRemove = 2;
// message
if (message != null) {
message.visit(acg);
operandStack.box();
- operandsToRemove++;
+ operandsToRemove += 1;
}
// arguments
@@ -399,7 +350,7 @@ public class InvocationWriter {
} else if (numberOfArguments > 0) {
operandsToRemove += numberOfArguments;
TupleExpression te = (TupleExpression) arguments;
- for (int i = 0; i < numberOfArguments; i++) {
+ for (int i = 0; i < numberOfArguments; i += 1) {
Expression argument = te.getExpression(i);
argument.visit(acg);
operandStack.box();
@@ -407,19 +358,14 @@ public class InvocationWriter {
}
}
- if (adapter==null) adapter = invokeMethod;
+ if (adapter == null) adapter = invokeMethod;
adapter.call(controller.getMethodVisitor(), numberOfArguments, safe, spreadSafe);
compileStack.popLHS();
- operandStack.replace(ClassHelper.OBJECT_TYPE,operandsToRemove);
- }
-
- protected void makeCall(
- Expression origin, ClassExpression sender,
- Expression receiver, Expression message, Expression arguments,
- MethodCallerMultiAdapter adapter,
- boolean safe, boolean spreadSafe, boolean implicitThis
- ) {
+ operandStack.replace(ClassHelper.OBJECT_TYPE, operandsToRemove);
+ }
+
+ protected void makeCall(Expression origin, ClassExpression sender, Expression receiver, Expression message, Expression arguments, MethodCallerMultiAdapter adapter, boolean safe, boolean spreadSafe, boolean implicitThis) {
// direct method call paths
boolean containsSpreadExpression = AsmClassGenerator.containsSpreadExpression(arguments);
@@ -435,18 +381,18 @@ public class InvocationWriter {
/**
* if Class.forName(x) is recognized, make a direct method call
*/
- protected boolean makeClassForNameCall(Expression origin, Expression receiver, Expression message, Expression arguments) {
- if (! (receiver instanceof ClassExpression)) return false;
+ protected boolean makeClassForNameCall(final Expression origin, final Expression receiver, final Expression message, final Expression arguments) {
+ if (!(receiver instanceof ClassExpression)) return false;
ClassExpression ce = (ClassExpression) receiver;
if (!ClassHelper.CLASS_Type.equals(ce.getType())) return false;
String msg = getMethodName(message);
if (!"forName".equals(msg)) return false;
ArgumentListExpression ae = makeArgumentList(arguments);
- if (ae.getExpressions().size()!=1) return false;
- return writeDirectMethodCall(CLASS_FOR_NAME_STRING,false, receiver, ae);
+ if (ae.getExpressions().size() != 1) return false;
+ return writeDirectMethodCall(CLASS_FOR_NAME_STRING, false, receiver, ae);
}
- public static ArgumentListExpression makeArgumentList(Expression arguments) {
+ public static ArgumentListExpression makeArgumentList(final Expression arguments) {
ArgumentListExpression ae;
if (arguments instanceof ArgumentListExpression) {
ae = (ArgumentListExpression) arguments;
@@ -460,14 +406,15 @@ public class InvocationWriter {
return ae;
}
- protected String getMethodName(Expression message) {
+ protected String getMethodName(final Expression message) {
String methodName = null;
if (message instanceof CastExpression) {
CastExpression msg = (CastExpression) message;
if (msg.getType() == ClassHelper.STRING_TYPE) {
final Expression methodExpr = msg.getExpression();
- if (methodExpr instanceof ConstantExpression)
- methodName = methodExpr.getText();
+ if (methodExpr instanceof ConstantExpression) {
+ methodName = methodExpr.getText();
+ }
}
}
@@ -478,28 +425,6 @@ public class InvocationWriter {
return methodName;
}
- private boolean isFunctionInterfaceCall(MethodCallExpression call) {
- if (!"call".equals(call.getMethodAsString())) {
- return false;
- }
-
- Expression objectExpression = call.getObjectExpression();
-
- if (null == objectExpression) {
- return false;
- }
-
- if (AsmClassGenerator.isThisExpression(objectExpression)) {
- return false;
- }
-
- if (ClassHelper.isFunctionalInterface(objectExpression.getType())) {
- return true;
- }
-
- return false;
- }
-
public void writeInvokeMethod(MethodCallExpression call) {
if (isClosureCall(call)) {
// let's invoke the closure method
@@ -508,54 +433,62 @@ public class InvocationWriter {
if (isFunctionInterfaceCall(call)) {
call = transformToRealMethodCall(call);
}
-
- boolean isSuperMethodCall = usesSuper(call);
MethodCallerMultiAdapter adapter = invokeMethod;
- if (isSuperMethodCall && call.isSafe()) {
- // safe is not necessary here because "super" is always not null
- // but keeping the flag would trigger a VerifyError (see GROOVY-6045)
- call.setSafe(false);
+ Expression objectExpression = call.getObjectExpression();
+ if (AsmClassGenerator.isSuperExpression(objectExpression)) {
+ adapter = invokeMethodOnSuper;
+ } else if (AsmClassGenerator.isThisExpression(objectExpression)) {
+ adapter = invokeMethodOnCurrent;
+ }
+ if (isStaticInvocation(call)) {
+ adapter = invokeStaticMethod;
+ }
+ Expression messageName = new CastExpression(ClassHelper.STRING_TYPE, call.getMethod());
+ makeCall(call, objectExpression, messageName, call.getArguments(), adapter, call.isSafe(), call.isSpreadSafe(), call.isImplicitThis());
+ }
+ }
+
+ private static boolean isFunctionInterfaceCall(final MethodCallExpression call) {
+ if ("call".equals(call.getMethodAsString())) {
+ Expression objectExpression = call.getObjectExpression();
+ if (!AsmClassGenerator.isThisExpression(objectExpression)) {
+ return ClassHelper.isFunctionalInterface(objectExpression.getType());
}
- if (AsmClassGenerator.isThisExpression(call.getObjectExpression())) adapter = invokeMethodOnCurrent;
- if (isSuperMethodCall) adapter = invokeMethodOnSuper;
- if (isStaticInvocation(call)) adapter = invokeStaticMethod;
- makeInvokeMethodCall(call, isSuperMethodCall, adapter);
}
+ return false;
}
- private MethodCallExpression transformToRealMethodCall(MethodCallExpression call) {
+ private static MethodCallExpression transformToRealMethodCall(MethodCallExpression call) {
ClassNode type = call.getObjectExpression().getType();
- final MethodNode methodNode = ClassHelper.findSAM(type);
+ MethodNode methodNode = ClassHelper.findSAM(type);
call = (MethodCallExpression) call.transformExpression(expression -> {
if (!(expression instanceof ConstantExpression)) {
return expression;
}
-
return new ConstantExpression(methodNode.getName());
});
call.setMethodTarget(methodNode);
return call;
}
- private boolean isClosureCall(MethodCallExpression call) {
+ private boolean isClosureCall(final MethodCallExpression call) {
// are we a local variable?
// it should not be an explicitly "this" qualified method call
// and the current class should have a possible method
-
ClassNode classNode = controller.getClassNode();
String methodName = call.getMethodAsString();
- if (methodName==null) return false;
+ if (methodName == null) return false;
if (!call.isImplicitThis()) return false;
if (!AsmClassGenerator.isThisExpression(call.getObjectExpression())) return false;
FieldNode field = classNode.getDeclaredField(methodName);
if (field == null) return false;
if (isStaticInvocation(call) && !field.isStatic()) return false;
Expression arguments = call.getArguments();
- return ! classNode.hasPossibleMethod(methodName, arguments);
+ return !classNode.hasPossibleMethod(methodName, arguments);
}
- private void invokeClosure(Expression arguments, String methodName) {
+ private void invokeClosure(final Expression arguments, final String methodName) {
AsmClassGenerator acg = controller.getAcg();
acg.visitVariableExpression(new VariableExpression(methodName));
controller.getOperandStack().box();
@@ -568,66 +501,52 @@ public class InvocationWriter {
controller.getOperandStack().replace(ClassHelper.OBJECT_TYPE);
}
- private boolean isStaticInvocation(MethodCallExpression call) {
+ private boolean isStaticInvocation(final MethodCallExpression call) {
if (!AsmClassGenerator.isThisExpression(call.getObjectExpression())) return false;
if (controller.isStaticMethod()) return true;
return controller.isStaticContext() && !call.isImplicitThis();
}
-
- private static boolean usesSuper(MethodCallExpression call) {
- Expression expression = call.getObjectExpression();
- if (expression instanceof VariableExpression) {
- VariableExpression varExp = (VariableExpression) expression;
- String variable = varExp.getName();
- return variable.equals("super");
- }
- return false;
- }
- public void writeInvokeStaticMethod(StaticMethodCallExpression call) {
- makeCall(call,
- new ClassExpression(call.getOwnerType()),
- new ConstantExpression(call.getMethod()),
- call.getArguments(),
- InvocationWriter.invokeStaticMethod,
- false, false, false);
+ public void writeInvokeStaticMethod(final StaticMethodCallExpression call) {
+ Expression receiver = new ClassExpression(call.getOwnerType());
+ Expression messageName = new ConstantExpression(call.getMethod());
+ makeCall(call, receiver, messageName, call.getArguments(), InvocationWriter.invokeStaticMethod, false, false, false);
}
-
- private boolean writeDirectConstructorCall(ConstructorCallExpression call) {
+
+ private boolean writeDirectConstructorCall(final ConstructorCallExpression call) {
if (!controller.isFastPath()) return false;
-
- StatementMeta meta = call.getNodeMetaData(StatementMeta.class);
+
+ OptimizingStatementWriter.StatementMeta meta = call.getNodeMetaData(OptimizingStatementWriter.StatementMeta.class);
ConstructorNode cn = null;
- if (meta!=null) cn = (ConstructorNode) meta.target;
- if (cn==null) return false;
-
+ if (meta != null) cn = (ConstructorNode) meta.target;
+ if (cn == null) return false;
+
String ownerDescriptor = prepareConstructorCall(cn);
TupleExpression args = makeArgumentList(call.getArguments());
loadArguments(args.getExpressions(), cn.getParameters());
finnishConstructorCall(cn, ownerDescriptor, args.getExpressions().size());
-
+
return true;
}
-
- protected String prepareConstructorCall(ConstructorNode cn) {
+
+ protected String prepareConstructorCall(final ConstructorNode cn) {
String owner = BytecodeHelper.getClassInternalName(cn.getDeclaringClass());
MethodVisitor mv = controller.getMethodVisitor();
-
mv.visitTypeInsn(NEW, owner);
mv.visitInsn(DUP);
return owner;
}
-
- protected void finnishConstructorCall(ConstructorNode cn, String ownerDescriptor, int argsToRemove) {
+
+ protected void finnishConstructorCall(final ConstructorNode cn, final String ownerDescriptor, final int argsToRemove) {
String desc = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, cn.getParameters());
MethodVisitor mv = controller.getMethodVisitor();
mv.visitMethodInsn(INVOKESPECIAL, ownerDescriptor, "<init>", desc, false);
-
+
controller.getOperandStack().remove(argsToRemove);
controller.getOperandStack().push(cn.getDeclaringClass());
}
- protected void writeNormalConstructorCall(ConstructorCallExpression call) {
+ protected void writeNormalConstructorCall(final ConstructorCallExpression call) {
Expression arguments = call.getArguments();
if (arguments instanceof TupleExpression) {
TupleExpression tupleExpression = (TupleExpression) arguments;
@@ -637,36 +556,33 @@ public class InvocationWriter {
}
}
- Expression receiverClass = new ClassExpression(call.getType());
- controller.getCallSiteWriter().makeCallSite(
- receiverClass, CallSiteWriter.CONSTRUCTOR,
- arguments, false, false, false,
- false);
+ Expression receiver = new ClassExpression(call.getType());
+ controller.getCallSiteWriter().makeCallSite(receiver, CallSiteWriter.CONSTRUCTOR, arguments, false, false, false, false);
}
-
- public void writeInvokeConstructor(ConstructorCallExpression call) {
+
+ public void writeInvokeConstructor(final ConstructorCallExpression call) {
if (writeDirectConstructorCall(call)) return;
if (writeAICCall(call)) return;
writeNormalConstructorCall(call);
}
- protected boolean writeAICCall(ConstructorCallExpression call) {
+ protected boolean writeAICCall(final ConstructorCallExpression call) {
if (!call.isUsingAnonymousInnerClass()) return false;
ConstructorNode cn = call.getType().getDeclaredConstructors().get(0);
OperandStack os = controller.getOperandStack();
-
+
String ownerDescriptor = prepareConstructorCall(cn);
-
+
List<Expression> args = makeArgumentList(call.getArguments()).getExpressions();
Parameter[] params = cn.getParameters();
// if a this appears as parameter here, then it should be
- // not static, unless we are in a static method. But since
+ // not static, unless we are in a static method. But since
// ACG#visitVariableExpression does the opposite for this case, we
// push here an explicit this. This should not have any negative effect
// sine visiting a method call or property with implicit this will push
// a new value for this again.
controller.getCompileStack().pushImplicitThis(true);
- for (int i=0; i<params.length; i++) {
+ for (int i = 0, n = params.length; i < n; i += 1) {
Parameter p = params[i];
Expression arg = args.get(i);
if (arg instanceof VariableExpression) {
@@ -681,8 +597,8 @@ public class InvocationWriter {
finnishConstructorCall(cn, ownerDescriptor, args.size());
return true;
}
-
- private void loadVariableWithReference(VariableExpression var) {
+
+ private void loadVariableWithReference(final VariableExpression var) {
if (!var.isUseReferenceDirectly()) {
var.visit(controller.getAcg());
} else {
@@ -690,11 +606,11 @@ public class InvocationWriter {
}
}
- public final void makeSingleArgumentCall(Expression receiver, String message, Expression arguments) {
+ public final void makeSingleArgumentCall(final Expression receiver, final String message, final Expression arguments) {
makeSingleArgumentCall(receiver, message, arguments, false);
}
- public void makeSingleArgumentCall(Expression receiver, String message, Expression arguments, boolean safe) {
+ public void makeSingleArgumentCall(final Expression receiver, final String message, final Expression arguments, final boolean safe) {
controller.getCallSiteWriter().makeSingleArgumentCall(receiver, message, arguments, safe);
}
@@ -704,7 +620,7 @@ public class InvocationWriter {
controller.getCompileStack().pop();
}
- private void visitSpecialConstructorCall(ConstructorCallExpression call) {
+ private void visitSpecialConstructorCall(final ConstructorCallExpression call) {
if (controller.getClosureWriter().addGeneratedClosureConstructorCall(call)) return;
ClassNode callNode = controller.getClassNode();
if (call.isSuperCall()) callNode = callNode.getSuperClass();
@@ -714,21 +630,18 @@ public class InvocationWriter {
}
}
- private static List<ConstructorNode> sortConstructors(ConstructorCallExpression call, ClassNode callNode) {
+ private static List<ConstructorNode> sortConstructors(final ConstructorCallExpression call, final ClassNode callNode) {
// sort in a new list to prevent side effects
- List<ConstructorNode> constructors = new ArrayList<ConstructorNode>(callNode.getDeclaredConstructors());
- Comparator comp = (arg0, arg1) -> {
- ConstructorNode c0 = (ConstructorNode) arg0;
- ConstructorNode c1 = (ConstructorNode) arg1;
+ List<ConstructorNode> constructors = new ArrayList<>(callNode.getDeclaredConstructors());
+ constructors.sort((c0, c1) -> {
String descriptor0 = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, c0.getParameters());
String descriptor1 = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, c1.getParameters());
return descriptor0.compareTo(descriptor1);
- };
- constructors.sort(comp);
+ });
return constructors;
}
- private boolean makeDirectConstructorCall(List<ConstructorNode> constructors, ConstructorCallExpression call, ClassNode callNode) {
+ private boolean makeDirectConstructorCall(final List<ConstructorNode> constructors, final ConstructorCallExpression call, final ClassNode callNode) {
if (!controller.isConstructor()) return false;
Expression arguments = call.getArguments();
@@ -744,13 +657,13 @@ public class InvocationWriter {
}
ConstructorNode cn = getMatchingConstructor(constructors, argumentList);
- if (cn==null) return false;
+ if (cn == null) return false;
MethodVisitor mv = controller.getMethodVisitor();
OperandStack operandStack = controller.getOperandStack();
Parameter[] params = cn.getParameters();
mv.visitVarInsn(ALOAD, 0);
- for (int i=0; i<params.length; i++) {
+ for (int i = 0, n = params.length; i < n; i += 1) {
Expression expression = argumentList.get(i);
expression.visit(controller.getAcg());
if (!AsmClassGenerator.isNullConstant(expression)) {
@@ -764,7 +677,7 @@ public class InvocationWriter {
return true;
}
- private void makeMOPBasedConstructorCall(List<ConstructorNode> constructors, ConstructorCallExpression call, ClassNode callNode) {
+ private void makeMOPBasedConstructorCall(final List<ConstructorNode> constructors, final ConstructorCallExpression call, final ClassNode callNode) {
MethodVisitor mv = controller.getMethodVisitor();
OperandStack operandStack = controller.getOperandStack();
call.getArguments().visit(controller.getAcg());
@@ -786,21 +699,21 @@ public class InvocationWriter {
mv.visitTypeInsn(NEW, BytecodeHelper.getClassInternalName(callNode));
}
mv.visitInsn(SWAP);
- TreeMap<Integer,ConstructorNode> sortedConstructors = new TreeMap<Integer, ConstructorNode>();
+ TreeMap<Integer,ConstructorNode> sortedConstructors = new TreeMap<>();
for (ConstructorNode constructor : constructors) {
String typeDescriptor = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, constructor.getParameters());
int hash = BytecodeHelper.hashCode(typeDescriptor);
ConstructorNode sameHashNode = sortedConstructors.put(hash, constructor);
- if (sameHashNode!=null) {
- controller.getSourceUnit().addError(
- new SyntaxException("Unable to compile class "+controller.getClassNode().getName() + " due to hash collision in constructors", call.getLineNumber(), call.getColumnNumber()));
+ if (sameHashNode != null) {
+ controller.getSourceUnit().addError(new SyntaxException(
+ "Unable to compile class "+controller.getClassNode().getName() + " due to hash collision in constructors", call.getLineNumber(), call.getColumnNumber()));
}
}
Label[] targets = new Label[constructors.size()];
int[] indices = new int[constructors.size()];
Iterator<Integer> hashIt = sortedConstructors.keySet().iterator();
Iterator<ConstructorNode> constructorIt = sortedConstructors.values().iterator();
- for (int i = 0; i < targets.length; i++) {
+ for (int i = 0, n = targets.length; i < n; i += 1) {
targets[i] = new Label();
indices[i] = hashIt.next();
}
@@ -845,9 +758,9 @@ public class InvocationWriter {
Parameter[] parameters = cn.getParameters();
int lengthWithoutVargs = parameters.length;
if (parameters.length > 0 && parameters[parameters.length - 1].getType().isArray()) {
- lengthWithoutVargs--;
+ lengthWithoutVargs -= 1;
}
- for (int p = 0; p < lengthWithoutVargs; p++) {
+ for (int p = 0; p < lengthWithoutVargs; p += 1) {
loadAndCastElement(operandStack, mv, parameters, p);
}
if (parameters.length > lengthWithoutVargs) {
@@ -891,7 +804,7 @@ public class InvocationWriter {
mv.visitInsn(POP);
}
- private static void loadAndCastElement(OperandStack operandStack, MethodVisitor mv, Parameter[] parameters, int p) {
+ private static void loadAndCastElement(final OperandStack operandStack, final MethodVisitor mv, final Parameter[] parameters, final int p) {
operandStack.push(ClassHelper.OBJECT_TYPE);
mv.visitInsn(DUP);
BytecodeHelper.pushConstant(mv, p);
@@ -904,7 +817,7 @@ public class InvocationWriter {
}
// we match only on the number of arguments, not anything else
- private static ConstructorNode getMatchingConstructor(List<ConstructorNode> constructors, List<Expression> argumentList) {
+ private static ConstructorNode getMatchingConstructor(final List<ConstructorNode> constructors, final List<Expression> argumentList) {
ConstructorNode lastMatch = null;
for (ConstructorNode cn : constructors) {
Parameter[] params = cn.getParameters();
@@ -921,7 +834,7 @@ public class InvocationWriter {
}
/**
- * This converts sourceType to a non primitive by using Groovy casting.
+ * Converts sourceType to a non primitive by using Groovy casting.
* sourceType might be a primitive
* This might be done using SBA#castToType
*/
@@ -946,12 +859,12 @@ public class InvocationWriter {
}
}
- public void castNonPrimitiveToBool(ClassNode last) {
+ public void castNonPrimitiveToBool(final ClassNode last) {
MethodVisitor mv = controller.getMethodVisitor();
BytecodeHelper.unbox(mv, ClassHelper.boolean_TYPE);
}
- public void coerce(ClassNode from, ClassNode target) {
+ public void coerce(final ClassNode from, final ClassNode target) {
if (from.isDerivedFrom(target)) return;
MethodVisitor mv = controller.getMethodVisitor();
OperandStack os = controller.getOperandStack();
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index 42f2fa4..e6abdaa 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -86,6 +86,7 @@ import static org.objectweb.asm.Opcodes.IFNULL;
import static org.objectweb.asm.Opcodes.INVOKESTATIC;
public class StaticInvocationWriter extends InvocationWriter {
+
private static final ClassNode INVOKERHELPER_CLASSNODE = ClassHelper.make(InvokerHelper.class);
private static final Expression INVOKERHELPER_RECEIVER = new ClassExpression(INVOKERHELPER_CLASSNODE);
private static final MethodNode INVOKERHELPER_INVOKEMETHOD = INVOKERHELPER_CLASSNODE.getMethod(
@@ -654,14 +655,12 @@ public class StaticInvocationWriter extends InvocationWriter {
mv.visitInsn(ACONST_NULL);
mv.visitLabel(endof);
} else {
- if ((adapter == AsmClassGenerator.getGroovyObjectField
- || adapter == AsmClassGenerator.getField ) && origin instanceof AttributeExpression) {
- String pname = ((PropertyExpression) origin).getPropertyAsString();
+ if (origin instanceof AttributeExpression && (adapter == AsmClassGenerator.getField || adapter == AsmClassGenerator.getGroovyObjectField)) {
CallSiteWriter callSiteWriter = controller.getCallSiteWriter();
- if (pname!=null && callSiteWriter instanceof StaticTypesCallSiteWriter) {
- StaticTypesCallSiteWriter stcsw = (StaticTypesCallSiteWriter) callSiteWriter;
- TypeChooser typeChooser = controller.getTypeChooser();
- if (stcsw.makeGetField(receiver, typeChooser.resolveType(receiver, controller.getClassNode()), pname, safe, false)) {
+ String fieldName = ((AttributeExpression) origin).getPropertyAsString();
+ if (fieldName != null && callSiteWriter instanceof StaticTypesCallSiteWriter) {
+ ClassNode receiverType = controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
+ if (((StaticTypesCallSiteWriter) callSiteWriter).makeGetField(receiver, receiverType, fieldName, safe, false)) {
return;
}
}
[groovy] 10/20: GROOVY-8762: make WriterController#isInClosure()
more selective
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit d62a3f028b5a8abce20d6a3717c9c6c012adfddd
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
(cherry picked from commit 3cbc7e3170330d250dc5a2036f910dbf15664199)
---
.../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
'''
}
}
[groovy] 06/20: refactor to remove repeated creation of Class
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 4d4be630e784a02b01852ce410f34de5aea202ea
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 25 10:52:53 2019 -0600
refactor to remove repeated creation of Class<Type>
(cherry picked from commit 7127bb5b3f120ddaffa6630adb436ce68117ed04)
---
.../transform/stc/StaticTypeCheckingVisitor.java | 39 +++++++---------------
1 file changed, 12 insertions(+), 27 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index bc36c28..cb5dd85 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -4913,42 +4913,27 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
private ClassNode getGenericsResolvedTypeOfFieldOrProperty(final AnnotatedNode an, final ClassNode type) {
if (!type.isUsingGenerics()) return type;
Map<GenericsTypeName, GenericsType> connections = new HashMap<>();
- //TODO: inner classes mean a different this-type. This is ignored here!
+ // TODO: inner classes mean a different this-type. This is ignored here!
extractGenericsConnections(connections, typeCheckingContext.getEnclosingClassNode(), an.getDeclaringClass());
return applyGenericsContext(connections, type);
}
- private ClassNode makeSuper() {
- ClassNode ret = typeCheckingContext.getEnclosingClassNode().getSuperClass();
- if (typeCheckingContext.isInStaticContext) {
- ClassNode staticRet = CLASS_Type.getPlainNodeReference();
- GenericsType gt = new GenericsType(ret);
- staticRet.setGenericsTypes(new GenericsType[]{gt});
- ret = staticRet;
+ private static ClassNode makeSelf(final ClassNode trait) {
+ ClassNode selfType = trait;
+ Set<ClassNode> selfTypes = Traits.collectSelfTypes(selfType, new LinkedHashSet<>());
+ if (!selfTypes.isEmpty()) {
+ selfTypes.add(selfType);
+ selfType = new UnionTypeClassNode(selfTypes.toArray(ClassNode.EMPTY_ARRAY));
}
- return ret;
+ return selfType;
}
- private ClassNode makeThis() {
- ClassNode ret = typeCheckingContext.getEnclosingClassNode();
- if (typeCheckingContext.isInStaticContext) {
- ClassNode staticRet = CLASS_Type.getPlainNodeReference();
- GenericsType gt = new GenericsType(ret);
- staticRet.setGenericsTypes(new GenericsType[]{gt});
- ret = staticRet;
- }
- return ret;
+ private ClassNode makeSuper() {
+ return makeType(typeCheckingContext.getEnclosingClassNode().getSuperClass(), typeCheckingContext.isInStaticContext);
}
- private static ClassNode makeSelf(final ClassNode trait) {
- ClassNode ret = trait;
- LinkedHashSet<ClassNode> selfTypes = new LinkedHashSet<>();
- Traits.collectSelfTypes(ret, selfTypes);
- if (!selfTypes.isEmpty()) {
- selfTypes.add(ret);
- ret = new UnionTypeClassNode(selfTypes.toArray(ClassNode.EMPTY_ARRAY));
- }
- return ret;
+ private ClassNode makeThis() {
+ return makeType(typeCheckingContext.getEnclosingClassNode(), typeCheckingContext.isInStaticContext);
}
/**