ArrayStoreException in ArrayUtils.append()

Joel,

I am going to pass this one back to you, I am not sure how to fix this properly.

The problem is in SubMenu.getKeyCommands() calling ArrayUtils.append(myC, c) to append the existing keycommands (c) to the submenu keycommands (myC).

ArrayUtils.append(x, y) creates a new array of the same type as x and copies the elements of x any y into it.

However, in SubMenu, x (SubMenuKeyCommand) is a subclass of y (KeyCommand), so we get an ArrayStoreExceptiontrying to stuff KeyCommands into a SubMenuKeyCommands array.

The pre-arrayUtils version of SubMenu correctly created the target array as type KeyCommand and so does not fail.

Easy fix is to just restore the old code that calls System.arraycopy directly.

A harder fix (and beyond me) is to modify ArrayUtils.append() to check the types of all the arguments for subclasses and create the array of type of the basest class.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

There was one in my builds directory very briefly.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

No problem Jeff,

It’s good to have the latest trunk being tested as well. This is one of those bugs that would not show up in many modules, it only occurs on counters with Submenus.

Be careful about building modules with 3.2 though. The Zip file handling is broken at the moment, and as we get more changes in, it will generate modules that will generate bug reports in 3.1.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

No it isn’t. That should work now.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

When I edit an existing module and go into a BasicPiece, there are no image names in the drop-down list. If I try and add a new image, I get


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

Are you saying that a SubMenuKeycommand is being created? Can you show me
where that’s coming from?


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

VASSAL.counters.SubMenu.java lines 41, 73 and 74


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

I think we’re doing something dangerous by having myGetKeyCommands()
widen the type of the array it returns to KeyCommand, due to the way
that Java handles arrays. If this were C++ and we had an array of
SubMenuKeyCommand pointers, we we could also treat it as an array of
KeyCommand pointers in any context which called for an array of KeyCommand
pointers because arrays in C++ aren’t objects, they’re just contiguous
blocks of memory. Java will implicitly downcast a SubMenuKeyCommand
to a KeyCommand for you, but then you’re stuck with the problem that
the array is only a KeyCommand by polymorphism, and you can’t later
recover any information about the downcast from it. Since the only way to
find out the type of an array is by reflection, if your SubMenuKeyCommand
is downcast to a KeyCommand and then later you need to copy the thing,
it looks like a SubMenuKeyCommand again (due to getComponentType()
returning the actual component type), not a KeyCommand like we wanted.

This would not be a problem at all if Java didn’t have both arrays as
objects and generic type erasure. (Without type erasure, it would be
possible to say “new T”, where T is a generic type, and thereby avoid
this while problem.) It’s annoying that you can’t treat a Foo which was
declared as a Foo and a Foo which was declared as a Bar (where Bar is
a subclass of Foo) in the same fashion. I thought that was the whole point
of polymorphism.

There are two ways around this that I can see:

  1. myGetKeyCommands() should return a real KeyCommand, or

  2. There should be another ArrayUtils.append() which takes as an argument
    the Class which should be the component type of the returned array, so
    like this:

public static T append(Class<? super T> type, T a, T… b);

Do you have a preference for or the other one of these (or both)?


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

All this with the caveat that I’m not actually doing any of the
programming.

On Feb 11, 2009, at 5:41 AM, Joel Uckelman wrote:

This seems like the simplest solution. And it also seems to fit the
name of the method a little better.

The question is whether there is anybody who calls this that uses any
methods that are specific to the SubMenuKeyCommand type, so that they
would need to cast the elements of the array to that type.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake Thomas Russ:

Now that I look at it, the array returned by SubMenu.myGetKeyCommands() is
private, so no other class should be expecting it to return a KeyCommand
which is also a KeyCommandSubMenu. It would violate encapsulation and be
type-unsafe if anyone was relying on that behavior.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

That would seem to be the way to go.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

I ended up doing the second thing, as it protects us from custom code which
downcasts arrays—but we might still want to go back and do the first thing
as well. (The changes would need to be made to SubMenu and DynamicProperty.)

There’s another version of ArrayUtils.append() now, for use when you want
to explicitly specify the type of the output array. The signature looks
like this:

<T,X extends T,Y extends T> T append(Class<T> type, X a, Y… b);

So, if you try to use this, it won’t compile unless the contents of the head
and tail both can be put into an array of the requested type—which I think
should wrap up the problem.

I’ve committed the fix to trunk@5038. Would you check to see if this works
for you?


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake Joel Uckelman:

Brent?


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Sorry…Yes, that fixes the problem.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

As of trunk@5169, I’ve also implemented the other solution, whereby
myGetKeyCommands() returns only a KeyCommand which is properly a
KeyCommand, so I’ve now closed this bug.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)