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 2022/12/19 17:09:29 UTC
[groovy] branch GROOVY_4_0_X updated (7fbc6044f9 -> ec3dc6d08b)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
from 7fbc6044f9 Check spread calls in advance
new cabea972bb classgen: `StringBuilder` for `assert` with message and general cleanup
new ec3dc6d08b GROOVY-10878: classgen: write `assert` using `throw` directly
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/AssertionWriter.java | 262 +++++++++++----------
.../org/codehaus/groovy/runtime/InvokerHelper.java | 13 +-
2 files changed, 147 insertions(+), 128 deletions(-)
[groovy] 02/02: GROOVY-10878: classgen: write `assert` using `throw` directly
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit ec3dc6d08b45d660abc49aa08eee3e5924b4cc09
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Dec 19 10:30:42 2022 -0600
GROOVY-10878: classgen: write `assert` using `throw` directly
---
.../org/codehaus/groovy/classgen/asm/AssertionWriter.java | 12 ++++++------
.../java/org/codehaus/groovy/runtime/InvokerHelper.java | 13 ++++++++++---
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/AssertionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/AssertionWriter.java
index e76b794134..c5cb1b71b1 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/AssertionWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/AssertionWriter.java
@@ -25,7 +25,6 @@ import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.stmt.AssertStatement;
import org.codehaus.groovy.control.Janitor;
-import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
import org.codehaus.groovy.runtime.powerassert.SourceText;
import org.codehaus.groovy.runtime.powerassert.SourceTextNotAvailableException;
import org.codehaus.groovy.syntax.Token;
@@ -48,8 +47,6 @@ import static org.objectweb.asm.Opcodes.NEW;
import static org.objectweb.asm.Opcodes.POP;
public class AssertionWriter {
- // assert
- private static final MethodCaller assertFailedMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "assertFailed");
private static class AssertionTracker {
int recorderIndex;
@@ -223,9 +220,12 @@ public class AssertionWriter {
}
}
- private void throwAssertError() { // TODO: GROOVY-10878
- assertFailedMethod.call(controller.getMethodVisitor());
- controller.getOperandStack().remove(2); // assertFailed called with 2 arguments
+ private void throwAssertError() {
+ // GROOVY-10878: call method that returns throwable, then throw it from here for better coverage metrics
+ controller.getMethodVisitor().visitMethodInsn(INVOKESTATIC, "org/codehaus/groovy/runtime/InvokerHelper",
+ "createAssertError", "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/AssertionError;", false);
+ controller.getMethodVisitor().visitInsn(ATHROW); // throw AssertionError
+ controller.getOperandStack().remove(2); // two call arguments
}
//--------------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java b/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
index f664720379..7e4f547e53 100644
--- a/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
+++ b/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
@@ -402,11 +402,18 @@ public class InvokerHelper {
return answer;
}
- public static void assertFailed(Object expression, Object message) {
+ public static void assertFailed(final Object expression, final Object message) {
+ throw createAssertError(expression, message);
+ }
+
+ /**
+ * @since 4.0.7
+ */
+ public static AssertionError createAssertError(final Object expression, final Object message) {
if (message == null || "".equals(message)) {
- throw new PowerAssertionError(expression.toString());
+ return new PowerAssertionError(expression.toString());
}
- throw new AssertionError(message + ". Expression: " + expression);
+ return new AssertionError(message + ". Expression: " + expression);
}
public static Object runScript(Class scriptClass, String[] args) {
[groovy] 01/02: classgen: `StringBuilder` for `assert` with message and general cleanup
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit cabea972bb5291c650d762bc102ad218760a64fd
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Dec 19 10:14:45 2022 -0600
classgen: `StringBuilder` for `assert` with message and general cleanup
---
.../groovy/classgen/asm/AssertionWriter.java | 256 +++++++++++----------
1 file changed, 134 insertions(+), 122 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/AssertionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/AssertionWriter.java
index 55dafb6b33..e76b794134 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/AssertionWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/AssertionWriter.java
@@ -21,7 +21,6 @@ package org.codehaus.groovy.classgen.asm;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.BooleanExpression;
-import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.stmt.AssertStatement;
@@ -36,6 +35,7 @@ import org.objectweb.asm.MethodVisitor;
import java.util.ArrayList;
import java.util.List;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
import static org.objectweb.asm.Opcodes.ALOAD;
import static org.objectweb.asm.Opcodes.ATHROW;
import static org.objectweb.asm.Opcodes.DUP;
@@ -60,123 +60,130 @@ public class AssertionWriter {
private AssertionTracker assertionTracker;
private AssertionTracker disabledTracker;
- public AssertionWriter(WriterController wc) {
+ public AssertionWriter(final WriterController wc) {
this.controller = wc;
}
- public void writeAssertStatement(AssertStatement statement) {
- MethodVisitor mv = controller.getMethodVisitor();
- OperandStack operandStack = controller.getOperandStack();
-
- // don't rewrite assertions with message
- boolean rewriteAssert = isNull(statement.getMessageExpression());
+ public void writeAssertStatement(final AssertStatement statement) {
AssertionTracker oldTracker = assertionTracker;
- Janitor janitor = new Janitor();
- final Label tryStart = new Label();
- if (rewriteAssert){
- assertionTracker = new AssertionTracker();
- try {
- // because source position seems to be more reliable for statements
- // than for expressions, we get the source text for the whole statement
- assertionTracker.sourceText = new SourceText(statement, controller.getSourceUnit(), janitor);
- mv.visitTypeInsn(NEW, "org/codehaus/groovy/runtime/powerassert/ValueRecorder");
- mv.visitInsn(DUP);
- mv.visitMethodInsn(INVOKESPECIAL, "org/codehaus/groovy/runtime/powerassert/ValueRecorder", "<init>", "()V", false);
- //TODO: maybe use more specialized type here
- controller.getOperandStack().push(ClassHelper.OBJECT_TYPE);
- assertionTracker.recorderIndex = controller.getCompileStack().defineTemporaryVariable("recorder", true);
- mv.visitLabel(tryStart);
- } catch (SourceTextNotAvailableException e) {
- // set assertionTracker to null to deactivate AssertionWriter#record calls
- assertionTracker = null;
- // don't rewrite assertions w/o source text
- rewriteAssert = false;
+ assertionTracker = null;
+ try {
+ MethodVisitor mv = controller.getMethodVisitor();
+ OperandStack operandStack = controller.getOperandStack();
+ CompileStack compileStack = controller.getCompileStack();
+
+ SourceText sourceText;
+ // don't rewrite assertions with message or no source
+ if (!isNullConstant(statement.getMessageExpression())
+ || (sourceText = getSourceText(statement)) == null) {
+ // test the condition
+ statement.getBooleanExpression().visit(controller.getAcg());
+ Label falseBranch = operandStack.jump(IFEQ);
+ Label afterAssert = new Label();
+ mv.visitJumpInsn(GOTO, afterAssert);
+
+ mv.visitLabel(falseBranch);
+ writeSourcelessAssertText(statement);
+ operandStack.push(ClassHelper.STRING_TYPE);
+ statement.getMessageExpression().visit(controller.getAcg());
+ operandStack.box();
+ throwAssertError();
+
+ mv.visitLabel(afterAssert);
+ return;
}
- }
- statement.getBooleanExpression().visit(controller.getAcg());
+ assertionTracker = new AssertionTracker();
+ assertionTracker.sourceText = sourceText;
- Label exceptionThrower = operandStack.jump(IFEQ);
+ mv.visitTypeInsn(NEW, "org/codehaus/groovy/runtime/powerassert/ValueRecorder");
+ mv.visitInsn(DUP);
+ mv.visitMethodInsn(INVOKESPECIAL, "org/codehaus/groovy/runtime/powerassert/ValueRecorder", "<init>", "()V", false);
+ operandStack.push(ClassHelper.OBJECT_TYPE); // TODO: maybe use more specialized type here
+ assertionTracker.recorderIndex = compileStack.defineTemporaryVariable("recorder", true);
+ Label tryStart = new Label(), tryEnd = new Label();
+ mv.visitLabel(tryStart);
+
+ // test the condition
+ statement.getBooleanExpression().visit(controller.getAcg());
+ Label falseBranch = operandStack.jump(IFEQ);
- // do nothing, but clear the value recorder
- if (rewriteAssert) {
- //clean up assertion recorder
+ // clear the value recorder and proceed normally
mv.visitVarInsn(ALOAD, assertionTracker.recorderIndex);
mv.visitMethodInsn(INVOKEVIRTUAL, "org/codehaus/groovy/runtime/powerassert/ValueRecorder", "clear", "()V", false);
- }
- Label afterAssert = new Label();
- mv.visitJumpInsn(GOTO, afterAssert);
- mv.visitLabel(exceptionThrower);
+ Label afterAssert = new Label();
+ mv.visitJumpInsn(GOTO, afterAssert);
- if (rewriteAssert) {
+ mv.visitLabel(falseBranch);
+ // load the (power assert) expression text
mv.visitLdcInsn(assertionTracker.sourceText.getNormalizedText());
mv.visitVarInsn(ALOAD, assertionTracker.recorderIndex);
mv.visitMethodInsn(INVOKESTATIC, "org/codehaus/groovy/runtime/powerassert/AssertionRenderer", "render", "(Ljava/lang/String;Lorg/codehaus/groovy/runtime/powerassert/ValueRecorder;)Ljava/lang/String;", false);
- } else {
- writeSourcelessAssertText(statement);
- }
- operandStack.push(ClassHelper.STRING_TYPE);
- AssertionTracker savedTracker = assertionTracker;
- assertionTracker = null;
+ operandStack.push(ClassHelper.STRING_TYPE);
- // now the optional exception expression
- statement.getMessageExpression().visit(controller.getAcg());
- operandStack.box();
- assertFailedMethod.call(mv);
- operandStack.remove(2); // assertFailed called static with 2 arguments
+ int recorderIndex = assertionTracker.recorderIndex;
+ assertionTracker = null; // deactivate AssertionWriter#record calls
+
+ // load the optional message expression
+ statement.getMessageExpression().visit(controller.getAcg());
+ operandStack.box();
+ throwAssertError();
- if (rewriteAssert) {
- final Label tryEnd = new Label();
mv.visitLabel(tryEnd);
mv.visitJumpInsn(GOTO, afterAssert);
- // finally block to clean assertion recorder
- final Label catchAny = new Label();
- mv.visitLabel(catchAny);
- mv.visitVarInsn(ALOAD, savedTracker.recorderIndex);
+
+ // catch-all block to clear value recorder
+ Label catchAll = new Label();
+ mv.visitLabel(catchAll);
+ mv.visitVarInsn(ALOAD, recorderIndex);
mv.visitMethodInsn(INVOKEVIRTUAL, "org/codehaus/groovy/runtime/powerassert/ValueRecorder", "clear", "()V", false);
- mv.visitInsn(ATHROW);
- // add catch any block to exception table
- controller.getCompileStack().addExceptionBlock(tryStart, tryEnd, catchAny, null);
- }
+ mv.visitInsn(ATHROW); // re-throw
+ compileStack.addExceptionBlock(tryStart, tryEnd, catchAll, null);
+
+ mv.visitLabel(afterAssert);
- mv.visitLabel(afterAssert);
- if (rewriteAssert) {
- controller.getCompileStack().removeVar(savedTracker.recorderIndex);
+ compileStack.removeVar(recorderIndex);
+ } finally {
+ assertionTracker = oldTracker;
}
- assertionTracker = oldTracker;
- // close possibly open file handles from getting a sample for
- // power asserts
- janitor.cleanup();
}
- private boolean isNull(Expression messageExpression) {
- return messageExpression instanceof ConstantExpression && ((ConstantExpression) messageExpression).isNullExpression();
+ private SourceText getSourceText(final AssertStatement statement) {
+ Janitor janitor = new Janitor();
+ try {
+ // because source position seems to be more reliable for statements
+ // than for expressions, get the source text for the whole statement
+ return new SourceText(statement, controller.getSourceUnit(), janitor);
+ } catch (SourceTextNotAvailableException e) {
+ return null;
+ } finally {
+ // close open file handles from getting a sample for power asserts
+ janitor.cleanup();
+ }
}
- private void writeSourcelessAssertText(AssertStatement statement) {
+ private void writeSourcelessAssertText(final AssertStatement statement) {
MethodVisitor mv = controller.getMethodVisitor();
OperandStack operandStack = controller.getOperandStack();
+ CompileStack compileStack = controller.getCompileStack();
BooleanExpression booleanExpression = statement.getBooleanExpression();
- // push expression string onto stack
String expressionText = booleanExpression.getText();
- List<String> list = new ArrayList<String>();
- addVariableNames(booleanExpression, list);
- if (list.isEmpty()) {
+ // push expression string onto stack
+ List<String> names = new ArrayList<>();
+ addVariableNames(booleanExpression, names);
+ if (names.isEmpty()) {
mv.visitLdcInsn(expressionText);
} else {
- boolean first = true;
-
- // let's create a new expression
- mv.visitTypeInsn(NEW, "java/lang/StringBuffer");
+ mv.visitTypeInsn(NEW, "java/lang/StringBuilder");
mv.visitInsn(DUP);
mv.visitLdcInsn(expressionText + ". Values: ");
- mv.visitMethodInsn(INVOKESPECIAL, "java/lang/StringBuffer", "<init>", "(Ljava/lang/String;)V", false);
- //TODO: maybe use more special type StringBuffer here
- operandStack.push(ClassHelper.OBJECT_TYPE);
- int tempIndex = controller.getCompileStack().defineTemporaryVariable("assert", true);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/StringBuilder", "<init>", "(Ljava/lang/String;)V", false);
+ operandStack.push(ClassHelper.OBJECT_TYPE); // TODO: maybe use more special type StringBuilder here
+ int tempIndex = compileStack.defineTemporaryVariable("assert", true);
- for (String name : list) {
+ boolean first = true;
+ for (String name : names) {
String text = name + " = ";
if (first) {
first = false;
@@ -186,52 +193,23 @@ public class AssertionWriter {
mv.visitVarInsn(ALOAD, tempIndex);
mv.visitLdcInsn(text);
- mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/StringBuffer", "append", "(Ljava/lang/Object;)Ljava/lang/StringBuffer;", false);
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/Object;)Ljava/lang/StringBuilder;", false);
mv.visitInsn(POP);
mv.visitVarInsn(ALOAD, tempIndex);
new VariableExpression(name).visit(controller.getAcg());
operandStack.box();
mv.visitMethodInsn(INVOKESTATIC, "org/codehaus/groovy/runtime/InvokerHelper", "toString", "(Ljava/lang/Object;)Ljava/lang/String;", false);
- mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/StringBuffer", "append", "(Ljava/lang/String;)Ljava/lang/StringBuffer;", false);
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/String;)Ljava/lang/StringBuilder;", false);
mv.visitInsn(POP);
operandStack.remove(1);
}
mv.visitVarInsn(ALOAD, tempIndex);
- controller.getCompileStack().removeVar(tempIndex);
+ compileStack.removeVar(tempIndex);
}
}
- public void record(Expression expression) {
- if (assertionTracker==null) return;
- record(assertionTracker.sourceText.getNormalizedColumn(expression.getLineNumber(), expression.getColumnNumber()));
- }
-
- public void record(Token op) {
- if (assertionTracker==null) return;
- record(assertionTracker.sourceText.getNormalizedColumn(op.getStartLine(), op.getStartColumn()));
- }
-
- private void record(int normalizedColumn) {
- if (assertionTracker==null) return;
-
- MethodVisitor mv = controller.getMethodVisitor();
- OperandStack operandStack = controller.getOperandStack();
-
- operandStack.dup();
- operandStack.box();
-
- mv.visitVarInsn(ALOAD, assertionTracker.recorderIndex);
- operandStack.push(ClassHelper.OBJECT_TYPE);
- //helper.swapWithObject(ClassHelper.OBJECT_TYPE);
- operandStack.swap();
- mv.visitLdcInsn(normalizedColumn);
- mv.visitMethodInsn(INVOKEVIRTUAL, "org/codehaus/groovy/runtime/powerassert/ValueRecorder", "record", "(Ljava/lang/Object;I)Ljava/lang/Object;", false);
- mv.visitInsn(POP);
- operandStack.remove(2);
- }
-
- private void addVariableNames(Expression expression, List<String> list) {
+ private static void addVariableNames(final Expression expression, final List<String> list) {
if (expression instanceof BooleanExpression) {
BooleanExpression boolExp = (BooleanExpression) expression;
addVariableNames(boolExp.getExpression(), list);
@@ -245,16 +223,50 @@ public class AssertionWriter {
}
}
+ private void throwAssertError() { // TODO: GROOVY-10878
+ assertFailedMethod.call(controller.getMethodVisitor());
+ controller.getOperandStack().remove(2); // assertFailed called with 2 arguments
+ }
+
+ //--------------------------------------------------------------------------
+
public void disableTracker() {
- if (assertionTracker==null) return;
- disabledTracker = assertionTracker;
- assertionTracker = null;
+ if (assertionTracker != null) {
+ disabledTracker = assertionTracker;
+ assertionTracker = null;
+ }
}
public void reenableTracker() {
- if (disabledTracker==null) return;
- assertionTracker = disabledTracker;
- disabledTracker = null;
+ if (disabledTracker != null) {
+ assertionTracker = disabledTracker;
+ disabledTracker = null;
+ }
}
+ public void record(final Token token) {
+ if (assertionTracker != null)
+ record(assertionTracker.sourceText.getNormalizedColumn(token.getStartLine(), token.getStartColumn()));
+ }
+
+ public void record(final Expression expression) {
+ if (assertionTracker != null)
+ record(assertionTracker.sourceText.getNormalizedColumn(expression.getLineNumber(), expression.getColumnNumber()));
+ }
+
+ private void record(final int normalizedColumn) {
+ MethodVisitor mv = controller.getMethodVisitor();
+ OperandStack operandStack = controller.getOperandStack();
+
+ operandStack.dup();
+ operandStack.box();
+
+ mv.visitVarInsn(ALOAD, assertionTracker.recorderIndex);
+ operandStack.push(ClassHelper.OBJECT_TYPE);
+ operandStack.swap();
+ mv.visitLdcInsn(normalizedColumn);
+ mv.visitMethodInsn(INVOKEVIRTUAL, "org/codehaus/groovy/runtime/powerassert/ValueRecorder", "record", "(Ljava/lang/Object;I)Ljava/lang/Object;", false);
+ mv.visitInsn(POP);
+ operandStack.remove(2);
+ }
}