-
Notifications
You must be signed in to change notification settings - Fork 1.1k
WORK IN PROGRESS: New representation for arrays of value classes #729
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It will be added by the VCXPrototype parent class
It will be useful to wrap SeqLiterals
This is needed so that Array[T] erases to Object, see the note in the code.
It should be "[Foo" not "[class Foo"
…RefArray wrapRefArray is defined as: def wrapRefArray[T <: AnyRef](xs: Array[T]): WrappedArray[T] After erasure this becomes: def wrapRefArray(xs: Object[]): WrappedArray This is fine for value classes as long as arrays are boxed, but once we change the representation this will result in ClassCastException. Therefore we introduce a wrapper just for value classes: def wrapVCArray[T <: AnyVal](xs: Array[T]): WrappedArray[T] Which is erased to: def wrapVCArray(xs: Object): WrappedArray Which lets us represent arrays of value classes using any reference type.
Note that this mean that the following does not pass Ycheck after VCParents: def foo[T <: AnyVal](...) foo[Meter](...) Ycheck will complain that Meter does not conform to the upper bound AnyVal, which is true. I'm not sure what the best way to deal with this is.
For a value class V whose underlying type is U, instead of representing an array of V as V[] on the JVM, we use our own class VCXArray where X is "U" if U is a primitive type and is "Object" otherwise. This avoids boxing when creating arrays but we still need to box and unbox when using such an array in a generic position, this also breaks reflection and makes it pretty hard to use an array of a value class from Java or Scala 2. See also the FIXMEs in tests/run/valueclasses-array.scala for the current implementation restrictions.
Note that this will crash if these methods are present in the source code, we either needs to disallow them in the source code or find an alternative design.
This required adding a new magic method Arrays#vcArray to remember the value class type of a SeqLiteral.
I have no idea why this works better.
Cool! Looking forward to this patch making it into dotty! Congrats @smarter and @DarkDimius! |
(Superceded by #1228) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For a value class V whose underlying type is U, instead of representing
an array of V as V[] on the JVM, we use our own class VCXArray where X
is "U" if U is a primitive type and is "Object" otherwise.
This avoids boxing when creating arrays but we still need to box and
unbox when using such an array in a generic position, this also breaks
reflection and makes it pretty hard to use an array of a value class
from Java or Scala 2.