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