Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1981,8 +1981,8 @@ public void visitListExpression(final ListExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
BytecodeHelper.pushConstant(mv, size);
mv.visitTypeInsn(ANEWARRAY, "java/lang/Object");
int maxInit = 1000;
if (size<maxInit || !containsOnlyConstants) {
final int maxInit = 1000; // top end for unroll
if (size < maxInit || !containsOnlyConstants) {
for (int i = 0; i < size; i += 1) {
mv.visitInsn(DUP);
BytecodeHelper.pushConstant(mv, i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,28 @@
*/
package org.codehaus.groovy.transform.sc.transformers;

import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.ConstructorNode;
import org.codehaus.groovy.ast.GroovyCodeVisitor;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.ArrayExpression;
import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ExpressionTransformer;
import org.codehaus.groovy.ast.expr.ListExpression;
import org.codehaus.groovy.classgen.AsmClassGenerator;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;

import java.util.ArrayList;
import java.util.List;

import static java.util.stream.Collectors.toList;
import static org.codehaus.groovy.classgen.AsmClassGenerator.containsSpreadExpression;
import static org.objectweb.asm.Opcodes.DUP;
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;

class ListExpressionTransformer {

Expand All @@ -42,7 +52,7 @@ class ListExpressionTransformer {
Expression transformListExpression(final ListExpression le) {
MethodNode mn = le.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
if (mn instanceof ConstructorNode) {
List<Expression> elements = le.getExpressions().stream().map(scTransformer::transform).collect(toList());
List<Expression> elements = le.getExpressions().stream().map(scTransformer::transform).toList();

if (mn.getDeclaringClass().isArray()) {
var ae = new ArrayExpression(mn.getDeclaringClass().getComponentType(), elements);
Expand All @@ -56,6 +66,60 @@ Expression transformListExpression(final ListExpression le) {
return cce;
}

// GROOVY-8699: emit direct bytecode for simple list
if (mn == null && le.getExpressions().size() < 25 && !containsSpreadExpression(le)) {
ClassNode type = scTransformer.getTypeChooser().resolveType(le, scTransformer.getClassNode());
if (ArrayList_TYPE.equals(type)) { // annotation attributes cannot be transformed
var list = new NewListExpression(le.getExpressions().stream().map(scTransformer::transform).toList());
list.setSourcePosition(le);
list.copyNodeMetaData(le);
return list;
}
}

return scTransformer.superTransform(le);
}

//--------------------------------------------------------------------------

private static final ClassNode ArrayList_TYPE = ClassHelper.makeWithoutCaching(ArrayList.class);

private static final MethodNode ArrayList_NEW = ArrayList_TYPE.getDeclaredConstructor(new Parameter[] {new Parameter(ClassHelper.int_TYPE, "capacity")});

private static class NewListExpression extends ListExpression {

NewListExpression(final List<Expression> values) {
super(values);
}

@Override
public Expression transformExpression(final ExpressionTransformer transformer) {
var list = new NewListExpression(transformExpressions(getExpressions(), transformer));
list.setSourcePosition(this);
list.copyNodeMetaData(this);
return list;
}

@Override
public void visit(final GroovyCodeVisitor visitor) {
if (!(visitor instanceof AsmClassGenerator g)) {
super.visit(visitor);
} else {
var mv = g.getController().getMethodVisitor();
var os = g.getController().getOperandStack ();

var list = new ConstructorCallExpression(ArrayList_TYPE, new ConstantExpression(getExpressions().size(), true));
list.putNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET, ArrayList_NEW);
list.visit(visitor);

for (Expression li : getExpressions()) {
mv.visitInsn(DUP);
li.visit(visitor);
os.box();
mv.visitMethodInsn(INVOKEVIRTUAL, "java/util/ArrayList", "add", "(Ljava/lang/Object;)Z", false);
os.pop(); // boolean return value
}
}
}
}
}
9 changes: 5 additions & 4 deletions src/test/groovy/bugs/Groovy10034.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,15 @@ final class Groovy10034 extends AbstractBytecodeTestCase {
["x"].toArray(new String[0])
}
'''
int offset = result.indexOf('INVOKESTATIC', result.indexOf('--BEGIN--'))
int offset = result.indexOf('INVOKEVIRTUAL', result.indexOf('--BEGIN--'))
assert result.hasStrictSequence([
'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.createList',
'CHECKCAST java/util/ArrayList', // not 'INVOKEDYNAMIC cast'
'INVOKEVIRTUAL java/util/ArrayList.add',
'POP',
'ICONST_0',
'ANEWARRAY java/lang/String',
// no 'INVOKEDYNAMIC cast' to Object[]
'INVOKEVIRTUAL java/util/ArrayList.toArray'
'INVOKEVIRTUAL java/util/ArrayList.toArray',
'ARETURN'
], offset)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@ final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsST
@Test
void testShouldNotThrowVerifyError() {
assertScript '''
def al = new ArrayList<Double>()
al.add(2.0d)
assert al.get(0) + 1 == 3.0d
def list = new ArrayList<Double>()
list.add(2.0d)
assert list.get(0) + 1 == 3.0d
'''
}

// GROOVY-5654
@Test
void testShouldNotThrowForbiddenAccessWithMapProperty() {
assertScript '''
Map<String, Integer> m = ['abcd': 1234]
assert m['abcd'] == 1234
assert m.abcd == 1234
Map<String, Integer> map = ['abcd': 1234]
assert map['abcd'] == 1234
assert map.abcd == 1234
'''
}

Expand Down Expand Up @@ -71,33 +71,41 @@ final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsST
@Test
void testSpreadSafeMethodCallsOnListLiteralShouldNotCreateListTwice() {
assertScript '''
class Foo {
static void test() {
def list = [1, 2]
def lengths = [list << 3]*.size()
assert lengths == [3]
assert list == [1, 2, 3]
}
void check(List items, List sizes) {
assert items == [1, 2, 3]
assert sizes == [3]
}
void test() {
def items = [1, 2]
def sizes = [items << 3]*.size()
check(items, sizes)
}
Foo.test()
test()
'''
assert astTrees['Foo'][1].count('ScriptBytecodeAdapter.createList') == 4
String bytecode = astTrees.values()[0][1]
int offset = bytecode.indexOf('test()V')
bytecode = bytecode.substring(offset, bytecode.indexOf('RETURN', offset))

assert bytecode.count('ScriptBytecodeAdapter.createList') == 0 // GROOVY-8699
assert bytecode.count('INVOKESPECIAL java/util/ArrayList.<init>') == 3 // one for the spread result
}

// GROOVY-7688
@Test
void testSpreadSafeMethodCallReceiversWithSideEffectsShouldNotBeVisitedTwice() {
assertScript '''
class Foo {
static void test() {
def list = ['a', 'b']
def lengths = list.toList()*.length()
assert lengths == [1, 1]
}
void test() {
def list = ['a', 'b']
def lengths = list.toList()*.length()
assert lengths == [1, 1]
}
Foo.test()
test()
'''
assert astTrees['Foo'][1].count('DefaultGroovyMethods.toList') == 1
String bytecode = astTrees.values()[0][1]
int offset = bytecode.indexOf('test()V')
bytecode = bytecode.substring(offset, bytecode.indexOf('RETURN', offset))

assert bytecode.count('DefaultGroovyMethods.toList') == 1
}

@Override @Test
Expand Down
Loading