RFR: 8146632: Add descriptive error messages for removed non-product logging flags.

[Coleen Phillimore]

Okay, I didn't see your reply.  I also thought that storing a version in 
the table might be helpful for documentation purposes so we know in the 
future when to remove the line from the table.  I agree with your 
implementation choice to have an additional table rather than twisting 
the other table to cover this use case.

http://cr.openjdk.java.net/~mockner/8146632.02/src/share/vm/runtime/arguments.cpp.udiff.html

I think Harold's refactoring makes sense and I think the #endif  // PRODUCT

}
*+ #ifndef PRODUCT*
*+ else if ((replacement = 
removed_develop_logging_flag_name(stripped_argname)) != NULL){*
*+ jio_fprintf(defaultStream::error_stream(),*
*+ "%s has been removed. Please use %s instead.\n", stripped_argname, 
replacement);*
*+ return false;*
*+ #endif*
*+ }*


I think this should be like this so the {} match inside of #ifndef PRODUCT:

*+ #ifndef PRODUCT*
*+ } else if ((replacement = 
removed_develop_logging_flag_name(stripped_argname)) != NULL) {*
*+ jio_fprintf(defaultStream::error_stream(),*
*+ "%s has been removed. Please use %s instead.\n", stripped_argname, 
replacement);*
*+ return false;*
*+ #endif*
*+ }*


Or refactor as Harold suggested.

Coleen

On 3/16/16 3:04 PM, Max Ockner wrote:
> I did, but it blended in with the rest of the text
>
> My response: "Seems appropriate to report a specific error message for 
> 9 and then remove it for 10. If it would help, we can store a Version 
> in the table to keep track of when each entry needs to be deleted, 
> like what is done in the table of obsolete flags. "
>
>
> On 3/16/2016 2:24 PM, Coleen Phillimore wrote:
>>
>> You should also answer David's concern.
>> Coleen
>>
>> On 3/16/16 2:05 PM, Max Ockner wrote:
>>> webrev: http://cr.openjdk.java.net/~mockner/8146632.02/
>>>  - Labeled #endif with // PRODUCT
>>>  - refactored table lookup code to only do lookup once.
>>>  - Added VerboseVerification to the table.
>>>
>>> Comments below.
>>>
>>> On 3/16/2016 1:48 AM, David Holmes wrote:
>>>> Hi Max,
>>>>
>>>> On 16/03/2016 3:45 AM, Max Ockner wrote:
>>>>> Hello again everyone!
>>>>>
>>>>> Please review this change which adds better error messages for
>>>>> non-product flags that are now converted to Unified Logging.
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8146632
>>>>> webrev: http://cr.openjdk.java.net/~mockner/8146632/
>>>>>
>>>>> Since these options have been removed, we want still want the vm to
>>>>> crash here, but now with an error message giving the correct command
>>>>> line option. The new message looks like this:
>>>>>
>>>>>  > TraceClassInitialization has been removed. Please use 
>>>>> -Xlog:classinit
>>>>> instead."
>>>>>
>>>>> The entire output looks like this:
>>>>>
>>>>>  > TraceClassInitialization has been removed. Please use 
>>>>> -Xlog:classinit
>>>>> instead.
>>>>>  > Error: Could not create the Java Virtual Machine.
>>>>>  > Error: A fatal exception has occurred. Program will exit.
>>>>
>>>> I'm concerned that this has introduced another variant of "flag 
>>>> deprecation". It begs the question as to when this new code should 
>>>> be removed. Maybe we need to add "replaced" as another type of flag 
>>>> change so we can report in 9 the flag has been replaced and then in 
>>>> 10 just report an "unknown option" error ?
>>>>
>>>> Thanks,
>>>> David
>>>>
>>> Seems appropriate to report a specific error message for 9 and then 
>>> remove it for 10. If it would help, we can store a Version in the 
>>> table to keep track of when each entry needs to be deleted, like 
>>> what is done in the table of obsolete flags.
>>>>> Tested with jtreg runtime tests. A new test checks for an appropriate
>>>>> error message for every develop flag that has so far been 
>>>>> converted to
>>>>> logging.
>>>>>
>>>>> Thanks,
>>>>> Max
>>>>>
>>>>>
>>>
>>
>