RFR (S): 8065070: (fmt) Avoid creating substrings when building FormatSpecifier

Martin Buchholz martinrb at google.com
Mon Nov 17 17:31:36 UTC 2014


I'll add my reviewer-bit (let's give more good people reviewer bits!).
Looks good!

(but also demonstrates deep existing problems with substring change)

Not a defect with this change, but it looks to me like those
assert(false) will trigger if e.g. a number is specified that is
greater than Integer.MAX_VALUE.

2632                 } catch (NumberFormatException x) {
2633                     assert(false);

On Mon, Nov 17, 2014 at 4:29 AM, Aleksey Shipilev
<aleksey.shipilev at oracle.com> wrote:
> On 11/17/2014 03:23 PM, Claes Redestad wrote:
>>
>> On 2014-11-17 12:54, Aleksey Shipilev wrote:
>>>> Perhaps rewriting to something like this would make the code
>>>> cleaner:
>>>>
>>>>                index(s, m.start(1), m.end(1));
>>>>                flags(s, m.start(2), m.end(2));
>>>>                width(s, m.start(3), m.end(3));
>>>>                precision(s, m.start(4), m.end(4));
>>>>                  int tTStart = m.start(5);
>>>>                if (tTStart >= 0) {
>>>>                    dt = true;
>>>>                    if (s.charAt(tTStart) == 'T') {
>>>>                        f.add(Flags.UPPERCASE);
>>>>                  }
>>>>                }
>>>>                conversion(s.charAt(m.start(6)));
>>> Yes please.
>>>
>>>> (I noticed that getting and checking tTEnd is basically redundant,
>>>> since the formatSpecifier regex guarantees either one char or
>>>> nothing is matched for that group)
>>> That makes sense.
>>
>> New webrev including the above cleanup:
>>
>> http://cr.openjdk.java.net/~redestad/8065070/webrev.01
>
> Good, thank you!
>
> -Aleksey.
>
>



More information about the core-libs-dev mailing list