RFR 8210527: NullPointerException in jdk.jshell.Eval.translateExceptionStack

[Aaron Scott-Boddendijk]

I'm curious about the policies within the JDK and whether there's a reason
not to follow a policy we practice.

In the supplied web-rev, the bug is identified as 'r.getFileName()'
resolving to null and then attempting to dereference null to invoke equals
against a literal argument.

That is attempting to evaluate:

    r.getFileName().equals("<none>")

The solution presented has been to explicitly add a null check:

    r.getFileName() != null && r.getFileName().equals("<none>")

In this scenario our practices would result in inverting the subject and
target of the equals call:

    "<none>".equals(r.getFileName())

Here as the literal is never null and the contract for equals specifies the
result is false if the target is null, the solution evaluates the getter
only once and is overall simpler.

Is there a reason not to do this? It seems to be a common recommendation of
static analysis tools. The only reason that I can think of, is to more
clearly indicate that r.getFileName() _could_ be null.

Could this result in less favourable optimisation?

--
Aaron Scott-Boddendijk


On Fri, Sep 6, 2019 at 9:38 AM Robert Field <robert.field at oracle.com> wrote:

> I'm still looking for a review.
>
> Thanks,
> Robert
>
>
> On 8/26/19 6:09 PM, Robert Field wrote:
> > Bug:
> >
> > https://bugs.openjdk.java.net/browse/JDK-8210527
> >
> > Webrev:
> >
> > http://cr.openjdk.java.net/~rfield/8210527/
> >
> >
> > Thanks,
> > Robert
> >
> >
>