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

SI-8627 make Stream.filterNot non-eager #3949

Merged
merged 1 commit into from Aug 27, 2014

Conversation

Projects
None yet
5 participants
@lrytz
Copy link
Member

lrytz commented Aug 26, 2014

The obvious fix, overriding filterNot in Stream, is not binary
compatible, see #3925

Instead, this makes filterImpl in TaversableLike private[scala],
which allows overriding it in Stream. The corresponding mima-failures
can be whitelisted, as the changes are only to private[scala].

In 2.12.x we can remove the override of filter in Stream, but in
2.11.x this is not binary compatible.

Eventually we'd also like to make filter / filterNot in
TraversableLike final, but that's not source compatible, so it cannot
be done in 2.12.x.

@lrytz

This comment has been minimized.

Copy link
Member Author

lrytz commented Aug 26, 2014

Oh, I realize this is not binary compatible, because of mixin.

@lrytz lrytz closed this Aug 26, 2014

@lrytz

This comment has been minimized.

Copy link
Member Author

lrytz commented Aug 26, 2014

Hm, actually, I'm not sure. Maybe this is OK. The mixed-in filterImpl methods are only ever called from within the collection library, but never from client code. So if you replace the scala library on the classpath, client code should always work. @gkossakowski wdyt?

@lrytz lrytz reopened this Aug 26, 2014

@Ichoran

This comment has been minimized.

Copy link
Contributor

Ichoran commented Aug 26, 2014

@lrytz - It sounds to me like this should work, but if not it can certainly be caught via type-casting and putting the logic in TraversableLike. I'm extremely reluctant to do that, though, given how much of a hack it is.

SI-8627 make Stream.filterNot non-eager
The obvious fix, overriding `filterNot` in Stream, is not binary
compatible, see #3925

Instead, this makes `filterImpl` in TaversableLike private[scala],
which allows overriding it in Stream. The corresponding mima-failures
can be whitelisted, as the changes are only to private[scala].

In 2.12.x we can remove the override of `filter` in Stream, but in
2.11.x this is not binary compatible.

Eventually we'd also like to make filter / filterNot in
TraversableLike final, but that's not source compatible, so it cannot
be done in 2.12.x.

@lrytz lrytz force-pushed the lrytz:t8627 branch from aa5be82 to 9276a12 Aug 27, 2014

@gkossakowski

This comment has been minimized.

Copy link
Member

gkossakowski commented Aug 27, 2014

@lrytz, yes your change is safe BC-wise for the reason you have given: scala-library is just one piece that can be replaced just entirely.

@lrytz

This comment has been minimized.

Copy link
Member Author

lrytz commented Aug 27, 2014

So LGTY?

@gkossakowski

This comment has been minimized.

Copy link
Member

gkossakowski commented Aug 27, 2014

Yep. LGTM.

gkossakowski added a commit that referenced this pull request Aug 27, 2014

Merge pull request #3949 from lrytz/t8627
SI-8627 make Stream.filterNot non-eager

@gkossakowski gkossakowski merged commit 7693cec into scala:2.11.x Aug 27, 2014

1 check passed

default pr-scala Took 54 min.
Details

@lrytz lrytz deleted the lrytz:t8627 branch Sep 3, 2014

@xuwei-k

This comment has been minimized.

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