I agree with Stuart and Brian that streams should be null-tolerant, including in this case. And of course I’m delighted to see an immutable container as the output to the utility method; I’m a fan of reduced-copy, race-free, bug-resistant algorithms you get from immutability.
On Nov 3, 2020, at 6:53 AM, Remi Forax <fo...@univ-mlv.fr> wrote: > > 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. Yes, this part concerns me, with my JIT hat on. It seems to me that the problem can be removed simply by allowing the existing ListN object to contain nulls. That’s not the same thing as allowing List.of(1,2,3, null): That factory method can reject nulls, while the privileged factory method that makes an array-backed ListN can simply sidestep the null check. And if a call to Stream::toList call returns 0, 1, or 2 items, it’s a straightforward matter to test if either is null, and then choose to use either ListN or List12. No new implementation classes, at all!
