Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 1.7.2
-
Fix Version/s: 1.7.4, 1.8-beta-1
-
Component/s: None
-
Labels:None
-
Environment:Windows 7 64 bit
Description
I have found an issue with array incrementation, when using randomly generated number as array index.
The idea was to randomly increment one of the indexes.
For array of size = 3 and 10 incrementations I can see array was modified like this [example from system.out]:
[0, 0, 1]
[0, 0, 1]
[2, 0, 1] should be [1,0,1]
[2, 3, 1] should be [1,1,1]
[3, 3, 1] should be [2,1,1]
[3, 2, 1] now I'm puzzled as the digit is decremented!
[3, 2, 1] what was incremented? no one knows! ;]
[3, 2, 2]
[3, 2, 3]
[3, 3, 3]
See the code attached.
Additional info:
When the randomiser invocation is moved to assign value to a variable, and then variable is used as array index it works fine.
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
The given code consistently works if I change
arr[rand()]++
to
int x = rand()
arr[x]++
More after some more investigation.
Simultaneous posts. Looks like Jochen already investigated the cause.
Hello.
Attaching here a patch for review. Here is how the bytecode looks like after applying the patch for the postfix evaluation part of "arr[exp]++":
Object tmp = exp; Object localObject1 = "getAt".call(arr, tmp); Object localObject2 = "next".call(localObject1); "putAt".call(arr, tmp, localObject2); return localObject1;
As captured in the test, the rand() expression is now correctly evaluated only 10 times instead of 30.
good work
Thanks Jochen. 
Thanks guys. Looking forward to stable release with this fix.
Groovy will become groovier.
after playing around a bit I think that the call to rand() is done multiple times... I added a simple counter to the method and found, that rand() is called 30 here. I think that a[exp]++ is converted into:
while the correct code should look more like:
arguably the code could be improved further to do:
This would save one more getAt call, increasin performance a little.