RFR: 8027314: Java should recognize Diagnostic options if -XX:+UnlockDiagnosticVMOptions is not specified and print an informative message

[Christian Thalinger]

Looks good.  Thanks.

On Jan 15, 2014, at 9:06 PM, Calvin Cheung <calvin.cheung at oracle.com> wrote:

> On 1/15/2014 6:43 PM, Christian Thalinger wrote:
>> On Jan 15, 2014, at 11:34 AM, Calvin Cheung <calvin.cheung at oracle.com> wrote:
>> 
>>> On 1/15/2014 11:22 AM, Christian Thalinger wrote:
>>>> On Jan 15, 2014, at 10:56 AM, Calvin Cheung <calvin.cheung at oracle.com> wrote:
>>>> 
>>>>> Christian,
>>>>> 
>>>>> Thanks for your review.
>>>>> I've made some changes based on your suggestion.
>>>>> You can view the webrev at the same location: http://cr.openjdk.java.net/~ccheung/8027314/webrev/
>>>>> Only globals.[c|h]pp have been modified.
>>>>> Previous version of webrev is at: http://cr.openjdk.java.net/~ccheung/8027314/webrev.00/
>>>> These two webrevs seem to be the same.
>>> They're different. If you look at the diff of globals.hpp:
>>> http://cr.openjdk.java.net/~ccheung/8027314/webrev/src/share/vm/runtime/globals.hpp.cdiff.html
>>> 
>>> +   static bool is_product_build();
>>> 
>>> 
>>> 
>>> http://cr.openjdk.java.net/~ccheung/8027314/webrev.00/src/share/vm/runtime/globals.hpp.cdiff.html
>>> 
>>> +   bool is_product_build() const;
>> Oh.  What I meant was making is_product_build() static local method, not a class method:
>> 
>> +static bool is_product_build() {
>> +#ifdef PRODUCT
>> +  return true;
>> +#else
>> +  return false;
>> +#endif
>> +}
> I've updated the webrev. It is at the same location:
>    http://cr.openjdk.java.net/~ccheung/8027314/webrev/
> 
> Please take a look.
> 
> thanks,
> Calvin
>> 
>>> Calvin
>>> 
>>>>> Calvin
>>>>> 
>>>>> On 1/14/2014 7:01 PM, Christian Thalinger wrote:
>>>>>> +bool Flag::is_product_build() const {
>>>>>> 
>>>>>> This is an odd method but I can see why you have it.  Making it a static local method would be better though.
>>>>>> 
>>>>>> Otherwise this looks good.  Thanks for fixing the "Did you mean” message; one of my changes broke that one.
>>>>>> 
>>>>>> On Jan 14, 2014, at 5:22 PM, Calvin Cheung <calvin.cheung at oracle.com> wrote:
>>>>>> 
>>>>>>> This fix is for clarifying the error messages associated with vm options of type
>>>>>>> diagnostic, experimental, develop, and notproduct.
>>>>>>> 
>>>>>>> Summary of fix:
>>>>>>> - added an optional boolean (return_flag) parameter in Flag::find_flag() so that
>>>>>>> Arguments::process_argument() can inspect the returned flag and call
>>>>>>> into get_locked_message();
>>>>>>> - added new function is_product_build() in globals.[c|h]pp;
>>>>>>> - in get_locked_message(), issues the appropriate error message;
>>>>>>> - added a new test VMOptionWarning.java;
>>>>>>> - minor cleanup in the existing tests - ConfigFileWarning, CompilerConfigFileWarning.
>>>>>>> 
>>>>>>> Sample output:
>>>>>>> ---------------
>>>>>>> 
>>>>>>> Before fix:
>>>>>>> 
>>>>>>> C:\jdk8_i586\jdk1.8.0\bin\java -XX:+PrintInlining -version
>>>>>>> Improperly specified VM option 'PrintInlining'
>>>>>>> Error: Could not create the Java Virtual Machine.
>>>>>>> Error: A fatal exception has occurred. Program will exit.
>>>>>>> 
>>>>>>> After fix:
>>>>>>> 
>>>>>>> C:\jdk8_i586\jdk1.8.0\bin\java -XX:+PrintInlining -version
>>>>>>> Error: VM option 'PrintInlining' is diagnostic and must be enabled via -XX:+UnlockDiagnosticVMOptions.
>>>>>>> Error: Could not create the Java Virtual Machine.
>>>>>>> Error: A fatal exception has occurred. Program will exit.
>>>>>>> 
>>>>>>> Before fix:
>>>>>>> 
>>>>>>> c:\jdk9_i586\jdk1.9.0\bin\java  -XX:+TraceJNICalls -version
>>>>>>> Unrecognized VM option 'TraceJNICalls'
>>>>>>> Did you mean '(+/-)TraceJNICalls'?
>>>>>>> Error: Could not create the Java Virtual Machine.
>>>>>>> Error: A fatal exception has occurred. Program will exit.
>>>>>>> 
>>>>>>> After fix:
>>>>>>> 
>>>>>>> C:\jdk8_i586\jdk1.8.0\bin\java -XX:+TraceJNICalls -version
>>>>>>> Error: VM option 'TraceJNICalls' is develop and is available only in debug version of VM.
>>>>>>> Error: Could not create the Java Virtual Machine.
>>>>>>> Error: A fatal exception has occurred. Program will exit.
>>>>>>> 
>>>>>>> Testing:
>>>>>>>    manual test via command line and the new test VMOptionWarning.
>>>>>>> 
>>>>>>> Bug:
>>>>>>>    https://bugs.openjdk.java.net/browse/JDK-8027314
>>>>>>> 
>>>>>>> Webrev:
>>>>>>>    http://cr.openjdk.java.net/~ccheung/8027314/webrev/
>>>>>>> 
>>>>>>> thanks,
>>>>>>> Calvin