On Tue, 24 Jan 2023 18:00:30 GMT, Stuart Marks <[email protected]> wrote:

>> src/java.base/share/classes/java/util/ImmutableCollections.java line 174:
>> 
>>> 172:             return List.of();
>>> 173:         } else {
>>> 174:             return (List<E>)List.of(coll.toArray()); // implicit 
>>> nullcheck of coll
>> 
>> The comment is no longer relevant here, as it now happens on line 171.
>
> Thanks @szegedi for catching this and @viktorklang-ora for fixing it. I like 
> having comments like this in cases where we need to throw NPE for null and 
> for which there's no explicit `Objects.requireNonNull`. We've had cases in 
> the past where an apparently innocuous refactoring postponed an implicit 
> nullcheck, which opened the possibility of a side effect occuring before NPE 
> was thrown (violates failure idempotency). So I think maintaining such 
> comments is important.
> 
> With that in mind, for the Set and Map cases, could you (Viktor) add similar 
> comments there? Arguably they should have been there already, but, oh well, 
> they weren't. Thanks.

@stuart-marks Makes sense. I've added those comments to Set and Map copyOf()

-------------

PR: https://git.openjdk.org/jdk/pull/11847

Reply via email to