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 <[email protected]> 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 } } -
