RFR: 8180352: Add Stream.toList() method

[Remi Forax]

----- Mail original -----
> De: "Stuart Marks" <smarks at openjdk.java.net>
> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mardi 3 Novembre 2020 04:18:08
> Objet: RFR: 8180352: Add Stream.toList() method

> This change introduces a new terminal operation on Stream. This looks like a
> convenience method for Stream.collect(Collectors.toList()) or
> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
> method directly on Stream enables it to do what can't easily by done by a
> Collector. In particular, it allows the stream to deposit results directly into
> a destination array (even in parallel) and have this array be wrapped in an
> unmodifiable List without copying.
> 
> In the past we've kept most things from the Collections Framework as
> implementations of Collector, not directly on Stream, whereas only fundamental
> things (like toArray) appear directly on Stream. This is true of most
> Collections, but it does seem that List is special. It can be a thin wrapper
> around an array; it can handle generics better than arrays; and unlike an
> array, it can be made unmodifiable (shallowly immutable); and it can be
> value-based. See John Rose's comments in the bug report:
> 
> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
> 
> This operation is null-tolerant, which matches the rest of Streams. This isn't
> specified, though; a general statement about null handling in Streams is
> probably warranted at some point.
> 
> Finally, this method is indeed quite convenient (if the caller can deal with
> what this operation returns), as collecting into a List is the most common
> stream terminal operation.

Hi Stuart,
I'm Okay with the idea of having a method toList() on Stream but really dislike the proposed semantics because tit is neither stream.collect(toList()) nor stream.collect(toUnmodifiableList()) but something in between.

It's true that a Stream support nulls, we want people to be able map() with a method that returns null and then filter out the nulls (even if using flatMap for this case is usually a better idea),
but it doesn't mean that all methods of the Stream interface has to support nulls, the original idea was more to allow nulls to flow in the stream because at some point they will be removed before being stored in a collection.

For me allowing in 2020 to store null in a collection is very backward, all collections since 1.6 doesn't support nulls.

Also, adding a third immutable list creates a problem, it means that now when we get an immutable list it can be 3 different implementations but the VM only uses bimorphic inlining cache,
so more callsites will fail to inline because of that. I think we have already reduced the number of implementation of immutable map from 3 to 2 for the very same reasons.

I believe that instead of inventing a third semantics that allows to store null in a collection, we should use the semantics of stream.collect(Collectors.toUnmodifiableList()) even if it means that toList() will throw a NPE if one element is null.

Rémi