Port Capture-checked Scala 2 collections #23769
Merged
+2,860
−1,093
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
odersky
requested changes
Aug 19, 2025
library/src/scala/collection/immutable/StrictOptimizedSeqOps.scala
Outdated
Show resolved
Hide resolved
Inline them into (Indexed)SeqOps and caps.Pure
natsukagami
commented
Aug 19, 2025
Comment on lines
+162
to
+163
| // CC note: seems like the compiler cannot figure out that this class <: Iterator[C^{s}], | ||
| // so we need a cast when upcasting is needed. |
There was a problem hiding this comment.
Need an issue for this one
natsukagami
commented
Aug 19, 2025
Comment on lines
+872
to
+878
| // CC Problem: if we add the logically needed `^`s to the `it` parameter and the | ||
| // self type, separation checking fails in the compiler-generated equals$extends | ||
| // method of the value class. There seems to be something wrong how pattern | ||
| // matching interacts with separation checking. A minimized test case | ||
| // is pending/pos-custom-args/captures/SizeCompareOps-redux.scala. | ||
| // Without the `^`s, the `sizeIs` method needs an unsafeAssumePure. | ||
|
|
There was a problem hiding this comment.
TODO make an issue
hamzaremmal
reviewed
Aug 19, 2025
natsukagami
commented
Aug 19, 2025
| @@ -218,22 +237,22 @@ transparent trait MapOps[K, +V, +CC[_, _] <: IterableOps[_, AnyConstr, _], +C] | |||
| * @return an [[Iterable]] collection of the keys contained by this map | |||
| */ | |||
| @deprecatedOverriding("This method should be an alias for keySet", since="2.13.13") | |||
| def keys: Iterable[K] = keySet | |||
| def keys: Iterable[K]^{this} = this.keySet | |||
There was a problem hiding this comment.
Technically this should've been an alias for this.keysIterator, which would be strictly more efficient, but alas
natsukagami
commented
Aug 19, 2025
Comment on lines
+429
to
+436
| private class LazyKeySet[K, +V, +CC[_, _] <: IterableOps[_, AnyConstr, _], +C](mp: MapOps[K, V, CC, C]) extends AbstractSet[K] with DefaultSerializable { | ||
| def iterator: Iterator[K] = mp.keysIterator | ||
| def diff(that: Set[K]): Set[K] = LazyKeySet.this.fromSpecific(this.view.filterNot(that)) | ||
| def contains(key: K): Boolean = mp.contains(key) | ||
| override def size: Int = mp.size | ||
| override def knownSize: Int = mp.knownSize | ||
| override def isEmpty: Boolean = mp.isEmpty | ||
| } |
There was a problem hiding this comment.
Actually is the old implementation of KeySet & GenKeySet
hamzaremmal
reviewed
Aug 19, 2025
odersky
approved these changes
Aug 19, 2025
hamzaremmal
reviewed
Aug 19, 2025
hamzaremmal
reviewed
Aug 19, 2025
library/src/scala/collection/immutable/StrictOptimizedSeqOps.scala
Outdated
Show resolved
Hide resolved
hamzaremmal
requested changes
Aug 19, 2025
odersky
approved these changes
Aug 19, 2025
|
PR for pure checking: #23784 |
hamzaremmal
approved these changes
Aug 19, 2025
|
I'm going to bed now. If the tests are green we can merge. |
bracevac
approved these changes
Aug 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
collection.immutable.LazyListIterable[A], implements Iterable and IterableOps.LazyListandStreamare not capture-checked. Note added toLazyList.New traitno longer neededcollection.StrictSeqOps[A, CC[_] <: Pure, C], which extends SeqOps (which can now be impure) and requires the implementation to be pure.Seqand other data structures extend this.Suspicious changes:
MapOps.keySetchanges implementation to be always strict, unless it is one of the library's known strict collectionsNot capture-checked (yet?)
collection.convert.impla.k.a Steppers, due to some purity problems (maybe because we don't compile steppers and rely on Scala 2, for specialization?).