You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2020/12/14 02:54:47 UTC
[groovy] branch GROOVY-9373 created (now 2df1ef3)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-9373
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at 2df1ef3 GROOVY-9373 (part 1): rework line numbers for statements
This branch includes the following new commits:
new 2df1ef3 GROOVY-9373 (part 1): rework line numbers for statements
The 1 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.
[groovy] 01/01: GROOVY-9373 (part 1): rework line numbers for
statements
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9373
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 2df1ef3f27de662017036b960f85b6d48cda940a
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Dec 13 20:04:31 2020 -0600
GROOVY-9373 (part 1): rework line numbers for statements
GROOVY-7647, GROOVY-9126, GROOVY-9199
"do" and "try" should not generate line numbers
instructions after last source line are tied to closing brace
instructions before first source line are tied to opening brace
---
.../groovy/classgen/AsmClassGenerator.java | 98 +++++++++++-----------
.../org/codehaus/groovy/classgen/ReturnAdder.java | 4 +-
.../groovy/classgen/asm/StatementWriter.java | 27 +-----
.../groovy/classgen/asm/WriterController.java | 18 +++-
src/test/groovy/bugs/Groovy8289Bug.groovy | 29 ++++++-
5 files changed, 93 insertions(+), 83 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 1d04b1d..ae99861 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -424,35 +424,34 @@ public class AsmClassGenerator extends ClassGenerator {
@Override
protected void visitConstructorOrMethod(final MethodNode node, final boolean isConstructor) {
- controller.resetLineNumber();
Parameter[] parameters = node.getParameters();
- String methodType = BytecodeHelper.getMethodDescriptor(node.getReturnType(), parameters);
- String signature = BytecodeHelper.getGenericsMethodSignature(node);
- int modifiers = node.getModifiers();
- if (isVargs(node.getParameters())) modifiers |= ACC_VARARGS;
- MethodVisitor mv = classVisitor.visitMethod(modifiers, node.getName(), methodType, signature, buildExceptions(node.getExceptions()));
+ MethodVisitor mv = classVisitor.visitMethod(
+ node.getModifiers() | (isVargs(parameters) ? ACC_VARARGS : 0), node.getName(),
+ BytecodeHelper.getMethodDescriptor(node.getReturnType(), parameters),
+ BytecodeHelper.getGenericsMethodSignature(node),
+ buildExceptions(node.getExceptions()));
controller.setMethodVisitor(mv);
+ controller.resetLineNumber();
visitAnnotations(node, mv);
for (int i = 0, n = parameters.length; i < n; i += 1) {
visitParameterAnnotations(parameters[i], i, mv);
}
- // add parameter names to the MethodVisitor (jdk8+ only)
+ // add parameter names to the MethodVisitor (JDK8+)
if (Optional.ofNullable(controller.getClassNode().getCompileUnit())
.orElseGet(context::getCompileUnit).getConfig().getParameters()) {
for (Parameter parameter : parameters) {
- // TODO: handle ACC_SYNTHETIC for enum method parameters?
- mv.visitParameter(parameter.getName(), 0);
+ mv.visitParameter(parameter.getName(), parameter.getModifiers());
}
}
if (controller.getClassNode().isAnnotationDefinition() && !node.isStaticConstructor()) {
visitAnnotationDefault(node, mv);
} else if (!node.isAbstract()) {
- Statement code = node.getCode();
mv.visitCode();
+ Statement code = node.getCode();
BytecodeInstruction instruction; // fast path for getters, setters, etc.
if (code instanceof BytecodeSequence && (instruction = ((BytecodeSequence) code).getBytecodeInstruction()) != null) {
instruction.visit(mv);
@@ -465,17 +464,16 @@ public class AsmClassGenerator extends ClassGenerator {
} catch (Exception e) {
Writer writer = null;
if (mv instanceof TraceMethodVisitor) {
- TraceMethodVisitor tracer = (TraceMethodVisitor) mv;
writer = new StringBuilderWriter();
PrintWriter p = new PrintWriter(writer);
- tracer.p.print(p);
+ ((TraceMethodVisitor) mv).p.print(p);
p.flush();
}
StringBuilder message = new StringBuilder(64);
message.append("ASM reporting processing error for ");
- message.append(controller.getClassNode().toString()).append("#").append(node.getName());
+ message.append(controller.getClassNode().toString(false)).append('#').append(node.getName());
message.append(" with signature ").append(node.getTypeDescriptor());
- message.append(" in ").append(sourceFile).append(":").append(node.getLineNumber());
+ message.append(" in ").append(sourceFile).append(':').append(node.getLineNumber());
if (writer != null) {
message.append("\nLast known generated bytecode in last generated method or constructor:\n");
message.append(writer);
@@ -503,49 +501,55 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
if (!hasCallToSuper) {
- // invokes the super class constructor
+ if (code != null) { // GROOVY-9373
+ controller.visitLineNumber(code.getLineNumber());
+ }
+ // add call to "super()"
mv.visitVarInsn(ALOAD, 0);
mv.visitMethodInsn(INVOKESPECIAL, controller.getInternalBaseClassName(), "<init>", "()V", false);
}
}
- // handle body
- super.visitConstructorOrMethod(node, isConstructor);
-
- controller.getCompileStack().clear();
-
- if (checkIfLastStatementIsReturnOrThrow(code)) return;
+ if (code != null) {
+ code.visit(this);
+ }
- if (node.isVoidMethod()) {
- mv.visitInsn(RETURN);
- } else {
- ClassNode type = node.getReturnType();
- if (ClassHelper.isPrimitiveType(type)) {
- mv.visitLdcInsn(0);
- controller.getOperandStack().push(ClassHelper.int_TYPE);
- controller.getOperandStack().doGroovyCast(type);
- BytecodeHelper.doReturn(mv, type);
- controller.getOperandStack().remove(1);
+ if (!isLastStatementReturnOrThrow(code)) {
+ if (code != null) { // GROOVY-7647, GROOVY-9373
+ controller.visitLineNumber(code.getLastLineNumber());
+ }
+ if (node.isVoidMethod()) {
+ mv.visitInsn(RETURN);
} else {
- mv.visitInsn(ACONST_NULL);
- BytecodeHelper.doReturn(mv, type);
+ ClassNode type = node.getReturnType();
+ if (ClassHelper.isPrimitiveType(type)) {
+ mv.visitLdcInsn(Integer.valueOf(0));
+ controller.getOperandStack().push(ClassHelper.int_TYPE);
+ controller.getOperandStack().doGroovyCast(type);
+ BytecodeHelper.doReturn(mv, type);
+ controller.getOperandStack().remove(1);
+ } else {
+ mv.visitInsn(ACONST_NULL);
+ BytecodeHelper.doReturn(mv, type);
+ }
}
}
+
+ controller.getCompileStack().clear();
}
- private boolean checkIfLastStatementIsReturnOrThrow(Statement code) {
+ private boolean isLastStatementReturnOrThrow(final Statement code) {
if (code instanceof BlockStatement) {
BlockStatement blockStatement = (BlockStatement) code;
List<Statement> statementList = blockStatement.getStatements();
- int statementCnt = statementList.size();
- if (statementCnt > 0) {
- Statement lastStatement = statementList.get(statementCnt - 1);
+ int nStatements = statementList.size();
+ if (nStatements > 0) {
+ Statement lastStatement = statementList.get(nStatements - 1);
if (lastStatement instanceof ReturnStatement || lastStatement instanceof ThrowStatement) {
return true;
}
}
}
-
return false;
}
@@ -649,10 +653,10 @@ public class AsmClassGenerator extends ClassGenerator {
}
// GroovyCodeVisitor interface
- //-------------------------------------------------------------------------
+ //--------------------------------------------------------------------------
// Statements
- //-------------------------------------------------------------------------
+ //--------------------------------------------------------------------------
@Override
protected void visitStatement(final Statement statement) {
@@ -739,7 +743,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
// Expressions
- //-------------------------------------------------------------------------
+ //--------------------------------------------------------------------------
@Override
public void visitTernaryExpression(final TernaryExpression expression) {
@@ -2146,7 +2150,7 @@ public class AsmClassGenerator extends ClassGenerator {
}
// Implementation methods
- //-------------------------------------------------------------------------
+ //--------------------------------------------------------------------------
public static int argumentSize(final Expression arguments) {
if (arguments instanceof TupleExpression) {
@@ -2222,15 +2226,9 @@ public class AsmClassGenerator extends ClassGenerator {
currentASTNode = statement;
int line = statement.getLineNumber();
- if (line < 0 || (!ASM_DEBUG && line == controller.getLineNumber())) return;
+ if (!ASM_DEBUG && line == controller.getLineNumber()) return;
- controller.setLineNumber(line);
- MethodVisitor mv = controller.getMethodVisitor();
- if (mv != null) {
- Label l = new Label();
- mv.visitLabel(l);
- mv.visitLineNumber(line, l);
- }
+ controller.visitLineNumber(line);
}
public void throwException(final String message) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java b/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
index b206c16..983339f 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
@@ -100,11 +100,11 @@ public class ReturnAdder {
private Statement addReturnsIfNeeded(final Statement statement, final VariableScope scope) {
if (statement instanceof ReturnStatement || statement instanceof ThrowStatement
- || statement instanceof BytecodeSequence) {
+ || statement instanceof EmptyStatement || statement instanceof BytecodeSequence) {
return statement;
}
- if (statement instanceof EmptyStatement || statement == null) {
+ if (statement == null) {
ReturnStatement returnStatement = new ReturnStatement(nullX());
listener.returnStatementAdded(returnStatement);
return returnStatement;
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 e2505fc..daeefcd 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -20,7 +20,6 @@ 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.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.BooleanExpression;
import org.codehaus.groovy.ast.expr.ClosureListExpression;
@@ -94,25 +93,9 @@ public class StatementWriter {
}
compileStack.pop();
- // GROOVY-7647, GROOVY-9126
- if (block.getLastLineNumber() > 0 && !isMethodOrConstructorNonEmptyBlock(block)) {
- MethodVisitor mv = controller.getMethodVisitor();
- Label blockEnd = new Label();
- mv.visitLabel(blockEnd);
- mv.visitLineNumber(block.getLastLineNumber(), blockEnd);
- }
-
controller.getOperandStack().popDownTo(mark);
}
- private boolean isMethodOrConstructorNonEmptyBlock(final BlockStatement block) {
- MethodNode methodNode = controller.getMethodNode();
- if (methodNode == null) {
- methodNode = controller.getConstructorNode();
- }
- return (methodNode != null && methodNode.getCode() == block && !block.isEmpty());
- }
-
public void writeForStatement(final ForStatement statement) {
if (statement.getVariable() == ForStatement.FOR_LOOP_DUMMY) {
writeForLoopWithClosureList(statement);
@@ -293,15 +276,13 @@ public class StatementWriter {
}
public void writeDoWhileLoop(final DoWhileStatement statement) {
- controller.getAcg().onLineNumber(statement, "visitDoWhileLoop");
writeStatementLabel(statement);
- MethodVisitor mv = controller.getMethodVisitor();
-
controller.getCompileStack().pushLoop(statement.getStatementLabels());
Label continueLabel = controller.getCompileStack().getContinueLabel();
Label breakLabel = controller.getCompileStack().getBreakLabel();
+ MethodVisitor mv = controller.getMethodVisitor();
mv.visitLabel(continueLabel);
statement.getLoopBlock().visit(controller.getAcg());
@@ -335,7 +316,6 @@ public class StatementWriter {
}
public void writeTryCatchFinally(final TryCatchStatement statement) {
- controller.getAcg().onLineNumber(statement, "visitTryCatchFinally");
writeStatementLabel(statement);
MethodVisitor mv = controller.getMethodVisitor();
@@ -403,11 +383,6 @@ public class StatementWriter {
operandStack.push(ClassHelper.make(Throwable.class));
int anyThrowableIndex = compileStack.defineTemporaryVariable("throwable", true);
- // GROOVY-9199
- controller.resetLineNumber();
- int line = finallyStatement.getLineNumber();
- if (line > 0) mv.visitLineNumber(line, catchAll);
-
finallyStatement.visit(controller.getAcg());
// load the throwable and rethrow it
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 c17c471..dd8ba29 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
@@ -32,6 +32,7 @@ import org.codehaus.groovy.classgen.asm.util.LoggableClassVisitor;
import org.codehaus.groovy.control.CompilerConfiguration;
import org.codehaus.groovy.control.SourceUnit;
import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
@@ -389,12 +390,25 @@ public class WriterController {
return lineNumber;
}
+ public void resetLineNumber() {
+ setLineNumber(-1);
+ }
+
public void setLineNumber(final int lineNumber) {
this.lineNumber = lineNumber;
}
- public void resetLineNumber() {
- setLineNumber(-1);
+ public void visitLineNumber(final int lineNumber) {
+ if (lineNumber > 0 && lineNumber != this.lineNumber) {
+ setLineNumber(lineNumber);
+
+ MethodVisitor mv = getMethodVisitor();
+ if (mv != null) {
+ Label label = new Label();
+ mv.visitLabel(label);
+ mv.visitLineNumber(lineNumber, label);
+ }
+ }
}
public int getBytecodeVersion() {
diff --git a/src/test/groovy/bugs/Groovy8289Bug.groovy b/src/test/groovy/bugs/Groovy8289Bug.groovy
index 2091f66..378263a 100644
--- a/src/test/groovy/bugs/Groovy8289Bug.groovy
+++ b/src/test/groovy/bugs/Groovy8289Bug.groovy
@@ -21,8 +21,8 @@ package groovy.bugs
import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase
class Groovy8289Bug extends AbstractBytecodeTestCase {
- void testConstructorLineIndex() {
- def bytecode= compile(method:'<init>', '''
+ void testNoArgCtorLines() {
+ def bytecode = compile '''\
@groovy.transform.CompileStatic
class C {
String string
@@ -32,12 +32,35 @@ class Groovy8289Bug extends AbstractBytecodeTestCase {
println c
}
}
- ''')
+ ''', method:'<init>'
assert bytecode.hasStrictSequence([
'L0',
+ 'LINENUMBER 4 L0',
'ALOAD 0',
'INVOKESPECIAL java/lang/Object.<init> ()V'
])
}
+
+ // GROOVY-9199
+ void testTryFinallyLines() {
+ def bytecode = compile '''\
+ def m(p) {
+ }
+ void test() {
+ try {
+ m(true)
+ } finally {
+ m(false)
+ }
+ }
+ ''', method:'test'
+
+ assert bytecode.hasStrictSequence([
+ 'L0',
+ 'LINENUMBER 5 L0',
+ 'ALOAD 0',
+ 'ICONST_1'
+ ])
+ }
}