[
https://issues.apache.org/jira/browse/GROOVY-7656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14985999#comment-14985999
]
Jochen Theodorou commented on GROOVY-7656:
------------------------------------------
your solution idea has two potential problems:
(1) you declare a variable, which might be already declared. There is no check
for this at this point, but imagine what happens should this be used inside the
expression you are spreading
(2) your temporary variable will show up as declared variable during debugging,
potentially confusing people
Looking at the code you basically do what Cedric did, only that Cedric did that
part wrong already. Maybe he had a good reason for using this, don't know.
Normally I use defineTemporaryVariable from the compile stack for this
(controller.getCompileStack()). But since you need it as node, and not just as
a slot number later on I suggest you use then my ExpressionAsVariableSlot or
Cedrics TemporaryVariableExpression (which he added after he wrote this part of
the code, which is why it is not used there). Beware, you will need to "remove"
the temporary variable after use. Either by using the compile stack, or if you
use TemporaryVariableExpression, the remove method on the expression.
> Spread safe method calls on list literals result in the list expression being
> evaluated twice (SC)
> --------------------------------------------------------------------------------------------------
>
> Key: GROOVY-7656
> URL: https://issues.apache.org/jira/browse/GROOVY-7656
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation
> Affects Versions: 2.4.5
> Reporter: Shil Sinha
> Assignee: Cédric Champeau
> Priority: Critical
>
> When a list literal is the receiver for a statically compiled spread safe
> method call expression, the instructions to create the list are included
> twice. Example:
> {code}
> @groovy.transform.CompileStatic
> void test() {
> ['a']*.size()
> }
> {code}
> The bytecode for the method above is:
> {code}
> L0
> LINENUMBER 3 L0
> NEW java/util/ArrayList
> DUP
> INVOKESPECIAL java/util/ArrayList.<init> ()V
> ASTORE 1
> L1
> ALOAD 1
> POP
> ICONST_1
> ANEWARRAY java/lang/Object
> DUP
> ICONST_0
> LDC "a"
> AASTORE
> INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.createList
> ([Ljava/lang/Object;)Ljava/util/List;
> IFNULL L2
> ACONST_NULL
> ASTORE 2
> L3
> ICONST_1
> ANEWARRAY java/lang/Object
> DUP
> ICONST_0
> LDC "a"
> AASTORE
> INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.createList
> ([Ljava/lang/Object;)Ljava/util/List;
> INVOKEINTERFACE java/util/List.iterator ()Ljava/util/Iterator;
> ASTORE 3
> L4
> ALOAD 3
> INVOKEINTERFACE java/util/Iterator.hasNext ()Z
> IFEQ L2
> ALOAD 3
> INVOKEINTERFACE java/util/Iterator.next ()Ljava/lang/Object;
> ASTORE 2
> ALOAD 1
> ALOAD 2
> DUP
> ASTORE 4
> IFNULL L5
> ALOAD 4
> INVOKESTATIC
> org/codehaus/groovy/runtime/typehandling/ShortTypeHandling.castToString
> (Ljava/lang/Object;)Ljava/lang/String;
> CHECKCAST java/lang/String
> INVOKESTATIC org/codehaus/groovy/runtime/StringGroovyMethods.size
> (Ljava/lang/String;)I
> INVOKESTATIC java/lang/Integer.valueOf (I)Ljava/lang/Integer;
> GOTO L6
> L5
> ACONST_NULL
> L6
> INVOKEVIRTUAL java/util/ArrayList.add (Ljava/lang/Object;)Z
> POP
> GOTO L4
> L2
> ALOAD 1
> POP
> {code}
> If the list expression contains method calls that have side effects, this can
> cause serious problems. The example below is trivial, but shows the general
> case:
> {code}
> @groovy.transform.CompileStatic
> void test() {
> def list = ['abc']
> def lengths = [list.removeAt(0)]*.length() // throws
> IndexOutOfBoundsException
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
