Skip to content

Automatic widening of numeric types for primitive Streams #66

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

Merged
merged 2 commits into from
Apr 6, 2016

Conversation

szeiger
Copy link
Contributor

@szeiger szeiger commented Apr 1, 2016

The widening conversions follow the usual pattern. Float becomes
Double, Byte/Char/Short become Int. Boolean and Unit are not widened.

For Arrays, where the actual data is unboxed, widening is implemented
through custom Stepper implementations, so you can get e.g. a native
IntStepper for an Array[Byte]. The StreamConverters hierarchy is
modified to prefer these widened primitive steppers, so that, for
example, the seqStream extension method on an Array[Byte] returns
an IntStream.

Other collection types with a boxed representation also get the same
widening conversions (so you can still benefit from the better
performance of unboxed streams) but there are no implicitly available
widening steppers. Only the Stream extension methods fall back to
applying a widening stepper in order to produce a widening primitive
stream.

Widening narrow numeric types for streams is consistent with the Streams
API in the JDK. For example, CharSequence.chars returns an IntStream
instead of a Stream[Character].

The widening conversions follow the usual pattern. Float becomes
Double, Byte/Char/Short become Int. Boolean and Unit are not widened.

For Arrays, where the actual data is unboxed, widening is implemented
through custom Stepper implementations, so you can get e.g. a native
IntStepper for an Array[Byte]. The StreamConverters hierarchy is
modified to prefer these widened primitive steppers, so that, for
example, the `seqStream` extension method on an Array[Byte] returns
an IntStream.

Other collection types with a boxed representation also get the same
widening conversions (so you can still benefit from the better
performance of unboxed streams) but there are no implicitly available
widening steppers. Only the Stream extension methods fall back to
applying a widening stepper in order to produce a widening primitive
stream.

Widening narrow numeric types for streams is consistent with the Streams
API in the JDK. For example, `CharSequence.chars` returns an `IntStream`
instead of a `Stream[Character]`.

//TODO: None of these currently work because there are no native Stepper implementations. This does not only
// affect widening conversions though. While you can get, for example, an IntStepper for a WrappedArray[Int],
// all values have to go through a boxing/unboxing step!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ichoran, I think this part brings us back to the role of Steppers in the API and whether they should be exposed to the user. If so, we may want to provide widening conversions at the Stepper level for convenience, otherwise we could probably remove many of the existing primitive Stepper implementations which are based on boxed collections, and apply an unboxing Stepper similar to the new widening stepper adapters.

Explaining widening conversions, fixing errors, and clarifying the role
of Steppers in the API.
@szeiger
Copy link
Contributor Author

szeiger commented Apr 6, 2016

Added a commit with some README changes. After trying to lock down Steppers for the first release, I came to the conclusion that we shouldn't do it. We may still have to do some changes for better specialization and widening conversions but if we make Steppers private, people won't be able to implement them for their own collections.

@szeiger
Copy link
Contributor Author

szeiger commented Apr 6, 2016

Merging this so we can get a release out.

@szeiger szeiger merged commit 907dfcf into scala:master Apr 6, 2016
@szeiger szeiger deleted the wip/widening branch April 6, 2016 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant