Hi Mandy,
> and also the cases when it requires to look at its caller
> frame.
>
> Never. Only the method that was executed when the exception is
> thrown is needed. I only mention the backtrace because it happens to
> store the top method and the corresponding bytecode position in the
> top method.
>
>
> That explains why I don't see an example of:
> getNull().m();
>
> public void m() {};
>
> static NPE getNull() {
> return null;
> }
>
> For a compiler expert or one who study the data flow analysis paper
Yes, I think compiler people should understand this.
> , is this example very clear to them that it won't be supported?
The example for your code is supported, see[1], examples 11-14. (I should
enumerate them)
For your code, it will print
"The return value of 'MyClass.getNULL()LNPE;' is null. Can not invoke method
'MyOtherClass.m()V'".
getNull().m() are not nested calls, but subsequent ones. Therefore you will not
find
them on the call stack.
The byte code index of the invoke bytecode invoking m() is known from the
backtrace or
StackWalker/StackFrame. Looking at the bytecode the part "Can not invoke
method 'MyOtherClass.m()V"
is printed.
With the result of the data flow analysis it is possible to find the bytecode
that produced
the reference on which m() is invoked. This is another invoke bytecode in the
method.
Looking at this other bytecode, " The return value of 'MyClass.getNULL()LNPE;'
is null."
is printed.
You probably meant that for
n().getNull().m()
it is not printed that getNull() was called on the result of n()?
This is a design decision not to make the printout too complex.
> This is related to my comment w.r.t. the data flow analysis comes from. I was
> looking for some level of information be spelled out in the JEP that can
> sufficiently give a non-compiler expert an idea but of course no point to
> replicate the paper here.
"The simulated stack" ... contains ..."information about which bytecode pushed
the value to the stack."
That's all.
> It's good to know that this proposal needs the top frame only. Please make
> it
> clear in the JEP.
I tried to point out more precisely why and when I need the top frame. I
pushed this down a bit in the text in "basic algorithm".
> The JEP can take out `StackWalker` but instead talks about
> `StackFrame` since doing it in Java means that it only needs to get back a
> `StackFrame`
> instance representing the top frame stored in `Throwable::backtrace`
Yes. I tried to make the text more precise.
> This will avoid the confusion on calling StackWalker in the NPE constructor
> and
> constructing many StackFrame and many ResolvedMethodName. That was
> part of the code review discussion.
Yes, only the top StackFrame is needed.
> "Computing either part of these messages can fail due to
> insufficient
> information."
> What are the cases that it fails to obtain the information? It'd
> be useful
> to list some examples if not all.
>
> (a ? b : c).d
> You can not know whether the ?-expression yielded b or c, thus
> you don't know why the access .d failed. There is a corresponding
> example
> in [1].
>
>
> Please describe in the JEP the known cases that this proposal doesn't cover.
> Conditional expression is one and
The algorithm looks at a lot of bytecodes to print the method. I can not
enumerate all combinations of bytecodes, it's millions. I will add a
per-bytecode table to
the "Message content" section describing what is printed for which bytecode.
>null receiver if it's the return value from a caller frame (I think this would
>help the readers).
... this is supported, see above.
> The "Different bytecode variants of a method" section
>
> This section raises the case when a method representing an
> execution stack
> frame throwing NPE becomes obsolete, it does not have the
> information
> to compute the message lazily. So this falls into the
> "insufficient
> information" category and no improved NPE message can be
> generated.
>
> Yes, note this also only addresses the method in which the exception
> was raised.
>
> This should also go to the list of known cases that this proposal does not
> cover.
This paragraph lists a possible problem -- multiple variants of methods
with the same name and class name -- and how this is solved.
I added a section "Cases not covered by this JEP" and mention it there, too.
> Since the JEP quotes that NullPointerExceptions are thrown frequently
> and swallowed, it would help if you can generate some numbers.
> This JEP will help the discussion on the proposed feature and design and avoid
> any
> confusion/assumption of the implementation.
I'll generate some numbers. Is running jvm2008 fine?
> I think this is related to this JEP. The question would be:
> - should it include all hidden frames?
> - should it include the top frame if it's a hidden frame since this feature
> only
> requires the top frame?
>
> This JEP requires the hidden frame to be recorded in the backtrace. This has
> impact to the printing of stack trace that needs to know about hidden frames
> and decide if it should be filtered or included when printing.
No. I propose to remember that the frame was hidden. I do not propose to
remember the hidden frame.
I now mention in the JEP that the message is not printed for hidden frames in
the
new "cases not covered" section.
> I made a webrev of its own for this problem, so that the code
> can be looked at isolated of other, orthogonal issues.
> http://cr.openjdk.java.net/~goetz/wr19/8221077-hidden_to_frame/
> I don't need the webrev as we are discussing the proposed feature and design.
Well, sometimes code is more precise than language ...
> The "Message persistance" section
>
> I read this section like an implementation details
>
> Yes, I had the feeling that I was talking too much about
> implementation details,
> but mostly in the "Basic algorithm" section.
> This section discusses characteristics of NPE that differ from other
> exceptions
> and that were proposed in the course of the review of my
> original webrev. They are visible to the Java implementor.
> So I think this is just the basic information needed in a JEP.
>
>
> This is a good start that allows us to start the discussion. This JEP will
> probably
> need several iterations of improvement. I'm writing a JEP that I have
> iterated
> many times already and it makes the JEP and proposed feature better. Just to
> share with you that it's part of the process.
>
>
>
>
> I think this section
> intends to call out that the message of NPE if constructed by
> user code
> i.e. via public constructor, should not be altered including no-
> arg
> constructor or pass a null message to 1-arg constructor. The
> enhanced
> NPE message is proposed only when NPE is thrown due to null
> dereference
> when executing bytecode.
>
> Yes.
>
>
> I don't understand what you tried to say about
> "npe.getMessage() ==
> npe.getMessage()".
> Throwable::getMessage does not guarantee to return same
> String object for
> each invocation.
>
> It does not guarantee so, but it's the case for most exceptions.
> Usually,
> one calls super(msg) in an Exception constructor.
> It is a design decision to implement it that way, my original proposal
> was different. I'm fine with this design, but it should be mentioned I
> think.
>
>
> I think the JEP should be made clearer what you intend to say.
>
> When deserializing the class bytes may be of a different
> version, so
> StackTraceElements
> are serialized with its string form. Serializing NPE with a
> computed message
> seems to be the only viable solution.
>
> I think implementing writeReplace() is a good idea, too. But
> Roger does not like it: http://mail.openjdk.java.net/pipermail/core-
> libs-dev/2019-February/058513.html
>
>
>
> The JEP should record this either if this is an open issue or makes it a clear
> proposal to compute the message when NPE is serialized and what the client
> would do (that may be running in a different version)
Ah, OK, I understand.
Here, I should simply state that persistence is not intended.
In the Alternatives section, I can then point out how to implement
persistence?
I changed the text accordingly.
> IAE quotes the names, which are user defined strings. It does not quote
> class names, method names etc.
> Here, I was asked to quote these, which is inconsistent with IAE. But
> there are other
> examples where method names are quoted ... So there is no clear line
> followed
> yet. In the end, I don't care, but want advice how to do it. (I would
> not
> quote ...).
>
> You can add it to the open issues section.
I'll just state they are quoted. This was the opinion of basically everybody
who commented on this. The text currently more reads like I want to
reflect the past discussions in the JEP, better be clear here...
> "Alternatives" section
>
> "The current proposal is to implement this in the Java runtime
> in C++ accessing
> directly the available datastructures in the metaspace."
>
> "Also, ASM and StackWalker do not support the full
> functionality needed.
> StackWalker must be called in the NullPointerException
> constructor."
>
>
>
> This is not true as you wrote in the subsequent sentence. If we
> choose
> the implementation to Java, StackWalker will need to be
> extended to
> read Throwable's backtrace.
>
> The senctence above describes the status quo. The current
> implementation
> of StackWalker must be called in the constructor.
>
>
> I just called out the confusion this section could cause. I think it should
> be
> updated or rephase.
I tried to rephrase it.
> Please refer to the JEP template (http://openjdk.java.net/jeps/2) about the
> Testing section.
>
> // What kinds of test development and execution will be required in order
> // to validate this enhancement, beyond the usual mandatory unit tests?
> // Be sure to list any special platform or hardware requirements.
I tried to improve it.
> To be clear:
>
> What I was suggesting is to describe the scenarios this proposed feature will
> improve the message
> but *not* to cut-n-paste the messages from [1]. So this is not tied to what
> the
> message will be
> printed from the implementation.
>
> Writing the scenario in English can help discussing the NPE messages should be
> implemented
> that will be easy for Joe developer to understand.
NPEs are a very basic thing in Java. They can happen in any Java code
running in any application, toy program ... etc. I think the message should
help in all cases. It's more to the person implementing the call to
getMessage() that
knows the scenario. I really don't know what to write here.
Thanks a lot for your detailed comments, I think it already
improved the text considerably!
I'll run the experiment to count the NPEs and write the
"message content" section about what is printed and what not.
Best regards,
Goetz.
>
> Mandy
>
>
>
> But I would
> like to do that once it is agreed on the messages, so I don't need to
> edit the JEP all the time.
> The file referenced is simply generated by the test, so it's easy to
> update
> and sure to reflect the implementation.
>
>
> This JEP includes a few links to the C++ functions in the
> current
> webrev. I
> appreciate that and I assume they will be taken out at some
> point.
>
> Yes, sure, I can remove that.
>
> Best regards,
> Goetz.
>
>
