RFR (S) 8136500: Integer/Long getChars and stringSize should be more idiomatic
ivan.gerasimov at oracle.com
Mon Nov 23 13:34:51 UTC 2015
> 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:
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.
> 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.
More information about the core-libs-dev