RFR(XS): 8033909: Objects.requireNonNull(T, Supplier) doesn't specify what happens if the passed supplier is null itself
volker.simonis at gmail.com
Tue Feb 11 09:03:27 UTC 2014
as described in the bug comments, I still think that correctly
handling a null Supplier is the cleanest and easiest solution. Without
the change our VM will throw the following NPE:
java.lang.NullPointerException: while trying to invoke the method
java.util.function.Supplier.get() of a null object loaded from the
second parameter of the method
which is more descriptive and perfectly legal, but it may be not what
the user expects (i.e. a NPE with a null message).
Anyhow, I don't insist on my change. If everybody agrees that it would
be better to change the documentation then please go ahead. But keep
in mind that Joe also had some objections against the documentation
changes in the bug comments.
On Tue, Feb 11, 2014 at 12:24 AM, Mike Duigou <mike.duigou at oracle.com> wrote:
> I would prefer to leave the implementation as is and amend the documentation. The difference in behaviour between the overloads seems OK since it will still be valid for the Supplier to return null. The String overload reasonably allows null since the Throwable(String) constructor allows null message. (See Throwable.getMessage()).
> It seems like ignoring/masking the likely error of passing a null Supplier isn't really being helpful.
> On Feb 10 2014, at 05:30 , Volker Simonis <volker.simonis at gmail.com> wrote:
>> could you please review the following tiny change which fixes a
>> problem in Objects.requireNonNull:
>> The problem is that Objects.requireNonNull(T, Supplier) does not check
>> for the Supplier argument being null. Instead it relies on the fact,
>> that the VM will implicitly throw a NullPointerException while trying
>> to call .get on the Supplier argument during the creation of the
>> explicit NullPointerException which is supposed to be thrown by
>> Objects.requireNonNull(T, Supplier) for a null T argument.
>> This makes the behavior of Objects.requireNonNull(T, Supplier)
>> slightly different from the one of the overladed
>> Objects.requireNonNull(T, String) version. The latter one always
>> throws a NPE with a null message in the case where the String argument
>> is null. For the first one, it depends on the implementation of the VM
>> whether the trown NPE will have a null message or not (i.e. the VM is
>> free to create a NPE with an arbitrary message).
>> The bug report mentions that this behavior makes it hard to develop a
>> conformance test for the scenario and suggests to tweak the JavaDoc of
>> Objects.requireNonNull(T, Supplier) such that it allows NPE with an
>> unspecified message.
>> However, I think the easiest fix is to just check for the supplier
>> object beeing null and explicitely creating a NPE with a null message
>> in that case. This will align the behavior of
>> Objects.requireNonNull(T, Supplier) with that of
>> Objects.requireNonNull(T, String). Also notice that the extra
>> null-check is only performed if the object argument T of
>> Objects.requireNonNull(T, Supplier) is null, which should be the
>> unusual case anyway. I therefore don't expect this change to have any
>> negative performance impact.
>> This webrev is for jdk9, but I think it should be also downported to jdk8.
>> Thank you and best regards,
More information about the core-libs-dev