search

scala / collection-strawman

Implementation of the new Scala 2.13 Collections
Apache License 2.0
200 stars 72 forks source link

Fix soundness #521

Closed allanrenucci closed 6 years ago

sjrd commented 6 years ago

I'd like to point out that those protected[this] members are (likely) unsound. AFAICT, they're very similar to thisCollection and toCollection in the old collections, which do cause ClassCastExceptions if they're not systematically overridden by subclasses.

For example, see the old issue https://github.com/scala-js/scala-js/issues/843 and its fix https://github.com/scala-js/scala-js/pull/851/files.

odersky commented 6 years ago

@sjrd Yes, that's my sentiment as well. It needs to be documented thoroughly.

julienrf commented 6 years ago

I’ve just pushed a commit to get withFilter working. It is implemented on top of the unsound IterableCC-like types, but I think if we want to get it right we’d really have to have one WithFilter implementation per subclass of Iterable.

julienrf commented 6 years ago

I’ve also fixed ArrayDeque so that Stack and Queue can benefit from its overrides.

julienrf commented 6 years ago

Some junit/tests are still failing with Dotty. They seem to be related with serialization, synchronization and garbage collection.

julienrf commented 6 years ago

This PR has been ported to scala/scala but I leave it open here because we will eventually have to also port the part that touches the collections-contrib project.

https://github.com/scala/scala/pull/6508

julienrf commented 6 years ago

Superseded by #561 and scala/scala#6508