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

[David Holmes]

Hi Goetz,

Overall this looks good to me - a few minor nits/comments below.

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.

---

src/hotspot/share/oops/method.cpp

Please put a blank line after each new method.

---

src/hotspot/share/oops/symbol.cpp

+       os->print(".");
+     } else {
+       os->print("%c", start[i]);

Please use os->put(char c) for individual characters.

--

The "start" name would seem better as "buf" to me.

--

+     } else if (start[i] == 'L') {
+       print_class(os, start+i+1, len-i-2);

Can you insert a comment that help explains the -2:

      } else if (start[i] == 'L') {
+      // Expected format: L<type name>;
        print_class(os, start+i+1, len-i-2);

--

+   for(SignatureStream ss(this); !ss.is_done(); ss.next()) {

space after for (2 occurrences)

---

 
test/hotspot/jtreg/runtime/exceptionMsgs/methodPrinting/TestPrintingMethods.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?

---

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