[9] RFR(S): 8059847: complement JDK-8055286 and JDK-8056964 changes

[Vladimir Kozlov]

Hi Zoltan,

You don't need /othervm if there  are no additional flags.

Instead of /startup/ I would suggest /oracle/ subdir name since we testing code in compilerOracle.

42     // Type (1) is used to check if a flag "someflag" is enabled for a
43     // method.

I would say:

42     // Type (1) is used to enable a boolean flag for a method.

Should it be "bool,MyBoolOption,100"?:

  135             // wrong value for bool flag
  136             "-XX:CompileCommand=option,Test::test,bool,MyBoolOption,intx,100",

Same problem with next test which does not match the comment:

140             // intx flag name missing
141             "-XX:CompileCommand=option,Test::test,bool,MyBoolOption,false,intx,100",


Please, separate next checks to be clear which one is incorrect:

  179         if (TYPE_1_ARGUMENTS.length != TYPE_1_EXPECTED_OUTPUTS.length
  180             || TYPE_2_ARGUMENTS.length != TYPE_2_EXPECTED_OUTPUTS.length) {
  181             throw new RuntimeException("Test is set up incorrectly: length of arguments and expected outputs does 
not match.");
  182         }

Thanks,
Vladimir

On 10/8/14 7:22 AM, Zoltán Majó wrote:
> Hi David and Vladimir,
>
>
> thank you for your feedback.
>
> On 10/07/2014 10:46 PM, Vladimir Kozlov wrote:
>> On 10/7/14 11:06 AM, David Chase wrote:
>>
>>> Do we need to add a test (? what would it look like?) to ensure that it works?
>>
>> Yes, please, add a regression test.
>
> I added a regression test ('compiler/startup/CheckCompileCommandOption.java'). Here is the updated webrev:
>
> http://cr.openjdk.java.net/~zmajo/8059847/webrev.01/
>
> Thank you and best regards,
>
>
> Zoltan
>
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> David
>>>
>>> On 2014-10-07, at 1:53 PM, Zoltán Majó <zoltan.majo at oracle.com> wrote:
>>>
>>>> Hi,
>>>>
>>>>
>>>> please review the following small patch.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8059847
>>>>
>>>> Problem: CompileCommand=option does not support flags of type double.
>>>>
>>>> Solution: Add support for flags of type double. Output:
>>>>
>>>> java -XX:CompileCommand=option,Test::test,ccstrlist,MyListOption,_foo,_bar
>>>> -XX:CompileCommand=option,Test::test,ccstr,MyStrOption,_foo
>>>> -XX:CompileCommand=option,Test::test,bool,MyBoolOption,false
>>>> -XX:CompileCommand=option,Test::test,intx,MyIntxOption,-1 -XX:CompileCommand=option,Test::test,uintx,MyUintxOption,1
>>>> -XX:CompileCommand=option,Test::test,MyFlag -XX:CompileCommand=option,Test::test,double,MyDoubleOption,1.123 -version
>>>>
>>>> CompilerOracle: option Test.test const char* MyListOption = '_foo _bar'
>>>> CompilerOracle: option Test.test const char* MyStrOption = '_foo'
>>>> CompilerOracle: option Test.test bool MyBoolOption = false
>>>> CompilerOracle: option Test.test intx MyIntxOption = -1
>>>> CompilerOracle: option Test.test uintx MyUintxOption = 1
>>>> CompilerOracle: option Test.test bool MyFlag = true
>>>> CompilerOracle: option Test.test double MyDoubleOption = '1.123000'
>>>> java version "1.9.0-ea"
>>>> Java(TM) SE Runtime Environment (build 1.9.0-ea-b33)
>>>> Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-fastdebug-internal, mixed mode)
>>>>
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~zmajo/8059847/webrev.00/
>>>>
>>>> Testing: JPRT
>>>>
>>>> Thank you and best regards,
>>>>
>>>>
>>>> Zoltan
>>>>
>>>
>