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/11/22 15:33:51 UTC
[groovy] branch GROOVY_2_5_X updated (4ba8777 -> 8820177)
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a change to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from 4ba8777 GROOVY-8775, GROOVY-9197: Ant: separate JVM and compilation classpaths
new b4d42b1 GROOVY-9126: Unreachable line numbers after ARETURN in bytecode (#1095)
new 8820177 Tweak GROOVY-9126: Remove unnecessary checks (#1096)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../groovy/classgen/asm/StatementWriter.java | 21 ++++++++++++++--
.../bugs/{Groovy9294.groovy => Groovy9126.groovy} | 28 ++++++++++++++--------
2 files changed, 37 insertions(+), 12 deletions(-)
copy src/test/groovy/bugs/{Groovy9294.groovy => Groovy9126.groovy} (62%)
[groovy] 01/02: GROOVY-9126: Unreachable line numbers after ARETURN
in bytecode (#1095)
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit b4d42b146fb348fae454e39289057e0a350a15df
Author: Daniel.Sun <su...@apache.org>
AuthorDate: Thu Nov 21 15:44:47 2019 +0800
GROOVY-9126: Unreachable line numbers after ARETURN in bytecode (#1095)
(cherry picked from commit ecb6c331edf08a0b41ac3ff4b63aae99b85a4b76)
---
.../groovy/classgen/asm/StatementWriter.java | 23 +++++++++-
src/test/groovy/bugs/Groovy9126.groovy | 51 ++++++++++++++++++++++
2 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
index 4872e4f..c2bff8c 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -20,6 +20,7 @@ package org.codehaus.groovy.classgen.asm;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.BooleanExpression;
@@ -88,13 +89,16 @@ public class StatementWriter {
int mark = controller.getOperandStack().getStackLength();
CompileStack compileStack = controller.getCompileStack();
compileStack.pushVariableScope(block.getVariableScope());
- for (Statement statement : block.getStatements()) {
+ List<Statement> statementList = block.getStatements();
+ for (Statement statement : statementList) {
statement.visit(controller.getAcg());
}
compileStack.pop();
// GROOVY-7647
- if (block.getLastLineNumber() > 0) {
+ if (block.getLastLineNumber() > 0
+ && !methodOrConstructorsLastReturnStatementExists(block) // GROOVY-9126
+ ) {
MethodVisitor mv = controller.getMethodVisitor();
Label blockEnd = new Label();
mv.visitLabel(blockEnd);
@@ -104,6 +108,21 @@ public class StatementWriter {
controller.getOperandStack().popDownTo(mark);
}
+ private boolean methodOrConstructorsLastReturnStatementExists(BlockStatement block) {
+ MethodNode methodNode = controller.getMethodNode();
+ if (null == methodNode) {
+ methodNode = controller.getConstructorNode();
+ }
+
+ if (null == methodNode || block != methodNode.getCode()) { // check if the block is method/constructor's code
+ return false;
+ }
+
+ List<Statement> statementList = block.getStatements();
+ final int size = statementList.size();
+ return size > 0 && statementList.get(size - 1) instanceof ReturnStatement;
+ }
+
public void writeForStatement(ForStatement loop) {
Parameter loopVar = loop.getVariable();
if (loopVar == ForStatement.FOR_LOOP_DUMMY) {
diff --git a/src/test/groovy/bugs/Groovy9126.groovy b/src/test/groovy/bugs/Groovy9126.groovy
new file mode 100644
index 0000000..3df9633
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9126.groovy
@@ -0,0 +1,51 @@
+/*
+ * 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.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+@CompileStatic
+final class Groovy9126 extends AbstractBytecodeTestCase {
+
+ @Test
+ void testUnreachableBytecode() {
+ def bytecode = compile([method:'nonVoidMethod'],'''
+ @groovy.transform.CompileStatic
+ int nonVoidMethod() {
+ 1 * 1
+ }
+ ''')
+
+ assert bytecode.hasStrictSequence(
+ ['public nonVoidMethod()I',
+ 'L0',
+ 'LINENUMBER 4 L0',
+ 'ICONST_1',
+ 'ICONST_1',
+ 'IMUL',
+ 'IRETURN',
+ 'L1',
+ 'FRAME FULL [] [java/lang/Throwable]']
+ )
+ }
+}
[groovy] 02/02: Tweak GROOVY-9126: Remove unnecessary checks (#1096)
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 8820177a9b407e9c943cd2b6acfc45b5ca2bd48d
Author: Daniel.Sun <su...@apache.org>
AuthorDate: Fri Nov 22 07:41:59 2019 +0800
Tweak GROOVY-9126: Remove unnecessary checks (#1096)
* Tweak GROOVY-9126: Remove unnecessary checks
* Check if block is empty
(cherry picked from commit d787ddd244a8e8c805d4941366faa1aa3f369c2c)
---
.../java/org/codehaus/groovy/classgen/asm/StatementWriter.java | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
index c2bff8c..d8b9182 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -97,7 +97,7 @@ public class StatementWriter {
// GROOVY-7647
if (block.getLastLineNumber() > 0
- && !methodOrConstructorsLastReturnStatementExists(block) // GROOVY-9126
+ && !isMethodOrConstructorNonEmptyBlock(block) // GROOVY-9126
) {
MethodVisitor mv = controller.getMethodVisitor();
Label blockEnd = new Label();
@@ -108,7 +108,7 @@ public class StatementWriter {
controller.getOperandStack().popDownTo(mark);
}
- private boolean methodOrConstructorsLastReturnStatementExists(BlockStatement block) {
+ private boolean isMethodOrConstructorNonEmptyBlock(BlockStatement block) {
MethodNode methodNode = controller.getMethodNode();
if (null == methodNode) {
methodNode = controller.getConstructorNode();
@@ -118,9 +118,7 @@ public class StatementWriter {
return false;
}
- List<Statement> statementList = block.getStatements();
- final int size = statementList.size();
- return size > 0 && statementList.get(size - 1) instanceof ReturnStatement;
+ return !block.getStatements().isEmpty();
}
public void writeForStatement(ForStatement loop) {