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.

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

Redesign Tuples with HList-like structure #2199

Closed
wants to merge 11 commits into from
Closed
4 changes: 2 additions & 2 deletions .drone.yml
Expand Up @@ -38,9 +38,9 @@ matrix:
include:
- CI_TEST: dotty-bin-tests/test
CI_PUBLISH: true
- CI_TEST: legacyTests
- CI_TEST: ;publishLocal ;legacyTests
CI_PUBLISH: false
- CI_TEST: test
- CI_TEST: ;publishLocal ;test
CI_PUBLISH: false
- CI_TEST: ;publishLocal ;dotty-bootstrapped/test
CI_PUBLISH: false
2 changes: 1 addition & 1 deletion .drone.yml.sig
@@ -1 +1 @@
eyJhbGciOiJIUzI1NiJ9.IyBBZnRlciB1cGRhdGluZyB0aGlzIGZpbGUsIHlvdSBuZWVkIHRvIHJlLXNpZ24gaXQ6CiMKIyAtIEluc3RhbGwgW2Ryb25lLWNsaV0oaHR0cDovL3JlYWRtZS5kcm9uZS5pby91c2FnZS9nZXR0aW5nLXN0YXJ0ZWQtY2xpLykKIyAtIENvcHkgeW91ciB0b2tlbiBmcm9tICBodHRwOi8vZG90dHktY2kuZXBmbC5jaC9hY2NvdW50IChDbGljayBTSE9XIFRPS0VOKQojIC0gKGV4cG9ydCBEUk9ORV9UT0tFTj15b3VyLXRva2VuOyBleHBvcnQgRFJPTkVfU0VSVkVSPWh0dHA6Ly9kb3R0eS1jaS5lcGZsLmNoOyBkcm9uZSBzaWduIGxhbXBlcGZsL2RvdHR5KQojCiMgUGxlYXNlIG5vdGUgdGhhdCB0aGUgc2lnbmluZyBjYW4gb25seSBiZSBkb25lIGJ5IGNvbGxhYm9yYXRvcnMuCgpwaXBlbGluZToKICB0ZXN0OgogICAgaW1hZ2U6IGxhbXBlcGZsL2RvdHR5OjI0LTA0LTIwMTcKICAgIHB1bGw6IHRydWUKICAgIGNvbW1hbmRzOgogICAgICAtIGxuIC1zIC92YXIvY2FjaGUvZHJvbmUvc2NhbGEtc2NhbGEgc2NhbGEtc2NhbGEKICAgICAgLSBsbiAtcyAvdmFyL2NhY2hlL2Ryb25lL2l2eTIgIiRIT01FLy5pdnkyIgogICAgICAtIC4vcHJvamVjdC9zY3JpcHRzL3VwZGF0ZVNjYWxhTGlicmFyeQogICAgICAtIHNidCAtSi1YbXg0MDk2bSAtSi1YWDpSZXNlcnZlZENvZGVDYWNoZVNpemU9NTEybSAtSi1YWDpNYXhNZXRhc3BhY2VTaXplPTEwMjRtIC1EZG90dHkuZHJvbmUubWVtPTQwOTZtICIke0NJX1RFU1R9IgogICAgd2hlbjoKICAgICAgYnJhbmNoOgogICAgICAgIGV4Y2x1ZGU6IGdoLXBhZ2VzCgogIGRvY3VtZW50YXRpb246CiAgICBpbWFnZTogbGFtcGVwZmwvZG90dHk6MjQtMDQtMjAxNwogICAgcHVsbDogdHJ1ZQogICAgY29tbWFuZHM6CiAgICAgIC0gLi9wcm9qZWN0L3NjcmlwdHMvZ2VuRG9jcyAiJHtDSV9QVUJMSVNIfSIgJEJPVF9QQVNTCiAgICB3aGVuOgogICAgICBicmFuY2g6IG1hc3RlcgoKICBzbGFjazoKICAgIGltYWdlOiBwbHVnaW5zL3NsYWNrCiAgICBjaGFubmVsOiBkb3R0eQogICAgd2hlbjoKICAgICAgYnJhbmNoOiBtYXN0ZXIKICAgICAgc3RhdHVzOiBjaGFuZ2VkCgptYXRyaXg6CiAgaW5jbHVkZToKICAgIC0gQ0lfVEVTVDogZG90dHktYmluLXRlc3RzL3Rlc3QKICAgICAgQ0lfUFVCTElTSDogdHJ1ZQogICAgLSBDSV9URVNUOiBsZWdhY3lUZXN0cwogICAgICBDSV9QVUJMSVNIOiBmYWxzZQogICAgLSBDSV9URVNUOiB0ZXN0CiAgICAgIENJX1BVQkxJU0g6IGZhbHNlCiAgICAtIENJX1RFU1Q6IDtwdWJsaXNoTG9jYWwgO2RvdHR5LWJvb3RzdHJhcHBlZC90ZXN0CiAgICAgIENJX1BVQkxJU0g6IGZhbHNlCg.8EX_-bxlrDkovwBAfZ6d7HE162C7skwpZLQqQNDBrPo
eyJhbGciOiJIUzI1NiJ9.IyBBZnRlciB1cGRhdGluZyB0aGlzIGZpbGUsIHlvdSBuZWVkIHRvIHJlLXNpZ24gaXQ6CiMKIyAtIEluc3RhbGwgW2Ryb25lLWNsaV0oaHR0cDovL3JlYWRtZS5kcm9uZS5pby91c2FnZS9nZXR0aW5nLXN0YXJ0ZWQtY2xpLykKIyAtIENvcHkgeW91ciB0b2tlbiBmcm9tICBodHRwOi8vZG90dHktY2kuZXBmbC5jaC9hY2NvdW50IChDbGljayBTSE9XIFRPS0VOKQojIC0gKGV4cG9ydCBEUk9ORV9UT0tFTj15b3VyLXRva2VuOyBleHBvcnQgRFJPTkVfU0VSVkVSPWh0dHA6Ly9kb3R0eS1jaS5lcGZsLmNoOyBkcm9uZSBzaWduIGxhbXBlcGZsL2RvdHR5KQojCiMgUGxlYXNlIG5vdGUgdGhhdCB0aGUgc2lnbmluZyBjYW4gb25seSBiZSBkb25lIGJ5IGNvbGxhYm9yYXRvcnMuCgpwaXBlbGluZToKICB0ZXN0OgogICAgaW1hZ2U6IGxhbXBlcGZsL2RvdHR5OjI0LTA0LTIwMTcKICAgIHB1bGw6IHRydWUKICAgIGNvbW1hbmRzOgogICAgICAtIGxuIC1zIC92YXIvY2FjaGUvZHJvbmUvc2NhbGEtc2NhbGEgc2NhbGEtc2NhbGEKICAgICAgLSBsbiAtcyAvdmFyL2NhY2hlL2Ryb25lL2l2eTIgIiRIT01FLy5pdnkyIgogICAgICAtIC4vcHJvamVjdC9zY3JpcHRzL3VwZGF0ZVNjYWxhTGlicmFyeQogICAgICAtIHNidCAtSi1YbXg0MDk2bSAtSi1YWDpSZXNlcnZlZENvZGVDYWNoZVNpemU9NTEybSAtSi1YWDpNYXhNZXRhc3BhY2VTaXplPTEwMjRtIC1EZG90dHkuZHJvbmUubWVtPTQwOTZtICIke0NJX1RFU1R9IgogICAgd2hlbjoKICAgICAgYnJhbmNoOgogICAgICAgIGV4Y2x1ZGU6IGdoLXBhZ2VzCgogIGRvY3VtZW50YXRpb246CiAgICBpbWFnZTogbGFtcGVwZmwvZG90dHk6MjQtMDQtMjAxNwogICAgcHVsbDogdHJ1ZQogICAgY29tbWFuZHM6CiAgICAgIC0gLi9wcm9qZWN0L3NjcmlwdHMvZ2VuRG9jcyAiJHtDSV9QVUJMSVNIfSIgJEJPVF9QQVNTCiAgICB3aGVuOgogICAgICBicmFuY2g6IG1hc3RlcgoKICBzbGFjazoKICAgIGltYWdlOiBwbHVnaW5zL3NsYWNrCiAgICBjaGFubmVsOiBkb3R0eQogICAgd2hlbjoKICAgICAgYnJhbmNoOiBtYXN0ZXIKICAgICAgc3RhdHVzOiBjaGFuZ2VkCgptYXRyaXg6CiAgaW5jbHVkZToKICAgIC0gQ0lfVEVTVDogZG90dHktYmluLXRlc3RzL3Rlc3QKICAgICAgQ0lfUFVCTElTSDogdHJ1ZQogICAgLSBDSV9URVNUOiA7cHVibGlzaExvY2FsIGxlZ2FjeVRlc3RzCiAgICAgIENJX1BVQkxJU0g6IGZhbHNlCiAgICAtIENJX1RFU1Q6IDtwdWJsaXNoTG9jYWwgdGVzdAogICAgICBDSV9QVUJMSVNIOiBmYWxzZQogICAgLSBDSV9URVNUOiA7cHVibGlzaExvY2FsIDtkb3R0eS1ib290c3RyYXBwZWQvdGVzdAogICAgICBDSV9QVUJMSVNIOiBmYWxzZQo.GVIr2uV70DwLepKvh4Gc7kGhHM5esvxWCNxabm_GUkw
8 changes: 7 additions & 1 deletion bin/common
Expand Up @@ -90,7 +90,13 @@ function build_all {
printf "done\n"

printf "Building dotty library..."
DOTTY_LIB_JAR=$(build_jar dotty-library/package "library/target/scala-$SCALA_BINARY_VERSION")

cd "$DOTTY_ROOT" >& /dev/null
sbt publish-local >& /dev/null
cd - >& /dev/null

DOTTY_LIB_JAR=$(build_jar dotty-library-bootstrapped/package "out/dotty-library-bootstrapped/scala-$SCALA_BINARY_VERSION")

printf "done\n"

printf "Building tests..."
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/Compiler.scala
Expand Up @@ -48,7 +48,8 @@ class Compiler {
List(new Pickler), // Generate TASTY info
List(new FirstTransform, // Some transformations to put trees into a canonical form
new CheckReentrant), // Internal use only: Check that compiled program has no data races involving global vars
List(new CheckStatic, // Check restrictions that apply to @static members
List(new TupleRewrites, // Rewrite tuple apply and unapply
new CheckStatic, // Check restrictions that apply to @static members
new ElimRepeated, // Rewrite vararg parameters and arguments
new RefChecks, // Various checks mostly related to abstract members and overriding
new NormalizeFlags, // Rewrite some definition flags
Expand Down
32 changes: 20 additions & 12 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Expand Up @@ -414,10 +414,10 @@ object desugar {
}
}

// Above MaxTupleArity we extend Product instead of ProductN, in this
// Above MaxImplementedTupleArity we extend Product instead of ProductN, in this
// case we need to synthesise productElement & productArity.
def largeProductMeths =
if (arity > Definitions.MaxTupleArity) productElement :: productArity :: Nil
if (arity > Definitions.MaxImplementedTupleArity) productElement :: productArity :: Nil
else Nil

if (isCaseClass)
Expand All @@ -432,7 +432,7 @@ object desugar {
if (targs.isEmpty) tycon else AppliedTypeTree(tycon, targs)
}
def product =
if (arity > Definitions.MaxTupleArity) scalaDot(str.Product.toTypeName)
if (arity > Definitions.MaxImplementedTupleArity) scalaDot(str.Product.toTypeName)
else productConstr(arity)

// Case classes and case objects get Product/ProductN parents
Expand Down Expand Up @@ -764,7 +764,7 @@ object desugar {
val param = makeSyntheticParameter()
def selector(n: Int) = Select(refOfDef(param), nme.selectorName(n))
val vdefs =
params.zipWithIndex.map{
params.zipWithIndex.map {
case (param, idx) =>
DefDef(param.name, Nil, Nil, TypeTree(), selector(idx)).withPos(param.pos)
}
Expand Down Expand Up @@ -1068,14 +1068,22 @@ object desugar {
t
case Tuple(ts) =>
val arity = ts.length
def tupleTypeRef = defn.TupleType(arity)
if (arity > Definitions.MaxTupleArity) {
ctx.error(TupleTooLong(ts), tree.pos)
unitLiteral
} else if (arity == 1) ts.head
else if (ctx.mode is Mode.Type) AppliedTypeTree(ref(tupleTypeRef), ts)
else if (arity == 0) unitLiteral
else Apply(ref(tupleTypeRef.classSymbol.companionModule.valRef), ts)
arity match {
case 0 => unitLiteral
case _ if ctx.mode is Mode.Type =>
// Transforming Tuple types: (T1, T2) → TupleCons[T1, TupleCons[T2, Unit]]
val nil: Tree = TypeTree(defn.UnitType)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use hnilType for consistency with hconsType?

def hconsType(l: Tree, r: Tree): Tree =
AppliedTypeTree(ref(defn.TupleConsType), l :: r :: Nil)
ts.foldRight(nil)(hconsType)
case _ =>
// Transforming Tuple trees: (T1, T2, ..., TN) → TupleCons(T1, TupleCons(T2, ... (TupleCons(TN, ())))
val nil: Tree = unitLiteral
Copy link
Contributor

Choose a reason for hiding this comment

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

Again for naming consistency, use hnil, hcons?

val cons = defn.TupleConsType.classSymbol.companionModule.valRef
def consTree(l: Tree, r: Tree): Tree =
Apply(ref(cons), l :: r :: Nil)
ts.foldRight(nil)(consTree)
}
case WhileDo(cond, body) =>
// { <label> def while$(): Unit = if (cond) { body; while$() } ; while$() }
val call = Apply(Ident(nme.WHILE_PREFIX), Nil).withPos(tree.pos)
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/config/CompilerCommand.scala
@@ -1,4 +1,3 @@

package dotty.tools.dotc
package config

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/config/Config.scala
Expand Up @@ -140,7 +140,7 @@ object Config {
final val LogPendingUnderlyingThreshold = 50

/** How many recursive calls to isSubType are performed before logging starts. */
final val LogPendingSubTypesThreshold = 50
final val LogPendingSubTypesThreshold = 70
Copy link
Contributor

@odersky odersky May 7, 2017

Choose a reason for hiding this comment

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

Why was this increased?

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 have some tests with size 24 tuples that triggers this limit, I'm going to revert this change and run these test without -Yno-deep-subtypes.


/** How many recursive calls to findMember are performed before logging names starts
* Note: this threshold has to be chosen carefully. Too large, and programs
Expand Down
46 changes: 24 additions & 22 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Expand Up @@ -12,11 +12,12 @@ import collection.mutable
import util.common.alwaysZero

object Definitions {

/** The maximum number of elements in a tuple or product.
* This should be removed once we go to hlists.
/** The maximum arity N of Tuple implemented as `scala.TupleN` case classes.
* Tuple of higher arity us an array based representation (`dotty.LargeTuple`).
* The limit 22 is chosen for Scala2x interop. It could be something
* else without affecting the set of programs that can be compiled.
*/
val MaxTupleArity = 22
val MaxImplementedTupleArity = 22

/** The maximum arity N of a function type that's implemented
* as a trait `scala.FunctionN`. Functions of higher arity are possible,
Expand Down Expand Up @@ -382,7 +383,6 @@ class Definitions {
lazy val ArrayModuleType = ctx.requiredModuleRef("scala.Array")
def ArrayModule(implicit ctx: Context) = ArrayModuleType.symbol.moduleClass.asClass


lazy val UnitType: TypeRef = valueTypeRef("scala.Unit", BoxedUnitType, java.lang.Void.TYPE, UnitEnc)
def UnitClass(implicit ctx: Context) = UnitType.symbol.asClass
lazy val BooleanType = valueTypeRef("scala.Boolean", BoxedBooleanType, java.lang.Boolean.TYPE, BooleanEnc)
Expand Down Expand Up @@ -701,8 +701,19 @@ class Definitions {
private lazy val ImplementedFunctionType = mkArityArray("scala.Function", MaxImplementedFunctionArity, 0)
def FunctionClassPerRun = new PerRun[Array[Symbol]](implicit ctx => ImplementedFunctionType.map(_.symbol.asClass))

lazy val TupleType = mkArityArray("scala.Tuple", MaxTupleArity, 2)
lazy val ProductNType = mkArityArray("scala.Product", MaxTupleArity, 0)
lazy val TupleNType = mkArityArray("scala.Tuple", MaxImplementedTupleArity, 1)
lazy val TupleNSymbol = TupleNType.map(t => if (t == null) t else t.classSymbol)
lazy val DottyTupleNType = mkArityArray("dotty.DottyTuple", MaxImplementedTupleArity, 1)
Copy link
Contributor

@odersky odersky May 7, 2017

Choose a reason for hiding this comment

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

Defined like this TupleNSymbol is an Array[AnyRef]. It's better to use if (t == null) NoSymbol which makes it an Array[Symbol].

lazy val DottyTupleNModule = DottyTupleNType.map(t => if (t == null) t else t.classSymbol.companionModule.symbol)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

lazy val ProductNType = mkArityArray("scala.Product", MaxImplementedTupleArity, 0)

lazy val TupleType = ctx.requiredClassRef("dotty.Tuple")
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to keep to the same conventions as done in the rest of Definitions here. The way you define things will make problems if we recompile the dotty classes in question in an incremental setting. Example of how it should be done:

lazy val TupleConsType: TypeRef = ctx.requiredClassRef("scala.runtime.BoxedUnit")
def TupleConsClass(implicit ctx: Context) = TupleConsType.symbol.asClass
def TupleConsModule = TupleConsClass.companionModule

lazy val TupleConsType = ctx.requiredClassRef("dotty.TupleCons")
lazy val TupleConsModule = TupleConsType.classSymbol.companionModule.symbol
lazy val TupleUnapplySeqType = ctx.requiredClassRef("dotty.LargeTupleUnapplySeq$")
lazy val TupleUnapplySeqModule = TupleUnapplySeqType.classSymbol.companionModule.symbol
lazy val LargeTupleType = ctx.requiredClassRef("dotty.LargeTuple")
lazy val LargeTupleModule = LargeTupleType.classSymbol.companionModule.symbol

def FunctionClass(n: Int, isImplicit: Boolean = false)(implicit ctx: Context) =
if (isImplicit) ctx.requiredClass("scala.ImplicitFunction" + n.toString)
Expand All @@ -716,9 +727,6 @@ class Definitions {
if (n <= MaxImplementedFunctionArity && (!isImplicit || ctx.erasedTypes)) ImplementedFunctionType(n)
else FunctionClass(n, isImplicit).typeRef

private lazy val TupleTypes: Set[TypeRef] = TupleType.toSet
private lazy val ProductTypes: Set[TypeRef] = ProductNType.toSet

/** If `cls` is a class in the scala package, its name, otherwise EmptyTypeName */
def scalaClassName(cls: Symbol)(implicit ctx: Context): TypeName =
if (cls.isClass && cls.owner == ScalaPackageClass) cls.asClass.name else EmptyTypeName
Expand Down Expand Up @@ -773,7 +781,7 @@ class Definitions {
*/
def erasedFunctionClass(cls: Symbol): Symbol = {
val arity = scalaClassName(cls).functionArity
if (arity > 22) defn.FunctionXXLClass
if (arity > MaxImplementedFunctionArity) defn.FunctionXXLClass
else if (arity >= 0) defn.FunctionClass(arity)
else NoSymbol
}
Expand All @@ -787,7 +795,7 @@ class Definitions {
*/
def erasedFunctionType(cls: Symbol): Type = {
val arity = scalaClassName(cls).functionArity
if (arity > 22) defn.FunctionXXLType
if (arity > MaxImplementedFunctionArity) defn.FunctionXXLType
else if (arity >= 0) defn.FunctionType(arity)
else NoType
}
Expand All @@ -805,8 +813,8 @@ class Definitions {
)

val PredefImportFns = List[() => TermRef](
() => ScalaPredefModuleRef,
() => DottyPredefModuleRef
() => ScalaPredefModuleRef,
() => DottyPredefModuleRef
)

lazy val RootImportFns =
Expand Down Expand Up @@ -838,14 +846,8 @@ class Definitions {
def isPolymorphicAfterErasure(sym: Symbol) =
(sym eq Any_isInstanceOf) || (sym eq Any_asInstanceOf)

def isTupleType(tp: Type)(implicit ctx: Context) = {
val arity = tp.dealias.argInfos.length
arity <= MaxTupleArity && TupleType(arity) != null && (tp isRef TupleType(arity).symbol)
}

def tupleType(elems: List[Type]) = {
TupleType(elems.size).appliedTo(elems)
}
def isTupleType(tp: Type)(implicit ctx: Context) =
tp.derivesFrom(TupleType.symbol)

def isProductSubType(tp: Type)(implicit ctx: Context) =
tp.derivesFrom(ProductType.symbol)
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Expand Up @@ -72,8 +72,6 @@ import Decorators.SymbolIteratorDecorator
*/
object Denotations {

implicit def eqDenotation: Eq[Denotation, Denotation] = Eq

Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this removed? Do we really want to compare denotations with arbitrary types for equality? This is dangerous as it is all too easy to compare a symbol with a denotation and get constant false!

/** A denotation is the result of resolving
* a name (either simple identifier or select) during a given period.
*
Expand Down Expand Up @@ -1225,4 +1223,4 @@ object Denotations {
util.Stats.record("stale symbol")
override def getMessage() = msg
}
}
}
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Flags.scala
Expand Up @@ -237,7 +237,7 @@ object Flags {

final val AccessorOrSealed = Accessor.toCommonFlags

/** A mutable var */
/** A mutable var */
final val Mutable = termFlag(12, "mutable")

/** Symbol is local to current class (i.e. private[this] or protected[this]
Expand All @@ -252,7 +252,7 @@ object Flags {
final val TermParamAccessor = ParamAccessor.toTermFlags
final val TypeParamAccessor = ParamAccessor.toTypeFlags

/** A value or class implementing a module */
/** A value or class implementing a module */
final val Module = commonFlag(15, "module")
final val ModuleVal = Module.toTermFlags
final val ModuleClass = Module.toTypeFlags
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Names.scala
Expand Up @@ -30,8 +30,6 @@ object Names {
def toTermName: TermName
}

implicit def eqName: Eq[Name, Name] = Eq
Copy link
Contributor

Choose a reason for hiding this comment

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

Same remark as for Denotations applies: Please don't remove multiversal equality markers.


/** A name is essentially a string, with three differences
* 1. Names belong in one of two name spaces: they are type names or term names.
* Term names have a sub-category of "local" field names.
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/StdNames.scala
Expand Up @@ -519,6 +519,7 @@ object StdNames {
val unapply: N = "unapply"
val unapplySeq: N = "unapplySeq"
val unbox: N = "unbox"
val underlying: N = "underlying"
val universe: N = "universe"
val update: N = "update"
val updateDynamic: N = "updateDynamic"
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Symbols.scala
Expand Up @@ -379,8 +379,6 @@ trait Symbols { this: Context =>

object Symbols {

implicit def eqSymbol: Eq[Symbol, Symbol] = Eq
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto


/** A Symbol represents a Scala definition/declaration or a package.
* @param coord The coordinates of the symbol (a position or an index)
* @param id A unique identifier of the symbol (unique per ContextBase)
Expand Down
29 changes: 27 additions & 2 deletions compiler/src/dotty/tools/dotc/core/TypeErasure.scala
Expand Up @@ -325,6 +325,10 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean

/** The erasure |T| of a type T. This is:
*
* - For dotty.TupleCons:
* - if lenght staticaly known as N, TupleN
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

* - otherwise Product
* - For dotty.Tuple, java.lang.Object
* - For a refined type scala.Array+[T]:
* - if T is Nothing or Null, []Object
* - otherwise, if T <: Object, []|T|
Expand All @@ -347,7 +351,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
* - For any other uncurried method type (Fs)T, (|Fs|)|T|.
* - For a curried method type (Fs1)(Fs2)T, (|Fs1|,Es2)ET where (Es2)ET = |(Fs2)T|.
* - For a polymorphic type [Ts](Ps)T, |(Ps)T|
* _ For a polymorphic type [Ts]T where T is not a method type, ()|T|
* - For a polymorphic type [Ts]T where T is not a method type, ()|T|
* - For the class info type of java.lang.Object, the same type without any parents.
* - For a class info type of a value class, the same type without any parents.
* - For any other class info type with parents Ps, the same type with
Expand All @@ -356,6 +360,27 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
* - For any other type, exception.
*/
private def apply(tp: Type)(implicit ctx: Context): Type = tp match {
case _ if tp.isRef(defn.TupleConsType.symbol) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

This imposes two leading checks on every call of erasure, which is a hot operation. It's better to roll the tests into the case for RefinedType, like this:

case tp: RefinedType =>
  val parent = tp.parent
  if (parent isRef defn.ArrayClass) eraseArray(tp)
  else if (parent isRef defn.TupleConsClass) eraseTuple(tp)
  else this(parent)

... and roll the logic here into a new method eraseTuple. It's easier to read and more efficient (need to avoid overly long hot methods, they never get optimized well).

// Compute the arity of a tuple type, -1 if it's not statically known.
def tupleArity(t: Type, acc: Int = 0): Int = t match {
case RefinedType(RefinedType(_, _, TypeAlias(headType)), _, TypeAlias(tailType)) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

This will get confused if you refine a tuple type explicitly. Better also check for the refinedNames here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or use an AppliedType extractor.

tupleArity(tailType, acc + 1)
case _ if t.isRef(defn.UnitType.symbol) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

This will get confused if you refine a tuple type explicitly. Better also check for the refinedNames here.

acc
case AnnotatedType(tpe, _) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant case: AnnotatedType is a TypeProxy.

tupleArity(tpe, acc)
case tp: TypeProxy =>
tupleArity(tp.underlying, acc)
case t =>
-1
}
val arity = tupleArity(tp)
if (arity > 0 && arity <= Definitions.MaxImplementedTupleArity)
defn.TupleNType(arity)
else
defn.ProductType
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not a more specific type instead of ProductType? I.e. LargeTupleType?

case _ if tp.isRef(defn.TupleType.symbol) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

This one needs to go as well.

defn.ObjectType
case _: ErasedValueType =>
tp
case tp: TypeRef =>
Expand Down Expand Up @@ -469,7 +494,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
// constructor method should not be semi-erased.
else if (isConstructor && isDerivedValueClass(sym)) eraseNormalClassRef(tp)
else this(tp)
case RefinedType(parent, _, _) if !(parent isRef defn.ArrayClass) =>
case RefinedType(parent, _, _) if !(parent isRef defn.ArrayClass) && !(tp isRef defn.TupleConsType.symbol) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

TupleConsClass

eraseResult(parent)
case _ =>
this(tp)
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Expand Up @@ -41,8 +41,6 @@ object Types {

@sharable private var nextId = 0

implicit def eqType: Eq[Type, Type] = Eq
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto


/** Main class representing types.
*
* The principal subclasses and sub-objects are as follows:
Expand Down