On Mon, Feb 13, 2017 at 9:27 AM, David Holmes <david.hol...@oracle.com> wrote: > FWIW I think a null Supplier should simply be ignored. >
What do you mean by ignored? Do you mean "ignored" in the sense of my proposed changes (i.e. throw a NPE with a a null message) or do you mean "ignored" in the sense that it is fine to generate a new, implicit NPE when accessing the null supplier (with the consequences detailed in the bug description')? > David > > > On 13/02/2017 6:04 PM, Volker Simonis wrote: >> >> Ping... >> >> On Thu, Feb 9, 2017 at 5:32 PM, Jeff Dinkins <jeff.dink...@oracle.com> >> wrote: >>> >>> >>> Adding Mike’s current email. >>> >>> -jeff >>> >>>> On Feb 9, 2017, at 10:18 AM, Volker Simonis <volker.simo...@gmail.com> >>>> wrote: >>>> >>>> Hi, >>>> >>>> I want to finally resolve this long standing issue (or close it as >>>> "will not fix" if that's not possible): >>>> >>>> http://cr.openjdk.java.net/~simonis/webrevs/2017/8033909/ >>>> >>>> This change has already been discussed in length on the mailing list: >>>> >>>> >>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/thread.html#24989 >>>> >>>> and in the bug comments: >>>> >>>> https://bugs.openjdk.java.net/browse/JDK-8033909 >>>> >>>> So I'll give just a short summary here: >>>> >>>> - 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 when it calls .get on the >>>> Supplier argument during the creation of the explicit >>>> NullPointerException which it is supposed to throw. >>>> >>>> - this behavior slightly differs from Objects.requireNonNull(T, >>>> String) which simply creates a NullPointerException with a null >>>> message in the case where the String argument is null. >>>> >>>> - the difference is not evident in the OpenJDK, because the HotSpot VM >>>> creates a NPE with a null message by default if we call a method on a >>>> null object. >>>> >>>> - however creating such a NPE with a null message when invoking a >>>> method on a null object is not enforced by the standard, and other >>>> implementations can do better :) For the following trivial program: >>>> >>>> public class NonNull { >>>> public static void main(String[] args) { >>>> Supplier<String> ss = null; >>>> Object o = Objects.requireNonNull(null, ss); >>>> } >>>> } >>>> >>>> the current OpenJDK implementation returns: >>>> >>>> Exception in thread "main" java.lang.NullPointerException >>>> at java.util.Objects.requireNonNull(Objects.java:290) >>>> at NonNull.main(NonNull.java:8) >>>> >>>> but the SAP JVM will print: >>>> >>>> Exception in thread "main" java.lang.NullPointerException: while >>>> trying to invoke the method java.util.function.Supplier.get() of a >>>> null object loaded from local variable 'messageSupplier' >>>> at java.util.Objects.requireNonNull(Objects.java:290) >>>> at NonNull.main(NonNull.java:8) >>>> >>>> which is at least confusing for a call to Objects.requireNonNul() with >>>> a null Supplier. We think the exception should be the same like the >>>> one we get when invoking Objects.requireNonNul() with a null String. >>>> >>>> - because of this difference (and because we like our extended >>>> Exception messages :) the JCK test for Objects.requireNonNul(T, >>>> Supplier) (i.e. >>>> api/java_util/Objects/index.html#RequireNonNullMessageSupplier) was >>>> removed from TCK 8 and is still commented out in TCK 9 >>>> (npe_checkingNullSupplier() in RequireNonNullMessageSupplier.java). >>>> >>>> I really think that the simplest and most natural fix for this problem >>>> is to simply check for a null Supplier argument and create the NPE >>>> with an explicit null message in that case (as outlined in the >>>> webrev). This: >>>> - makes the two requireNonNul() overloads for String and Supplier >>>> behave the same (which I think was the initial intention). >>>> - doesn't require documentation changes >>>> - makes it possible to write a simple and conforming TCK test >>>> >>>> If we can't agree on this quickly, I suggest to close the issue as >>>> "will-not-fix" and leave it untested by the TCK. >>>> >>>> Thank you and best regards, >>>> Volker >>>> >>>> PS: the 'XS' obviously only applies to the fix, not to this message :) >>> >>> >
