RFR(s): 8178364: Command-line flags of type double should accept integer values

[Per Liden]

Hi David,

On 2017-04-10 13:03, David Holmes wrote:
> On 10/04/2017 5:40 PM, Per Liden wrote:
>> Hi David,
>>
>> On 2017-04-10 09:27, David Holmes wrote:
>>> Hi Per,
>>>
>>> On 10/04/2017 5:05 PM, Per Liden wrote:
>>>> Hi,
>>>>
>>>> For convenience, command-line flags of type double should accept
>>>> integer
>>>> values (as well as the normal double values). This would allow users to
>>>> write the short form, e.g:
>>>>
>>>> -XX:G1ConcMarkStepDurationMillis=10
>>>>
>>>> instead of always having to do:
>>>>
>>>> -XX:G1ConcMarkStepDurationMillis=10.0
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8178364
>>>> Webrev: http://cr.openjdk.java.net/~pliden/8178364/webrev.0/
>>>> Testing: JTreg test added, passes jprt
>>>
>>> I don't understand why we aren't using set_fp_numeric_flag in this case
>>> - and surely that already handles this case ??
>>
>> With the way this code is structured we only end up in
>> set_fp_numeric_flag() if the string looks like a float to begin with,
>> i.e. has the format "<flag>=<num>.<num>". If the flag looks like
>> "<flag>=<num>" we end up in set_numeric_flag(). Have a look at the call
>> sites for set_fp_numeric_flag() and set_numeric_flag() and you'll see.
>
> That's somewhat odd - shouldn't the type of the flag dictate how we try
> to parse it ??

Indeed, Arguments::parse_argument() does things a bit backwards if you 
ask me, by "guessing" what's on the other side of the "=" instead of 
letting the flag type dictate how it's parsed. Maybe there is (or was) a 
reason for doing so that I don't see.

Anyway, I think my patch is in line with the style/intent of the current 
code. Overhauling Arguments::parse_argument() is probably a good idea, 
but I think that's a separate enhancement.

cheers,
Per

>
> David
>
>> cheers,
>> Per
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> cheers,
>>>> Per
>>>>