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

[David Holmes]

On 17/03/2016 5:20 AM, Coleen Phillimore wrote:
> 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.

I don't I'm afraid - it is yet another special case and no automatic 
dropping of the message when we switch to JDK 10. Can't it be folded 
into the AliasedLoggingFlag support? I'm really not seeing why the 
conversion of the non-product flags to UL has to be handled differently 
to the product ones ??

Aren't these just deprecated flags? We internally convert them to their 
replacement form and issue a warning that they are going away?

David

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