What sanity checks you had in mind? Because
java/lang/Integer/IntegerDecode.java fails, and it fails *in compiler*.
Oh, how embarrassing! Overlooked a sign :(
I had tested it only with new lang/Integer/ToString.java, in a hope it
gives enough coverage for sanity, but I was wrong.
I tried to see what might be wrong, but failed.
The mere fact the debugging is hard for this code tells me that we are
much better off having cleaner code that handles MIN_VALUE separately.
If you still want MIN_VALUE to go away, then file a follow up, and try
to disentangle those weird failures ;)
I don't think the change is too complicated.
It just moves all the calculations into negative area, so we only need
to be careful with signs :)
With this fixed patch:
http://cr.openjdk.java.net/~igerasim/8136500/8136500-addition-1.patch
all tests from test/lang pass.
Would you give it another chance?
Also, with this kind of change, AbstractStringBuilders would *still*
have the MIN_VALUE checks, *plus* all the additional handling logic in
stringSize and getChars, that will run for positive values.
Incorporating this into ASB will result in removal of one if-statement
altogether, so it seems to be a clear win.
Of course, it can be done later as a separate fix.
Sincerely yours,
Ivan
You can
explore removing that as well, but it makes sense only after Indify
String Concat lands and brings its own stringSize/getChar usages (and
tests!) on the table.
Do you think it's worth it to reorganize the code this way?
No, I think the perfect is the enemy of done here.
Thanks,
-Aleksey
