Skip to content

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Oct 29, 2021

In Scala 2.13, collection.to(Target) conversions return the collection object unchanged if it is immutable and is already a subtype of the Target type.

This PR brings the same behavior to Scala 2.12 when using scala-collection-compat.

The 2.12 codebase is unchanged, so a to[Target] conversion may still create unnecessary copies.

Remaining differences between 2.12 and 2.13:

  • static types: 2.12's to is defined as def to[Col[_]](implicit cbf: CanBuildFrom[Nothing, A, Col[A]]): Col[A], so to(Map) has static type Iterable[(K, V)], not Map[K, V]

  • lazy maps: mapValues returns a lazy map that's a subtype of immutable.Map, so

    scala> val mv = Map(1 -> 1).mapValues(_ + 1) scala> mv.to(Map) eq mv res7: Boolean = true // false in 2.13, a strict copy is created 
@lrytz lrytz force-pushed the toConserve branch 2 times, most recently from 0af7d33 to 0b2fc40 Compare October 29, 2021 12:40
Copy link
Member Author

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review by @retronym

sharedSourceDir / "scala-2.11_2.12"
}
},
Test / unmanagedSourceDirectories += {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy paste from above, not sure how to factor things out in sbt

@lrytz
Copy link
Member Author

lrytz commented Oct 29, 2021

cc @martijnhoekstra, since you originally wrote IdentityPreservingBuilder for #238. This PR applies it more widely.

@SethTisue
Copy link
Member

this might interest @scala/collections

Copy link
Contributor

@NthPortal NthPortal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial review is that it looks correct. if I get more energy, hopefully I can do a more thorough review.

I did not review of the sbt changes, as I'm less confident in my ability to refactor sbt configs than in yours

@lrytz
Copy link
Member Author

lrytz commented Nov 1, 2021

Thank you @NthPortal!

@lrytz lrytz merged commit d55a12f into scala:main Nov 8, 2021
@SethTisue
Copy link
Member

@lrytz can you fill in the PR description?

@SethTisue SethTisue changed the title preserve immutable collections in to(Target) conversions preserve immutable collections in to(Target) conversions Nov 9, 2021
@lrytz
Copy link
Member Author

lrytz commented Nov 9, 2021

👍 done

@NthPortal
Copy link
Contributor

oh, this is already released, awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants