[ping] RFR(L): 8221470: Print methods in exception messages in java-like Syntax.

[David Holmes]

Two follow ups ...

On 2/04/2019 11:26 pm, Lindenmaier, Goetz wrote:
> Hi David,
> 
>> Overall this looks good to me - a few minor nits/comments below.
> thanks!
> 
>> I've applied the patch and am running it through our internal build and
>> test system (tiers 1-3 initially).
>>
>> I have a suspicion there will be other tests that need to be updated -
>> possibly even JCK tests. Discovering those a-priori will be difficult
>> (simply running all the tests would take an extremely long time). Will
>> have a discussion about how best to handle those internally.
> 
> I ran most JCK test without problem. They usually don't check messages.
> I ran all hotspot, jdk, langtools, nashorn and jaxp test (except
> for headful tests).

Thanks for the additional testing info. I duplicated some of that but 
found no issues, other than a couple of closed tests.

>> src/hotspot/share/oops/method.cpp
>> Please put a blank line after each new method.
> Fixed.
> 
>> src/hotspot/share/oops/symbol.cpp
>>
>> +       os->print(".");
>> +     } else {
>> +       os->print("%c", start[i]);
>>
>> Please use os->put(char c) for individual characters.
> Fixed.
>   
>> The "start" name would seem better as "buf" to me.
> Hmm, buf to me is a local chunk of memory used temporarily.
> What about array_sig, class_sig?

Not really "sigs".

str? Else just leave it.

Thanks,
David
-----

>> +     } else if (start[i] == 'L') {
>> +       print_class(os, start+i+1, len-i-2);
>> Can you insert a comment that help explains the -2:
> Done.
> 
>> +   for(SignatureStream ss(this); !ss.is_done(); ss.next()) {
>> space after for (2 occurrences)
> Fixed.
> 
>> test/hotspot/jtreg/runtime/exceptionMsgs/methodPrinting/TestPrintingMeth
>> ods.java
>>
>> Not sure the special characters can be used directly in the sources. Can
>> they not be put in as unicode escapes at all places?
> I'll try what Ioi proposed.  I'll post a new webrev including that.
> 
> Best regards,
>    Goetz.
> 
> 
>>
>> ---
>>
>> Thanks,
>> David
>> -------
>>
>>
>> On 1/04/2019 12:32 pm, David Holmes wrote:
>>> Hi Goetz,
>>>
>>> I'm looking at this ...
>>>
>>> On 29/03/2019 8:26 pm, Lindenmaier, Goetz wrote:
>>>> Hi,
>>>>
>>>> Any interest in this change?
>>>
>>> I'm personally of two minds here because these VM generated exceptions
>>> are not only delivered to Java source code. I'd like to know how other
>>> language developers using the JVM runtime would view this.
>>>
>>> That aside if you're going to make a change like this then I think the
>>> full signature string has to be quoted in some way to delineate it
>>> within the larger message.
>>>
>>>> Should I split it to adapt the exceptions separately one-by-one to
>>>> make the change smaller and simplify the review?
>>>
>>> I don't think that is necessary.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>> I would propose to start out with AbstractMethodError only.
>>>>
>>>> Best regards,
>>>>     Goetz.
>>>>
>>>>
>>>>
>>>> From: Lindenmaier, Goetz
>>>> Sent: Tuesday, March 26, 2019 1:06 PM
>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>> Subject: RFR(L): 8221470: Print methods in exception messages in
>>>> java-like Syntax.
>>>>
>>>> Hi,
>>>>
>>>> A row of exceptions are thrown from the hotspot runtime.
>>>> They print methods with their JNI signatures. To increase
>>>> readability and resemblance to source code, this change proposes
>>>> to print them in a Java-like syntax.
>>>>
>>>> Some examples:
>>>> current method printouts:
>>>>
>>>> test.TeMe3_B.ma()V
>>>> test.TeMe3_B.ma(IZ[[BF)[[D
>>>> test.TeMe3_B.ma([[[Ljava/lang/Object;)[[Ltest/TeMe3_B;
>>>>
>>>> improved format:
>>>>
>>>> void test.TeMe3_B.ma()
>>>> double[][] test.TeMe3_B.ma(int, boolean, byte[][], float)
>>>> test.TeMe3_B[][] test.TeMe3_B.ma(java.lang.Object[][][])
>>>>
>>>> So far, Method::name_and_sig_as_C_string() is used to print
>>>> these messages.
>>>>
>>>> This change implements function Method::external_name() that prints
>>>> the better
>>>> format.
>>>> external_name() is chosen according to Klass::external_name().
>>>>
>>>> Printing the better format requires parsing the signature
>>>> Symbol. This is implemented in
>>>> void Symbol::print_as_signature_external_return_type(outputStream *os);
>>>> void Symbol::print_as_signature_external_parameters(outputStream *os);
>>>> These method names are chosen according to
>>>> Symbol::as_class_external_name().
>>>>
>>>> See this partial webrev for the new functions:
>>>> http://cr.openjdk.java.net/~goetz/wr19/8221470-exMsg-signature/01-
>> new_methods/
>>>>
>>>>
>>>> Also, I changed a lot of exception messages to use the new format.
>>>> This required to adapt a row of tests. I added a test to check
>>>> the signature printing does not regress.  For all these changes, see
>>>> the full webrev:
>>>> http://cr.openjdk.java.net/~goetz/wr19/8221470-exMsg-signature/01/
>>>>
>>>> I hope I detected all places where method signatures are printed to
>>>> exception messages.
>>>>
>>>> Best regards,
>>>>     Goetz.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>