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

[Aleksey Shipilev]

On 11/17/2014 02:49 PM, Claes Redestad wrote:
> On 2014-11-17 11:45, Aleksey Shipilev wrote:
>> On 11/17/2014 02:09 AM, Claes Redestad wrote:
>>> http://cr.openjdk.java.net/~redestad/8065070/webrev.00
>>> https://bugs.openjdk.java.net/browse/JDK-8065070
>> (Not a Reviewer) The change looks very sane given the capture ranges are
>> already available in Matcher.I wonder if you want to cache m.start()
>> and m.end() in locals while you are at it -- I would think this amounts
>> to moving tTStart, tTEnd a few lines before.
> 
> Thanks for taking a look at this!
> 
> Not sure how you mean locally caching m.start(idx) would
> improve things, as we never reuse the same start/end values.

Ah, right. Curse those inline side effects.

> 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.

-Aleksey.