-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Forked ScalaRuntime and minimal classtags for arrays of value classes. #515
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
Conversation
@smarter please review. |
@@ -88,6 +88,7 @@ class ScalaSettings extends Settings.SettingGroup { | |||
val Xshowobj = StringSetting("-Xshow-object", "object", "Show internal representation of object.", "") | |||
val showPhases = BooleanSetting("-Xshow-phases", "Print a synopsis of compiler phases.") | |||
val sourceReader = StringSetting("-Xsource-reader", "classname", "Specify a custom method for reading source files.", "") | |||
val XnoValueClasses = BooleanSetting("-Xno-value-classes", "Do not use value classes. Helps debugging.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be a -Y option? It's a "compiler hacker" option and it's a bit dangerous since toggling it can break an API since it changes method signatures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would actually be using it during my everyday development and would recommend others, because it simplifies debugging. So I'd not consider it as "compiler hacker" option.
Though I'm fine with it being either -X
or -Y
restarting build. |
Otherwise LGTM |
final def _1$extension(underlying: Boolean) = underlying | ||
final def hashCode$extension(underlying: Boolean) = underlying.hashCode() | ||
final def toString$extension(underlying: Boolean) = s"${productPrefix$extension(underlying)}($underlying)" | ||
def productPrefix$extension(underlying: Boolean): String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Value classes which are not case classes will not implement productPrefix
, this should not be here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scalac actually synthesizes toString on value classes, which is productPrefix
+ hex-encoded underlying value.
I belive that always implementing productPrefix
does make sense.
Making actual value classes extend this prototypes serves two goals: - This is a basis for implementing arrays of value classes. - Having underlying final in those glasses makes all unbox methods monomorphic and final. - this decreases size of an empty case-class to 1/3 of previous size.
Decreases the size of companion of case value class to 1/8 of it's original size.
Works as Array[T] is erased to Object.
Unfortunately needed to sacrifice the immutability of inner field.
[error] /Users/dark/workspace/dotty/src/dotty/runtime/vc/VCPrototype.scala:10: overriding method clone in class VCArrayPrototype of type ()Object; [error] method clone in class Object of type ()Object has weaker access privileges; it should be public; [error] (Note that method clone in class VCArrayPrototype of type ()Object is abstract, [error] and is therefore overridden by concrete method clone in class Object of type ()Object) [error] abstract class VCArrayPrototype[T <: VCPrototype] extends Object with Cloneable {
It can be done with non-abstract rhs of method.
c7e7cc5
to
9d8a8de
Compare
Forked ScalaRuntime and minimal classtags for arrays of value classes.
No description provided.