Fix soundness #521
Fix soundness #521
Conversation
The specializations were all unsound since they typically returned WithFilter of
a collection {Iterable/Map/Set} where a mutable or immutable collection was required.
We need to either drop them or come up with a sound design.
There were three methods specialized in ArrayDeque to return some structure of ArrayDeques instead of the underlying collection type. These were inherited without overridng by Queue and Stack. This means the contract is broken: The type says that e.g. sliding returns an Iterator[Queue] but in reality it returns an Iterator[ArrayDeque].
|
I'd like to point out that those For example, see the old issue scala-js/scala-js#843 and its fix https://github.com/scala-js/scala-js/pull/851/files. |
|
@sjrd Yes, that's my sentiment as well. It needs to be documented thoroughly. |
allanrenucci
force-pushed
the
fix-soundness
branch
from
5d88bf6 to
57e2094
Compare
.travis.yml
Outdated
| script: | ||
| - sbt ++2.12.4 memoryBenchmark/compile test ++2.13.0-M2 timeBenchmark/compile test ++0.7.0-RC1 test | ||
| - sbt ++2.12.4 memoryBenchmark/compile test ++2.13.0-M2 timeBenchmark/compile test | ||
| - sbt ++0.8.0-bin-20180323-5be1360-NIGHTLY test junit/test:compile |
There was a problem hiding this comment.
The collections-contrib still fail to compile under Dotty due to an issue with WithFilter. Running the tests in the junit project fails with Dotty. I'll be looking into it
There was a problem hiding this comment.
@julienrf I you managed to fix the collections-contrib with Dotty, you can add them to the CI tests
There was a problem hiding this comment.
There is still a compiler crash when compiling MapDecoratorTest.scala, but at least collections-contribJVM/compile works with Dotty.
There was a problem hiding this comment.
Ok thanks! I'll look into the crash
It is still built on top of the unsound `protected[this] type IterableCC` thing, though.
|
I’ve just pushed a commit to get |
|
I’ve also fixed |
|
Some |
julienrf
added
the
port to scala/scala
Insert explicit @uncheckedVariance annotations even for private[this] or protected[this] members. This commit is a port of scala/collection-strawman#521
|
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 |
Insert explicit @uncheckedVariance annotations even for private[this] or protected[this] members. This commit is a port of scala/collection-strawman#521
Subset of scala#521 containing only changes applied to the collections-contrib module. The other changes have already been ported to scala/scala#6508.
|
Superseded by #561 and scala/scala#6508 |
Subset of scala/collection-strawman#521 containing only changes applied to the collections-contrib module. The other changes have already been ported to scala/scala#6508.
…riance` annotations. This commit is a port of scala/collection-strawman#521 Let’s assume that our code is unsound by not relying on the `protected[this]` escape hatch that disables variance checking. Instead, properly insert `@uncheckedVariance` annotations to see where we are doing bad things. On a positive note, this also encourages us to better document the restrictions of such unsound members. Add tests showing that `reverse` behaves correctly on ArrayDeque subclasses. Fixes scala/collection-strawman#505 Fixes scala/collection-strawman#547
…riance` annotations. This commit is a port of scala/collection-strawman#521 Let’s assume that our code is unsound by not relying on the `protected[this]` escape hatch that disables variance checking. Instead, properly insert `@uncheckedVariance` annotations to see where we are doing bad things. On a positive note, this also encourages us to better document the restrictions of such unsound members. Add tests showing that `reverse` behaves correctly on ArrayDeque subclasses. Fixes scala/collection-strawman#505 Fixes scala/collection-strawman#547
4 participants
No description provided.