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/08/01 14:07:22 UTC
[groovy] branch GROOVY-10666 updated (1764ade5e5 -> 97bc4481f1)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-10666
in repository https://gitbox.apache.org/repos/asf/groovy.git
discard 1764ade5e5 GROOVY-5744, GROOVY-10666: multi-assign via `iterator()` or `getAt(int)`
new 97bc4481f1 GROOVY-5744, GROOVY-10666: multi-assign via `iterator()` or `getAt(int)`
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (1764ade5e5)
\
N -- N -- N refs/heads/GROOVY-10666 (97bc4481f1)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
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.
Summary of changes:
.../groovy/classgen/asm/BytecodeVariable.java | 65 +++++++++++-----------
.../MultipleAssignmentDeclarationTest.groovy | 31 ++++++++++-
2 files changed, 61 insertions(+), 35 deletions(-)
[groovy] 01/01: GROOVY-5744, GROOVY-10666: multi-assign via `iterator()` or `getAt(int)`
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-10666
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 97bc4481f124f808dda1f6d6d3d786839649e799
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jul 31 21:07:21 2022 -0500
GROOVY-5744, GROOVY-10666: multi-assign via `iterator()` or `getAt(int)`
---
.../classgen/asm/BinaryExpressionHelper.java | 99 +++++++++++++++++-----
.../groovy/classgen/asm/BytecodeVariable.java | 65 +++++++-------
.../MultipleAssignmentDeclarationTest.groovy | 55 ++++++++++--
.../gls/statements/MultipleAssignmentTest.groovy | 4 +-
src/test/gls/syntax/Gep3Test.groovy | 89 +++++++++----------
5 files changed, 207 insertions(+), 105 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
index e5810243ea..068b225059 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
@@ -53,8 +53,11 @@ import org.objectweb.asm.MethodVisitor;
import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.boolX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ternaryX;
import static org.codehaus.groovy.syntax.Types.ASSIGN;
import static org.codehaus.groovy.syntax.Types.BITWISE_AND;
import static org.codehaus.groovy.syntax.Types.BITWISE_AND_EQUAL;
@@ -105,10 +108,14 @@ import static org.codehaus.groovy.syntax.Types.RIGHT_SHIFT_EQUAL;
import static org.codehaus.groovy.syntax.Types.RIGHT_SHIFT_UNSIGNED;
import static org.codehaus.groovy.syntax.Types.RIGHT_SHIFT_UNSIGNED_EQUAL;
import static org.objectweb.asm.Opcodes.ACONST_NULL;
+import static org.objectweb.asm.Opcodes.ALOAD;
+import static org.objectweb.asm.Opcodes.DUP;
import static org.objectweb.asm.Opcodes.GOTO;
import static org.objectweb.asm.Opcodes.IFEQ;
import static org.objectweb.asm.Opcodes.IFNE;
+import static org.objectweb.asm.Opcodes.IF_ACMPEQ;
import static org.objectweb.asm.Opcodes.INSTANCEOF;
+import static org.objectweb.asm.Opcodes.POP;
public class BinaryExpressionHelper {
// compare
@@ -371,11 +378,13 @@ public class BinaryExpressionHelper {
public void evaluateEqual(final BinaryExpression expression, final boolean defineVariable) {
AsmClassGenerator acg = controller.getAcg();
+ MethodVisitor mv = controller.getMethodVisitor();
CompileStack compileStack = controller.getCompileStack();
OperandStack operandStack = controller.getOperandStack();
Expression leftExpression = expression.getLeftExpression();
Expression rightExpression = expression.getRightExpression();
- boolean directAssignment = defineVariable && !(leftExpression instanceof TupleExpression);
+ boolean singleAssignment = !(leftExpression instanceof TupleExpression);
+ boolean directAssignment = defineVariable && singleAssignment; //def x=y
if (directAssignment && rightExpression instanceof EmptyExpression) {
VariableExpression ve = (VariableExpression) leftExpression;
@@ -430,7 +439,7 @@ public class BinaryExpressionHelper {
rhsValueId = compileStack.defineTemporaryVariable("$rhs", rhsType, true);
}
// TODO: if RHS is VariableSlotLoader already, then skip creating a new one
- BytecodeExpression rhsValueLoader = new VariableSlotLoader(rhsType,rhsValueId,operandStack);
+ Expression rhsValueLoader = new VariableSlotLoader(rhsType, rhsValueId, operandStack);
// assignment for subscript
if (leftExpression instanceof BinaryExpression) {
@@ -444,12 +453,49 @@ public class BinaryExpressionHelper {
compileStack.pushLHS(true);
- if (leftExpression instanceof TupleExpression) {
- // multiple declaration
- TupleExpression tuple = (TupleExpression) leftExpression;
- int i = 0;
- for (Expression e : tuple.getExpressions()) {
- callX(rhsValueLoader, "getAt", args(constX(i++))).visit(acg);
+ if (directAssignment) {
+ rhsValueLoader.visit(acg);
+ operandStack.remove(1);
+ compileStack.popLHS();
+ return;
+ }
+
+ if (singleAssignment) {
+ int mark = operandStack.getStackLength();
+ rhsValueLoader.visit(acg);
+ leftExpression.visit(acg);
+ operandStack.remove(operandStack.getStackLength() - mark);
+ } else { // multiple declaration or assignment
+ MethodCallExpression iterator = callX(rhsValueLoader, "iterator");
+ iterator.setImplicitThis(false);
+ iterator.visit(acg);
+
+ int iteratorId = compileStack.defineTemporaryVariable("$iter", true);
+ Expression seq = new VariableSlotLoader(iteratorId, operandStack);
+
+ MethodCallExpression hasNext = callX(seq, "hasNext");
+ hasNext.setImplicitThis(false);
+ boolX(hasNext).visit(acg);
+
+ Label done = new Label(), useGetAt = new Label();
+ Label useGetAt_noPop = operandStack.jump(IFEQ);
+
+ MethodCallExpression next = callX(seq, "next");
+ next.setImplicitThis(false);
+ next.visit(acg);
+
+ // check if first element is RHS; indicative of DGM#iterator(Object)
+ mv.visitInsn(DUP);
+ mv.visitVarInsn(ALOAD, rhsValueId);
+ mv.visitJumpInsn(IF_ACMPEQ, useGetAt);
+
+ boolean first = true;
+ for (Expression e : (TupleExpression) leftExpression) {
+ if (first) {
+ first = false;
+ } else {
+ ternaryX(hasNext, next, nullX()).visit(acg);
+ }
if (defineVariable) {
Variable v = (Variable) e;
operandStack.doGroovyCast(v);
@@ -459,19 +505,32 @@ public class BinaryExpressionHelper {
e.visit(acg);
}
}
- } else if (defineVariable) {
- // single declaration
- rhsValueLoader.visit(acg);
- operandStack.remove(1);
- compileStack.popLHS();
- return;
- } else {
- // normal assignment
- int mark = operandStack.getStackLength();
- rhsValueLoader.visit(acg);
- leftExpression.visit(acg);
- operandStack.remove(operandStack.getStackLength() - mark);
+
+ mv.visitJumpInsn(GOTO, done);
+
+ mv.visitLabel(useGetAt);
+
+ mv.visitInsn(POP); // discard result of "rhs.iterator().next()"
+
+ mv.visitLabel(useGetAt_noPop);
+
+ int i = 0;
+ for (Expression e : (TupleExpression) leftExpression) {
+ MethodCallExpression getAt = callX(rhsValueLoader, "getAt", constX(i++, true));
+ getAt.setImplicitThis(false);
+ getAt.visit(acg);
+
+ if (defineVariable) {
+ Variable v = (Variable) e;
+ operandStack.doGroovyCast(v);
+ }
+ e.visit(acg);
+ }
+
+ mv.visitLabel(done);
+ compileStack.removeVar(iteratorId);
}
+
compileStack.popLHS();
// return value of assignment
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeVariable.java b/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeVariable.java
index 0ec751a374..4c4c5dda5f 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeVariable.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeVariable.java
@@ -34,28 +34,34 @@ public class BytecodeVariable {
private ClassNode type;
private String name;
private final int prevCurrent;
+ private boolean dynamicTyped;
private boolean holder;
// br for setting on the LocalVariableTable in the class file
// these fields should probably go to jvm Operand class
- private Label startLabel = null;
- private Label endLabel = null;
- private boolean dynamicTyped;
+ private Label startLabel;
+ private Label endLabel;
- private BytecodeVariable(){
+ private BytecodeVariable() {
+ index = 0;
+ prevCurrent = 0;
dynamicTyped = true;
- index=0;
- holder=false;
- prevCurrent=0;
}
- public BytecodeVariable(int index, ClassNode type, String name, int prevCurrent) {
+ public BytecodeVariable(final int index, final ClassNode type, final String name, final int prevCurrent) {
this.index = index;
this.type = type;
this.name = name;
this.prevCurrent = prevCurrent;
}
+ /**
+ * @return the stack index for this variable
+ */
+ public int getIndex() {
+ return index;
+ }
+
public String getName() {
return name;
}
@@ -64,11 +70,21 @@ public class BytecodeVariable {
return type;
}
- /**
- * @return the stack index for this variable
- */
- public int getIndex() {
- return index;
+ public void setType(final ClassNode type) {
+ this.type = type;
+ dynamicTyped = dynamicTyped || ClassHelper.isDynamicTyped(type);
+ }
+
+ public int getPrevIndex() {
+ return prevCurrent;
+ }
+
+ public boolean isDynamicTyped() {
+ return dynamicTyped;
+ }
+
+ public void setDynamicTyped(final boolean b) {
+ dynamicTyped = b;
}
/**
@@ -78,7 +94,7 @@ public class BytecodeVariable {
return holder;
}
- public void setHolder(boolean holder) {
+ public void setHolder(final boolean holder) {
this.holder = holder;
}
@@ -86,7 +102,7 @@ public class BytecodeVariable {
return startLabel;
}
- public void setStartLabel(Label startLabel) {
+ public void setStartLabel(final Label startLabel) {
this.startLabel = startLabel;
}
@@ -94,7 +110,7 @@ public class BytecodeVariable {
return endLabel;
}
- public void setEndLabel(Label endLabel) {
+ public void setEndLabel(final Label endLabel) {
this.endLabel = endLabel;
}
@@ -102,21 +118,4 @@ public class BytecodeVariable {
public String toString() {
return name + "(index=" + index + ",type=" + type + ",holder="+holder+")";
}
-
- public void setType(ClassNode type) {
- this.type = type;
- dynamicTyped |= ClassHelper.isDynamicTyped(type);
- }
-
- public void setDynamicTyped(boolean b) {
- dynamicTyped = b;
- }
-
- public boolean isDynamicTyped() {
- return dynamicTyped;
- }
-
- public int getPrevIndex() {
- return prevCurrent;
- }
}
diff --git a/src/test/gls/statements/MultipleAssignmentDeclarationTest.groovy b/src/test/gls/statements/MultipleAssignmentDeclarationTest.groovy
index 8ae6744b3e..b2ddd6aeeb 100644
--- a/src/test/gls/statements/MultipleAssignmentDeclarationTest.groovy
+++ b/src/test/gls/statements/MultipleAssignmentDeclarationTest.groovy
@@ -18,7 +18,6 @@
*/
package gls.statements
-import groovy.test.NotYetImplemented
import org.junit.Test
import static groovy.test.GroovyAssert.assertScript
@@ -79,10 +78,10 @@ final class MultipleAssignmentDeclarationTest {
}
@Test
- void testNestedScope() {
+ void testNestedScope1() {
assertScript '''import static groovy.test.GroovyAssert.shouldFail
- def c = {
+ def c = { ->
def (i,j) = [1,2]
assert i == 1
assert j == 2
@@ -107,6 +106,33 @@ final class MultipleAssignmentDeclarationTest {
'''
}
+ @Test
+ void testNestedScope2() {
+ assertScript '''
+ class C {
+ int m() {
+ def (i,j) = [1,2]
+ assert i == 1
+ assert j == 2
+
+ def x = { ->
+ assert i == 1
+ assert j == 2
+
+ i = 3
+ assert i == 3
+ }
+ x()
+
+ assert i == 3
+ return j
+ }
+ }
+ int n = new C().m()
+ assert n == 2
+ '''
+ }
+
@Test
void testMultiAssignChain() {
assertScript '''
@@ -117,7 +143,24 @@ final class MultipleAssignmentDeclarationTest {
'''
}
- @NotYetImplemented @Test // GROOVY-5744
+ @Test
+ void testMultiAssignFromObject() {
+ shouldFail MissingMethodException, '''
+ def obj = new Object()
+ def (x) = obj
+ '''
+ }
+
+ @Test
+ void testMultiAssignFromCalendar() {
+ assertScript '''
+ def (_, y, m) = Calendar.instance
+ assert y >= 2022
+ assert m in 0..11
+ '''
+ }
+
+ @Test // GROOVY-5744
void testMultiAssignFromIterator() {
assertScript '''
def list = [1,2,3]
@@ -129,7 +172,7 @@ final class MultipleAssignmentDeclarationTest {
'''
}
- @NotYetImplemented @Test // GROOVY-10666
+ @Test // GROOVY-10666
void testMultiAssignFromIterable() {
assertScript '''
class MySet {
@@ -172,7 +215,7 @@ final class MultipleAssignmentDeclarationTest {
'''
}
- @NotYetImplemented @Test // GROOVY-10666
+ @Test // GROOVY-10666
void testMultiAssignFromJavaStream() {
assertScript '''import java.util.stream.Stream
diff --git a/src/test/gls/statements/MultipleAssignmentTest.groovy b/src/test/gls/statements/MultipleAssignmentTest.groovy
index 71e58743c4..0a815dcf08 100644
--- a/src/test/gls/statements/MultipleAssignmentTest.groovy
+++ b/src/test/gls/statements/MultipleAssignmentTest.groovy
@@ -93,7 +93,7 @@ final class MultipleAssignmentTest {
}
@Test
- void testChainedMultiAssignment() {
+ void testMultiAssignChain() {
assertScript '''
def a, b, c, d
@@ -105,4 +105,4 @@ final class MultipleAssignmentTest {
assert [c, d] == [3, 4]
'''
}
-}
\ No newline at end of file
+}
diff --git a/src/test/gls/syntax/Gep3Test.groovy b/src/test/gls/syntax/Gep3Test.groovy
index b24ce46be0..9457cb5dd3 100644
--- a/src/test/gls/syntax/Gep3Test.groovy
+++ b/src/test/gls/syntax/Gep3Test.groovy
@@ -20,10 +20,10 @@ package gls.syntax
import groovy.test.GroovyTestCase
-import static Container.*
-import static Ingredient.*
-import static CookingAction.*
-import static Temperature.*
+import static gls.syntax.Container.*
+import static gls.syntax.CookingAction.*
+import static gls.syntax.Ingredient.*
+import static gls.syntax.Temperature.*
/**
* Test case for "extended command expressions" (GEP-3) added in Groovy 1.8
@@ -72,7 +72,7 @@ class Gep3Test extends GroovyTestCase {
Integer.metaClass = null
}
- static String txt = "Lidia is Groovy ;)"
+ static String txt = 'Lidia is Groovy ;)'
void testSimpleClassicalCommandExpressions() {
foo txt
@@ -87,17 +87,17 @@ class Gep3Test extends GroovyTestCase {
static a2(Closure c) { return txt }
void testNewSyntax() {
- def expectedResult = "from:Lidia;to:Guillaume;body:how are you?;"
+ def expectedResult = 'from:Lidia;to:Guillaume;body:how are you?;'
def e = new Email()
- e.from "Lidia" to "Guillaume" body "how are you?"
+ e.from 'Lidia' to 'Guillaume' body 'how are you?'
def result = e.send()
assert expectedResult == result
}
void testContactInfo() {
- def contact = [name: { String name -> assert name == "Guillaume"; [age: { int age -> assert age == 33 }]}]
- contact.name "Guillaume" age 33
+ def contact = [name: { String name -> assert name == 'Guillaume'; [age: { int age -> assert age == 33 }]}]
+ contact.name 'Guillaume' age 33
}
void testArtistPaintingWithAMixOfNamedAndNonNamedParams() {
@@ -105,15 +105,15 @@ class Gep3Test extends GroovyTestCase {
def artist = [
paint: { String what ->
- assert what == "wall"
+ assert what == 'wall'
[
with: { Map m, String c1, String c2 ->
- assert m.and == "Blue"
- assert c1 == "Red"
- assert c2 == "Green"
+ assert m.and == 'Blue'
+ assert c1 == 'Red'
+ assert c2 == 'Green'
[
at: { String time ->
- assert time == "3 PM"
+ assert time == '3 PM'
}
]
}
@@ -121,12 +121,12 @@ class Gep3Test extends GroovyTestCase {
}
]
- artist.paint "wall" with "Red", "Green", and: "Blue" at 3.pm
+ artist.paint 'wall' with 'Red', 'Green', and: 'Blue' at 3.pm
}
void testArgWith() {
- def arr = ["he", "ll", "o"]
- def concat = { String s1 -> [with: { String s2 -> [and: { String s3 -> assert s1+s2+s3 == "hello"}]}]}
+ def arr = ['he', 'll', 'o']
+ def concat = { String s1 -> [with: { String s2 -> [and: { String s3 -> assert s1+s2+s3 == 'hello'}]}]}
concat arr[0] with arr[1] and arr[2]
}
@@ -140,18 +140,18 @@ class Gep3Test extends GroovyTestCase {
static execute(Closure c) { c }
- static String message = ""
+ static String message = ''
static drugQuantity, drug
void testMedicine() {
- Number.metaClass.getPills = { -> new DrugQuantity(q: delegate, form: "pills") }
- Number.metaClass.getHours = { -> new Duration(q: delegate, unit: "hours") }
+ Number.metaClass.getPills = { -> new DrugQuantity(q: delegate, form: 'pills') }
+ Number.metaClass.getHours = { -> new Duration(q: delegate, unit: 'hours') }
- def chloroquinine = new Drug(name: "Chloroquinine")
+ def chloroquinine = new Drug(name: 'Chloroquinine')
take 3.pills of chloroquinine after 6.hours
- assert message == "Take 3 pills of Chloroquinine after 6 hours"
+ assert message == 'Take 3 pills of Chloroquinine after 6 hours'
}
@@ -160,7 +160,8 @@ class Gep3Test extends GroovyTestCase {
def after(Duration dur) { message = "Take $drugQuantity of $drug after $dur" }
void testRecipeDsl() {
- def (once, twice) = [1, 2]
+ final once = 1
+ final twice = 2
Integer.metaClass.getMinutes { delegate }
@@ -189,7 +190,7 @@ class Gep3Test extends GroovyTestCase {
void testExtendedCommandExpressionsOnTheRHS() {
def ( coffee, sugar, milk, liquor ) =
- ["coffee", "sugar", "milk", "liquor"]
+ ['coffee', 'sugar', 'milk', 'liquor']
def drink = Drink.&drink
def r1 = drink coffee
@@ -310,15 +311,15 @@ class Gep3Test extends GroovyTestCase {
* Case where an Integer is used as name
*
* case a b 1 2
- * equivalent a(b)."1"(2)
+ * equivalent a(b).'1'(2)
*/
void testIntegerAsName() {
- Integer.metaClass."1" = {x-> assert delegate == 10; x}
- def a = {x-> assert x == "b"; 10}
- def b = "b"
+ Integer.metaClass.'1' = {x-> assert delegate == 10; x}
+ def a = {x-> assert x == 'b'; 10}
+ def b = 'b'
def x = a b 1 2
assert x == 2
- assert a(b)."1"(2) == 2
+ assert a(b).'1'(2) == 2
}
void testMethodAndMethodCallTakingClosureArgument() {
@@ -348,7 +349,7 @@ class Gep3Test extends GroovyTestCase {
// equivalent task(copy(type:Copy,{10}))
void testInnerMethodWithClosure() {
// with simple expression
- assertScript """
+ assertScript '''
class Copy{}
def task(x) {
assert x == 2
@@ -359,9 +360,9 @@ class Gep3Test extends GroovyTestCase {
2
}
task copy(type: Copy) { 10 }
- """
+ '''
// with nested gep3
- assertScript """
+ assertScript '''
class Copy{}
def task(x) {
assert x == 2
@@ -373,8 +374,8 @@ class Gep3Test extends GroovyTestCase {
}
def a(x){this}
def b(x){x*10}
- task copy(type: Copy) { a 10 b 10 }
- """
+ task copy(type: Copy) { a 10 b 10 }
+ '''
}
void testGradleDSL() {
@@ -395,7 +396,7 @@ class Gep3Test extends GroovyTestCase {
def constraints(Closure c) {
c.delegate = [authCode: { Map m -> println m }]
c.resolveStrategy = Closure.DELEGATE_FIRST
- c()
+ c()
}
def authCode(Map m) {
@@ -406,14 +407,14 @@ class Gep3Test extends GroovyTestCase {
name xxx: "/c/$val"(controller: 'foo', action: 'bar') {
constraints {
- authCode blank: false
+ authCode blank: false
}
- }
+ }
'''
}
void testUsageOfInnerClass() {
- assertScript """
+ assertScript '''
class Demo {
void doit() {
execute new Runnable(){
@@ -428,10 +429,11 @@ class Gep3Test extends GroovyTestCase {
}
}
new Demo().doit()
- """
+ '''
}
}
+//------------------------------------------------------------------------------
class Drink {
String beverage
@@ -507,20 +509,20 @@ class Duration {
}
class Email {
- String msg = ""
+ String msg = ''
def from(address) {
- msg += "from:" + address + ";"
+ msg += 'from:' + address + ';'
return this
}
def to(address) {
- msg += "to:" + address + ";"
+ msg += 'to:' + address + ';'
return this
}
def body(text) {
- msg += "body:" + text + ";"
+ msg += 'body:' + text + ';'
return this
}
@@ -528,4 +530,3 @@ class Email {
return msg
}
}
-