On Tue, 3 Nov 2020 18:53:23 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> Should there be a test that tests the default implementation in
>> `j.u.s.Stream`? Or maybe there is and I missed that?
>
> @dfuch wrote:
>> Should there be a test that tests the default implementation in
>> `j.u.s.Stream`? Or maybe there is and I missed that?
>
> The test method `testDefaultOps` does that. The stream test framework has a
> thing called `DefaultMethodStreams` that delegates everything except default
> methods to another Stream instance, so this should test the default
> implementation.
OK, there are rather too many forking threads here for me to try to reply to
any particular message, so I'll try to summarize things and say what I intend
to do.
Null tolerance. While part of me wants to prohibit nulls, the fact is that
Streams pass through nulls, and toList() would be much less useful if it were
to reject nulls. The affinity here is closer to Stream.toArray(), which allows
nulls, rather than Collectors.to[Unmodifiable]List.
Unmodifiability. Unlike with nulls, this is where we've been taking a strong
stand recently, where new constructs are unmodifiable ("shallowly immutable").
Consider the Java 9 unmodifiable collections, records, primitive classes, etc.
-- they're all unmodifiable. They're data-race-free and are resistant to a
whole class of bugs that arise from mutability.
Unfortunately, the combination of the above means that the returned List is
neither like an ArrayList nor like an unmodifiable list produced by List.of()
or by Collectors.toUnmodifiableList(). [Heavy sigh.] While I've been somewhat
reluctant to introduce this new thing, the alternatives of trying to reuse one
of the existing things are worse.
John and Rémi pointed out that the way I implemented this, using a subclass,
reintroduces the possibility of problems with megamorphic dispatch which we had
so carefully avoided by reducing the number of implementation classes in this
area. I agree this is a problem.
I also had an off-line discussion with John where we discussed the
serialization format, which unfortunately is somewhat coupled with this issue.
(Fortunately I think it can mostly be decoupled.) Given that we're introducing
a new thing, which is an unmodifiable-list-that-allows-nulls, this needs to be
manifested in the serialized form of these new objects. (This corresponds to
the new tag value of IMM_LIST_NULLS in the CollSer class.) The alternative is
to reuse the existing serialized form, IMM_LIST, for both of these cases,
relaxing it to allow nulls. This would be a serialization immutability problem.
Suppose I had an application that created a data structure that used lists from
List.of(), and I had a global assertion over that structure that it contained
no nulls. Further suppose that I serialized and deserizalized this structure.
I'd want that assertion to be preserved after deserialization. If another app
(or a future version of this app) created the structure using Stream.to
List(), this would allow nulls to leak into that structure and violate that
assertion. Therefore, the result of Stream.toList() should not be
serialization-compatible with List.of() et. al. That's why there's the new
IMM_LIST_NULLS tag in the serial format.
However, the new representation in the serial format does NOT necessarily
require the implementation to be a different class. I'm going to investigate
collapsing ListN and ListNNullsAllowed back into a single class, while
preserving the separate serialized forms. This should mitigate the concerns
about megamorphic dispatch.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1026
