Collection.toArray(new Type[0]) returns null

[GoogleCodeExporter]

http://www.codenameone.com/blog/the-toarraynew-array-antipattern
This is a bug.  Need to fix this.

Original issue reported on code.google.com by st...@weblite.ca on 27 Jan 2015 at 6:15

[GoogleCodeExporter]

I suggest you spend your time on more productive things. This isn't fixable.

Original comment by shai.almog on 27 Jan 2015 at 6:17

[GoogleCodeExporter]

This bug is wasting my time when trying to achieve productive things.  I don't 
see why this wouldn't be fixable.

E.g. My task to flush bugs out of the NewVM which is VERY high priority.  Most 
of my libraries don't work on the NewVM for various reasons.   Having to fix 
all of these issues in libraries just to see if there are any other issues in 
the NewVM that cause it to crash is a time sink.  I just spent 30 minutes going 
through the charts library removing all instances of this.  Potentially every 
piece of code I've ever written now must be re-analyzed to see if it includes 
this function - which is valid java.

I'll do this in my own time. Every behaviour where the new VM deviates from 
standard Java makes it less like java, and thus reduces confidence in the code 
that is deployed.  IMO The new VM should be as close in behaviour to regular 
java as possible 

Original comment by st...@weblite.ca on 27 Jan 2015 at 6:27

[GoogleCodeExporter]

Its not fixable since you can't allocate the right array type its easy to do 
new Object[x] but not so much new TypeOfObjectPassedToTheMethod[x]. So this 
isn't fixable.
I don't see what's so difficult about right click find "toArray(new ". 

Original comment by shai.almog on 27 Jan 2015 at 6:43

[GoogleCodeExporter]

>"Its not fixable since you can't allocate the right array type its easy to do 
new Object[x] but not so much new TypeOfObjectPassedToTheMethod[x]. So this 
isn't fixable."

We have a working Class.newInstance() implementation.  Couldn't we use a 
similar strategy to implement Array.newInstance()?

>"I don't see what's so difficult about right click find "toArray(new ". "

It's death by a thousand cuts.  When porting code into codename one, we already 
need to do a number of things to get things to work (e.g. replace missing class 
references with ones in cn1, etc...).  This particular thing is harder to catch 
because the compiler won't catch it.  So it means I have to also remember to do 
a find for all instances of toArray() and see if they are being passed a 
zero-length array as input.  It's not impossible to do this, just annoying.

It's 

Original comment by st...@weblite.ca on 27 Jan 2015 at 6:53

[GoogleCodeExporter]

It would work to instantiate a 0 length array but to create a new instance of 
an array with a given size you would need an entirely different method. 

Perhaps a better way is to throw an exception rather than return null. It might 
also be more practical to fix the JavaSE port so it will fail in a similar way 
when invoking the toArray method.

I think you are making a bigger deal of it than it is. The old VM has so many 
bugs that are WAY worse than this e.g. if(Object[] instanceof byte[]) would 
fail. You literally couldn't compare array types and we have a shitload of 
workarounds in the code for those things.

Original comment by shai.almog on 27 Jan 2015 at 7:08

[GoogleCodeExporter]

Original comment by st...@weblite.ca on 28 Jan 2015 at 12:46

[GoogleCodeExporter]

I'll stipulate that it isn't a critical bug.  Just that it's a bug.  I'm sure 
that, once I'm more familiar with the new vm's architecture, I'll be able to 
spot a relatively easy way to add this.  I'll admit that I don't see the full 
solution right now.

Original comment by steve.ha...@codenameone.com on 28 Jan 2015 at 4:49

[GoogleCodeExporter]

Are you aware that this will happen on J2ME/RIM devices as well?

The right solution will be to somehow simulate this.

Original comment by shai.almog on 28 Jan 2015 at 5:04

[GoogleCodeExporter]

No, I wasn't aware of that.  Does Class.newInstance() work on those devices?

Original comment by steve.ha...@codenameone.com on 28 Jan 2015 at 5:21

[GoogleCodeExporter]

Yes but not for arrays (due to lack of size argument) obviously.

Original comment by shai.almog on 28 Jan 2015 at 6:26

[GoogleCodeExporter]

A short-term solution would be to just throw an exception then.  Long term, I 
think I can come up with a solution that uses bytecode manipulation, similar to 
retroweaver/retrolambda to fix this case for all platforms.

Original comment by steve.ha...@codenameone.com on 28 Jan 2015 at 6:32

[GoogleCodeExporter]

Bytecode manipulation can't fix this. It can fix the trivial case of:
toArray(new X[0]) but it can't fix toArray(obj) where you have no idea where 
obj came from.
Its really easy to see that this issue is "unfixable" without reflection. The 
only solution is to reproduce the behavior on the simulator (whether its an 
exception or null doesn't matter).

Original comment by shai.almog on 28 Jan 2015 at 6:43

[GoogleCodeExporter]

I can figure out the type of `obj` using static analysis.  I can use that 
information to do the bytecode manipulation.  I did a little bit of this type 
of stuff while I was working on the Mirah compiler, so I'm pretty sure this can 
work.

Original comment by steve.ha...@codenameone.com on 28 Jan 2015 at 6:46

[GoogleCodeExporter]

Not really because you can't infer an object that you didn't get from somewhere 
close. E.g. Util.readObject()
So you can say "oh that's an edge case", but I'd say that having a "more 
extreme" edge case makes matters WAY worse than having consistent behavior.

The level of static analysis you will need would also slow down the build 
process in a noticeable way and it just doesn't make sense to do it in order to 
allow what is obviously very broken code to exist.

Worse. Assuming you do that you would need to replicate the behavior correctly. 
How would you do that? Would you still allocate the 0 length array? What if 
code relies on the broken behavior (e.g. the zero array still existing). 

Either way you are digging a deep hole instead of solving a far simpler problem 
on the simulator level.

Original comment by shai.almog on 28 Jan 2015 at 6:53

[GoogleCodeExporter]

Fair enough.  I was thinking that it would be sufficient to just take the 
static type of the variable that was passed to it, but that doesn't take into 
account the it could be a subclass.

I guess an exception is the only acceptable approach for now.

In the future it might be nice to add a build hint to enable a newer API, that 
would allow the developer to optionally cut the ties to legacy platforms.  That 
would allow us to fix the String and Number class issues.  This could be part 
of such a move.   The need to still support J2ME and Blackberry seems to be the 
nail in the coffin of this feature.  

Original comment by steve.ha...@codenameone.com on 28 Jan 2015 at 7:24

[GoogleCodeExporter]

We would like to move out of support for legacy, I agree. Especially with Java 
8 support. Regardless even if we didn't need to support J2ME fixing this 
specific issue in the way you describe is too much work to persist what is 
essentially a bug in the JDK that should never have worked in the first place.

Original comment by shai.almog on 28 Jan 2015 at 7:48

[GoogleCodeExporter]

If we didn't have the J2ME issue, I would advocate adding just enough 
reflection to implement this feature.

Original comment by steve.ha...@codenameone.com on 28 Jan 2015 at 3:07

[GoogleCodeExporter]

I changed it to throw an exception.  I'll mark this fixed for now.
https://code.google.com/p/codenameone/source/detail?r=2055

Original comment by st...@weblite.ca on 30 Jan 2015 at 1:28

• Changed state: Fixed