negative characters? prefer positive actors

Hi Paul,

On Fri, 2009-09-18 at 19:35 -0700, Paul Phillips wrote:
> Was working my way toward fixing #2087:
>
> http://lampsvn.epfl.ch/trac/scala/ticket/2087
> "short/char comparisons are wrong"
>
> ...when I encountered this interesting breakage. I had naively thought
> that the unsignedness of char was something enforced by the JVM, but by
> observation, such is not the case.

I had a brief look and this can be reproduced in in 2.7.5 and r18718 by
changing the code a bit:

object ShortToChar {
def foo = new (Short => Unit) { def apply(s: Short) {
Character.valueOf(s.toChar)
}
}
def main(args: Array[String]) {
foo(-100)
}
}

> I'll just paste the patch here, it
> explains the situation. I don't fully understand all the
> boxing/unboxing logic in GenJVM, but whatever is supposed to do the job
> of keeping chars positive is sometimes getting cut out of the loop.

It seems to me that a i2c is missing here:

public void apply(short);
Code:
Stack=1, Locals=2, Args_size=2
0: iload_1
1: invokestatic #46; //Method
java/lang/Character.valueOf:(C)Ljava/lang/Character;
4: pop
5: return
LineNumberTable:
line 5: 0

> public static Character boxToCharacter(char c) {
> - return Character.valueOf(c);
> + // !!! Temporarily working around the "impossible" (?) fact that
> + // c can have a negative value here. In any revision since r17461 try:
> + // def foo = new (Short => Char) { def apply(x: Short) = x.toChar }
> + // foo(-100)
> + // and the -100 will get to Character, which will duly crash.
> + // The bug was masked before because the Characters were created
> + // with "new Character(c)" and the constructor avenue must have
> + // some check against negative values, whereas the static method doesn't

It's not what new Character(c) has, but what it doesn't have.
Character.valueOf does:

public static Character valueOf(char c) {
if(c <= 127) { // must cache
return CharacterCache.cache[(int)c];
}
return new Character(c);
}

Due to the missing cast, the if succeeds with a negative value and we
end up indexing into the array with it.

Hope it helps,
Ismael