RFR(L): 8218628: Add detailed message to NullPointerException describing what is null.

[Lindenmaier, Goetz]

Hi,

ok, so here a new webrev just adding a setter for the field:
http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/02

... and incorporating the other comments.

Best regards,
  Goetz.





> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Freitag, 8. Februar 2019 13:31
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> dev at openjdk.java.net; Java Core Libs <core-libs-dev at openjdk.java.net>
> Subject: Re: RFR(L): 8218628: Add detailed message to NullPointerException
> describing what is null.
> 
> Hi Goetz,
> 
> Just one follow up for now:
> 
> >   * Add package visible "void setMessage (String msg)" to Throwable.
> 
> Yes, just use package accessibility to deal with this, no need to jump
> through hoops (or the VM :) ).
> 
> Thanks,
> David
> 
> On 8/02/2019 9:51 pm, Lindenmaier, Goetz wrote:
> > Hi David,
> >
> >> Hi Volker,
> > ... I assume Volker could have contributed this as well, but actually
> > I must mention Ralf Schmelter as the original author of this :)
> >
> >> You know I'm not going to be a big fan of this :), but as long as we
> >> don't pay for it if we don't want it, then that's okay. (I'm still
> >> trying to gauge that)
> >>
> >> I have a little test for this that I ran through your patch:
> >>
> >> public class NPE {
> >>     static class B {
> >>       C b() { return null; }
> >>     }
> >>     static class C {
> >>       int c(Object o, String s) {  return 0; }
> >>     }
> >>
> >>     public static void main(String[] args) {
> >>       int x  = a().b().c(d(), e().toString());
> >>     }
> >>
> >>     static B a() { return null; }
> >>     static Object d() { return null; }
> >>     static Object e() { return null; }
> >>
> >> }
> >>
> >> and the result was a bit confusing for me:
> >>
> >> java.lang.NullPointerException: while trying to invoke the method
> >> NPE$B.b(()LNPE$C;) of a null object returned from NPE.a(()LNPE$B;)
> >>
> >> The placement and format of the return type descriptors obfuscates
> >> things to me - especially the Lxxx; format. Can we make that more Java
> >> programmer friendly eg:
> >>
> >> "while trying to invoke the method 'NPE$C NPE$B.b()' ..."
> >>
> >> though I think trying to produce signatures within the message is going
> >> to be very awkward in the general case. The key part is the "method
> >> NPE.b() ... returned from NPE.a()"
> > Actually, I have left out code that changes the signatures to the
> > Java source code wording. I already left that out in my former
> > exception message contributions.  For example see the messages in
> > jtreg/runtime/exceptionMsgs/IllegalAccessError/IllegalAccessErrorTest.java,
> > they have the same bad format:
> > "class test.Runner4 tried to access private method
> test.IllegalAccessErrorTest.iae4_m()V"
> >
> > I would like to fix them altogether in a follow-up, is that acceptable to
> > you?
> >
> >> Also "of a null object" would read better as "on a null reference".
> > Makes sense, fixed.
> >
> > But I'm not that sure about changing these:
> > "while trying to read the field '%s' of a null object"
> > --> "while trying to read the field '%s' from a null reference"
> > "while trying to write the field %s of a null object"
> > --> "while trying to write the field %s  of a null reference"
> >
> >> First you will need to file a CSR request for the new product flags.
> > I'm not sure whether I need the product flags altogether. I would
> > prefer removing them.
> >
> >> Second, I don't understand why you need to call into the VM with
> >> JVM_SetDefaultMessage, to set a field in the Java object? Why isn't that
> >> done in Java?
> > Obviously, the problem is that the field is private.
> > As Christoph points out, there are several ways to implement this.
> > Please give advice:
> >    * reflection
> >    * shared secret
> >    * Add package visible "void setMessage (String msg)" to Throwable.
> >
> > Best regards,
> >    Goetz
> >
> >
> >>
> >> Thanks,
> >> David
> >>
> >> On 8/02/2019 2:43 am, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> since Java 5, our internal VM reports verbose null pointer exception
> >>> messages. I would like to contribute this feature to OpenJDK.
> >>>
> >>> With this change, messages as
> >>>      "java.lang.NullPointerException: while trying to load from a null int
> array
> >> loaded from local variable 'ia1'"
> >>> are printed.  For more examples see the JBS bug or the test included.
> >>> https://bugs.openjdk.java.net/browse/JDK-8218628
> >>>
> >>> The messages are generated by parsing the bytecodes. For not to have any
> >> overhead when the
> >>> NPE is allocated, the message is only generated when it is accessed by
> >> getMessage() or
> >>> serialization. For this I added a field to NPE to indicate that the message
> still
> >> needs to be
> >>> computed lazily.
> >>>
> >>> Please review:
> >>> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/01/
> >>> I'm happy to incorporate your comments.
> >>>
> >>> Best regards,
> >>>     Goetz
> >>>
> >>>