Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make MapView#values preserve laziness #9090

Merged
merged 1 commit into from Jul 2, 2020
Merged

Make MapView#values preserve laziness #9090

merged 1 commit into from Jul 2, 2020

Conversation

NthPortal
Copy link
Contributor

ChangeMapView#values to return a View instead
of an AbstractIterable, so that further operations on
the returned value remain lazy.

Fixes scala/bug#12059

@NthPortal NthPortal requested a review from julienrf June 27, 2020 22:04
@scala-jenkins scala-jenkins added this to the 2.13.4 milestone Jun 27, 2020
@NthPortal
Copy link
Contributor Author

NthPortal commented Jun 28, 2020 •
edited

community build at https://scala-ci.typesafe.com/job/scala-2.13.x-integrate-community-build/3551/ (outdated) eventually, unless I screwed up the job in which case not

@NthPortal NthPortal added the library:collections PRs involving changes to the standard collection library label Jun 28, 2020
julienrf
julienrf reviewed Jun 28, 2020
View reviewed changes
Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks @NthPortal!

Do we have something to change in SortedMap as well?

@NthPortal
Copy link
Contributor Author

Do we have something to change in SortedMap as well?

no, it seems that SortedMap#view uses the same implementation. however, we may need to add something for keys

@NthPortal
[bug#12059] Make MapView#values and #keys return Views
e15f471 
Change`MapView#values` and `MapView#keys` to return `View`s
instead of `AbstractIterable`s, so that further operations on
the returned values remain lazy.
@NthPortal
Copy link
Contributor Author

honestly, it would be nice if keySet could be lazy too, but it returns a Set, so that's not really possible

dwijnand
dwijnand reviewed Jun 29, 2020
View reviewed changes
build.sbt

// Fix for scala/bug#12059
ProblemFilters.exclude[MissingClassProblem]("scala.collection.MapView$Keys"),
ProblemFilters.exclude[MissingClassProblem]("scala.collection.MapView$Values"),
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to avoid making these jvm-public classes by creating anonymous inner classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know. I see your point, but at the same time, I think it's cleaner the way it is. I'd love to know what others think cc @julienrf

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a slight preference for having (private) named classes

@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Jul 1, 2020
dwijnand
dwijnand previously requested changes Jul 1, 2020
View reviewed changes
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

If you'll squash, I'll merge

@NthPortal
Copy link
Contributor Author

squashed

@SethTisue SethTisue dismissed dwijnand’s stale review July 2, 2020 00:47

requested change was made

@SethTisue SethTisue merged commit dae21fe into scala:2.13.x Jul 2, 2020
@NthPortal NthPortal deleted the topic/12059/PR branch July 2, 2020 07:57
@dwijnand dwijnand changed the title [bug#12059] Make MapView#values return a View Make MapView#values return a View Nov 12, 2020
@SethTisue SethTisue changed the title Make MapView#values return a View Make MapView#values preserve laziness Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienrf julienrf julienrf left review comments

@neonxray neonxray neonxray approved these changes

@dwijnand dwijnand dwijnand left review comments

Assignees
No one assigned
Labels
library:collections PRs involving changes to the standard collection library release-notes worth highlighting in next release notes
Projects
None yet
Milestone
2.13.4
6 participants
@NthPortal @julienrf @dwijnand @neonxray @SethTisue @scala-jenkins