Skip to content

Map.values.map is not strict in Scala 2.13 #11589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ijuma opened this issue Jun 25, 2019 · 2 comments · Fixed by scala/scala#8195
Closed

Map.values.map is not strict in Scala 2.13 #11589

ijuma opened this issue Jun 25, 2019 · 2 comments · Fixed by scala/scala#8195

Comments

@ijuma
Copy link

ijuma commented Jun 25, 2019

I am not sure if this is intentional, but I didn't see any mention of it in the migration notes. mutable.Map.values.map is not strict in 2.13 which can cause side-effects to be executed multiple times when compared to 2.12 and older.

A simple example follows with 2.12.8 and then 2.13.0:

Welcome to Scala 2.12.8 (OpenJDK 64-Bit Server VM, Java 11.0.2).
Type in expressions for evaluation. Or try :help.
scala> scala.collection.mutable.Map(1 -> "1", 2 -> "2")
res0: scala.collection.mutable.Map[Int,String] = Map(2 -> 2, 1 -> 1)

scala> var iterationCounter = 0
iterationCounter: Int = 0

scala> res0.values.map { v => iterationCounter += 1; v }
res1: Iterable[String] = List(2, 1)

scala> res1.toSeq
res2: Seq[String] = List(2, 1)

scala> iterationCounter
res3: Int = 2

scala> res1.toSeq
res4: Seq[String] = List(2, 1)

scala> iterationCounter
res5: Int = 2

scala> scala.collection.mutable.Map(1 -> "1", 2 -> "2")
res0: scala.collection.mutable.Map[Int,String] = HashMap(1 -> 1, 2 -> 2)

Welcome to Scala 2.13.0 (OpenJDK 64-Bit Server VM, Java 11.0.2).
Type in expressions for evaluation. Or try :help.

scala> var iterationCounter = 0
iterationCounter: Int = 0

scala> res0.values.map { v => iterationCounter += 1; v }
res1: Iterable[String] = View()

scala> res1.toSeq
res2: Seq[String] = List(1, 2)

scala> iterationCounter
res3: Int = 2

scala> res1.toSeq
res4: Seq[String] = List(1, 2)

scala> iterationCounter
res5: Int = 4

@ijuma ijuma changed the title Map.values.map is lazy in Scala 2.13 Map.values.map is not strict in Scala 2.13 Jun 25, 2019
@NthPortal
Copy link

is there a reason for Map#values not returning View rather than Iterable?

@julienrf
Copy link

Not that I know. It should return a strict Iterable

@lrytz lrytz added this to the 2.13.1 milestone Jun 26, 2019
@joshlemer joshlemer changed the title Map.values.map is not strict in Scala 2.13 mutable.Map.values.map is not strict in Scala 2.13 Jun 30, 2019
@joshlemer joshlemer changed the title mutable.Map.values.map is not strict in Scala 2.13 Map.values.map is not strict in Scala 2.13 Jun 30, 2019
ijuma added a commit to apache/kafka that referenced this issue Jul 2, 2019
Scala 2.13 support was added to build via #5454. This PR adjusts the code so that
it compiles with 2.11, 2.12 and 2.13.

Changes:
* Add `scala-collection-compat` dependency.
* Import `scala.collection.Seq` in a number of places for consistent behavior between
Scala 2.11, 2.12 and 2.13.
* Remove wildcard imports that were causing the Java classes to have priority over the
Scala ones, related Scala issue: scala/scala#6589.
* Replace parallel collection usage with `Future`. The former is no longer included by
default in the standard library.
* Replace val _: Unit workaround with one that is more concise and works with Scala 2.13
* Replace `filterKeys` with `filter` when we expect a `Map`. `filterKeys` returns a view
that doesn't implement the `Map` trait in Scala 2.13.
* Replace `mapValues` with `map` or add a `toMap` as an additional transformation
when we expect a `Map`. `mapValues` returns a view that doesn't implement the
`Map` trait in Scala 2.13.
* Replace `breakOut` with `iterator` and `to`, `breakOut` was removed in Scala
2.13.
* Replace to() with toMap, toIndexedSeq and toSet
* Replace `mutable.Buffer.--` with `filterNot`.
* ControlException is an abstract class in Scala 2.13.
* Variable arguments can only receive arrays or immutable.Seq in Scala 2.13.
* Use `Factory` instead of `CanBuildFrom` in DecodeJson. `CanBuildFrom` behaves
a bit differently in Scala 2.13 and it's been deprecated. `Factory` has the behavior
we need and it's available via the compat library.
* Fix failing tests due to behavior change in Scala 2.13,
"Map.values.map is not strict in Scala 2.13" (scala/bug#11589).
* Use Java collections instead of Scala ones in StreamResetter (a Java class).
* Adjust CheckpointFile.write to take an `Iterable` instead of `Seq` to avoid
unnecessary collection copies.
* Fix DelayedElectLeader to use a Map instead of Set and avoid `to` call that
doesn't work in Scala 2.13.
* Use unordered map for mapping in SimpleAclAuthorizer, mapping of ordered
maps require an `Ordering` in Scala 2.13 for safety reasons.
* Adapt `ConsumerGroupCommand` to compile with Scala 2.13.
* CoreUtils.min takes an `Iterable` instead of `TraversableOnce`, the latter does
not exist in Scala 2.13.
* Replace `Unit` with `()` in a couple places. Scala 2.13 is stricter when it expects
a value instead of a type.
* Fix bug in CustomQuotaCallbackTest where we did not necessarily set `partitionRatio`
correctly, `forall` can terminate early.
* Add a couple of spotbugs exclusions that are needed by code generated by Scala 2.13
* Remove unused variables, simplify some code and remove procedure syntax in a few
places.
* Remove unused `CoreUtils.JSONEscapeString`.

Reviewers: Manikumar Reddy <[email protected]>, José Armando García Sancio <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants