RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/16/src/hotspot/share/interpreter/bytecodeUtils.cpp.html

  215 /*
  216  * Prints the name of the method that is described at constant pool
  217  * index cp_index in the constant pool of method 'method'.
  218  */


Can you change the comment blocks to // comments?

1377     // Print string describing which action (bytecode) could not be
1378     // performed because of the null reference.
1379     emb.print_NPE_failed_action(ss, bci);
1380     // Print a description of what is null.
1381     if (!emb.print_NPE_cause(ss, bci, slot)) {
1382       // Nothing was printed. End the sentence without the 'because'
1383       // subordinate sentence.
1384       ss->print(".");
1385     }


I think you should have a ResourceMark here.

1099   assert(bci >= 0, "BCI too low");
1100   assert(bci < get_size(), "BCI too large");
1101


I think sanity checking the bci should be in the beginning of the 
ExceptionMessageBuilder constructor, or even in the main function before 
the constructor call.  This seems too late; after it's done all the work.

http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/16/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NullPointerExceptionTest.java.html

I think we're supposed to put @bug 8218628 in the test (even if it's an 
RFE).

http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/16/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NPEInHiddenTopFrameTest.java.html

   31  * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:-ShowHiddenFrames -XX:-ShowCodeDetailsInExceptionMessages NPEInHiddenTopFrameTest hidden

Shouldn't this be -XX:+ShowCodeDetailsInExceptionMessages to show that 
the hidden frame isn't printed with the new code?

I've done a few passes of reviewing this code.  These were the only new 
things that I noticed.  Thank you for making the names more descriptive 
in BytecodeUtils.   This looks good to me!   Hope that people will love 
this feature!

Thank you Goetz for all your hard work!

Coleen


On 9/10/19 10:44 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> the subject should mention 8218628. (Not 8218626).
> Sorry for this!
>
> Best regards,
>    Goetz.
>
> From: Lindenmaier, Goetz
> Sent: Dienstag, 10. September 2019 11:48
> To: 'Hotspot dev runtime' <hotspot-runtime-dev at openjdk.java.net>; Java Core Libs <core-libs-dev at openjdk.java.net>
> Subject: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.
>
> Hi,
>
> this is the implementation of JEP 358: Helpful NullPointerExceptions.
>
> The JEP is in status "Candidate". Coleen (many, many thanks!) ran
> it through the Oracle-internal processes.  Now I please need final reviews
> for this change so that I can propose it to target jdk 14.
>
> JEP: https://bugs.openjdk.java.net/browse/JDK-8220715
> Enhancement: https://bugs.openjdk.java.net/browse/JDK-8218628
> webrev: http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/16/
>
> The change ran through a lot of testing, all jtreg and jck tests to name
> only some. The webrev points to patch
> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/16/enable_NPE_message.patch
> that enables the change by default,  which was useful for testing to
> assure the code is used in the tests.
> I just pushed the change to jdk/submit once more.
>
> Please review.
>
> Thanks!
>    Goetz.