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 2020/12/16 15:07:04 UTC
[groovy] branch master updated: GROOVY-9373 (part 1): rework line
numbers for statements
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 703b644 GROOVY-9373 (part 1): rework line numbers for statements
703b644 is described below
commit 703b6446399b310edb8aa2807ef01db51b3e19b0
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'
+ ])
+ }
}