-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Remove PreName add (almost) all other implicit conversions #4077
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
Changes from all commits
1e471b7
2aa965c
57df5b5
163509d
f4e9ccd
a50b2d4
b18fba5
e07a68d
ebe084f
d870b36
c064915
9866c74
27ee9a4
4f119bc
f2fba0b
c01612a
0bed6fd
01cc814
c49d246
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,3 +73,4 @@ compiler/after-pickling.txt | |
*.dotty-ide-version | ||
|
||
*.decompiled.out | ||
bench/compile.txt | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -154,9 +154,9 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
lazy val Predef_classOf: Symbol = defn.ScalaPredefModule.requiredMethod(nme.classOf) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer we leave PreName, but move the String -> PreName conversion to the companion object of PreName. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That doesn't work as moving it the companion of PreName is not enough to make .toTermName and .toTypeName available on Strings. I guess we could have two implicit conversions, one in PreName's companion and one available by import, but I feel it would just make things worse... Are you sure you don't like this commit as is? In my opinion, completely removing an abstraction from the code base is a clear simplification. The diff might look large but in reality, most of the line changes are about making type more precise by having the TermName/TypeName instead of PreName. The balance sheet in term of added boilerplate is also reasonable: toTermName added: 22 toTypeName added: 12 +4 overloaded methods with String & Name arguments There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think we should leave PreName. I don't like to have to overload methods, and the problem is, if you are systematic about it (and you should be!) you will end up with a lot more overloaded methods. I realize it's a contentious issue, but then the maxime "when in doubt, don't change it" applies. One other thing: We have to tell everyone working on the compiler that |
||
|
||
lazy val AnnotationRetentionAttr = ctx.requiredClass("java.lang.annotation.Retention") | ||
lazy val AnnotationRetentionSourceAttr = ctx.requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("SOURCE") | ||
lazy val AnnotationRetentionClassAttr = ctx.requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("CLASS") | ||
lazy val AnnotationRetentionRuntimeAttr = ctx.requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("RUNTIME") | ||
lazy val AnnotationRetentionSourceAttr = ctx.requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("SOURCE".toTermName) | ||
lazy val AnnotationRetentionClassAttr = ctx.requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("CLASS".toTermName) | ||
lazy val AnnotationRetentionRuntimeAttr = ctx.requiredClass("java.lang.annotation.RetentionPolicy").linkedClass.requiredValue("RUNTIME".toTermName) | ||
lazy val JavaAnnotationClass = ctx.requiredClass("java.lang.annotation.Annotation") | ||
|
||
def boxMethods: Map[Symbol, Symbol] = defn.ScalaValueClasses().map{x => // @darkdimius Are you sure this should be a def? | ||
|
@@ -366,7 +366,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
def getAnnotPickle(jclassName: String, sym: Symbol): Option[Annotation] = None | ||
|
||
|
||
def getRequiredClass(fullname: String): Symbol = ctx.requiredClass(fullname.toTermName) | ||
def getRequiredClass(fullname: String): Symbol = ctx.requiredClass(fullname) | ||
|
||
def getClassIfDefined(fullname: String): Symbol = NoSymbol // used only for android. todo: implement | ||
|
||
|
@@ -376,12 +376,12 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
} | ||
|
||
def requiredClass[T](implicit evidence: ClassTag[T]): Symbol = | ||
ctx.requiredClass(erasureString(evidence.runtimeClass).toTermName) | ||
ctx.requiredClass(erasureString(evidence.runtimeClass)) | ||
|
||
def requiredModule[T](implicit evidence: ClassTag[T]): Symbol = { | ||
val moduleName = erasureString(evidence.runtimeClass) | ||
val className = if (moduleName.endsWith("$")) moduleName.dropRight(1) else moduleName | ||
ctx.requiredModule(className.toTermName) | ||
ctx.requiredModule(className) | ||
} | ||
|
||
|
||
|
@@ -498,7 +498,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
// unrelated change. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am against merging this as is. I believe the original version is both clearer and more efficient. Computing |
||
ctx.base.settings.YnoGenericSig.value | ||
|| sym.is(Flags.Artifact) | ||
|| sym.is(Flags.allOf(Flags.Method, Flags.Lifted)) | ||
|| sym.isBoth(Flags.Method, and = Flags.Lifted) | ||
|| sym.is(Flags.Bridge) | ||
) | ||
|
||
|
@@ -679,7 +679,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
def isConstructor: Boolean = toDenot(sym).isConstructor | ||
def isAnonymousFunction: Boolean = toDenot(sym).isAnonymousFunction | ||
def isMethod: Boolean = sym is Flags.Method | ||
def isPublic: Boolean = sym.flags.is(Flags.EmptyFlags, Flags.Private | Flags.Protected) | ||
def isPublic: Boolean = sym.flags.is(Flags.EmptyFlags, butNot=Flags.Private | Flags.Protected) | ||
def isSynthetic: Boolean = sym is Flags.Synthetic | ||
def isPackageClass: Boolean = sym is Flags.PackageClass | ||
def isModuleClass: Boolean = sym is Flags.ModuleClass | ||
|
@@ -716,7 +716,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
def isDeprecated: Boolean = false | ||
def isMutable: Boolean = sym is Flags.Mutable | ||
def hasAbstractFlag: Boolean = | ||
(sym is Flags.Abstract) || (sym is Flags.JavaInterface) || (sym is Flags.Trait) | ||
(sym is Flags.Abstract) || (sym is Flags.Trait) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because is(JavaInterface) implies is(Trait): https://github.com/lampepfl/dotty/blob/84bf2fac1aac3f381f2f3f68cc8a7dde538aca95/compiler/src/dotty/tools/dotc/core/Flags.scala#L614 |
||
def hasModuleFlag: Boolean = sym is Flags.Module | ||
def isSynchronized: Boolean = sym is Flags.Synchronized | ||
def isNonBottomSubClass(other: Symbol): Boolean = sym.derivesFrom(other) | ||
|
@@ -808,26 +808,28 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
else Nil | ||
|
||
def annotations: List[Annotation] = toDenot(sym).annotations | ||
def companionModuleMembers: List[Symbol] = { | ||
|
||
def companionModuleMembers: List[Symbol] = { | ||
// phase travel to exitingPickler: this makes sure that memberClassesOf only sees member classes, | ||
// not local classes of the companion module (E in the exmaple) that were lifted by lambdalift. | ||
if (linkedClass.isTopLevelModuleClass) /*exitingPickler*/ linkedClass.memberClasses | ||
else Nil | ||
} | ||
|
||
def fieldSymbols: List[Symbol] = { | ||
toDenot(sym).info.decls.filter(p => p.isTerm && !p.is(Flags.Method)) | ||
} | ||
|
||
def methodSymbols: List[Symbol] = | ||
for (f <- toDenot(sym).info.decls.toList if f.isMethod && f.isTerm && !f.isModule) yield f | ||
def serialVUID: Option[Long] = None | ||
|
||
def serialVUID: Option[Long] = None | ||
|
||
def freshLocal(cunit: CompilationUnit, name: String, tpe: Type, pos: Position, flags: Flags): Symbol = { | ||
ctx.newSymbol(sym, name.toTermName, FlagSet(flags), tpe, NoSymbol, pos) | ||
} | ||
def freshLocal(cunit: CompilationUnit, name: String, tpe: Type, pos: Position, flags: Flags): Symbol = | ||
ctx.newSymbol(sym, name.toTermName, FlagSet(flags), tpe, NoSymbol, pos.toCoord) | ||
|
||
def getter(clz: Symbol): Symbol = decorateSymbol(sym).getter | ||
def setter(clz: Symbol): Symbol = decorateSymbol(sym).setter | ||
def getter(clz: Symbol): Symbol = new SymUtilsOps(sym).getter | ||
def setter(clz: Symbol): Symbol = new SymUtilsOps(sym).setter | ||
|
||
def moduleSuffix: String = "" // todo: validate that names already have $ suffix | ||
def outputDirectory: AbstractFile = DottyBackendInterface.this.outputDirectory | ||
|
@@ -840,7 +842,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
* Redundant interfaces are removed unless there is a super call to them. | ||
*/ | ||
def superInterfaces: List[Symbol] = { | ||
val directlyInheritedTraits = decorateSymbol(sym).directlyInheritedTraits | ||
val directlyInheritedTraits = new SymUtilsOps(sym).directlyInheritedTraits | ||
val directlyInheritedTraitsSet = directlyInheritedTraits.toSet | ||
val allBaseClasses = directlyInheritedTraits.iterator.flatMap(_.symbol.asClass.baseClasses.drop(1)).toSet | ||
val superCalls = superCallsMap.getOrElse(sym, Set.empty) | ||
|
@@ -1193,8 +1195,8 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |
val arity = field.meth.tpe.widenDealias.paramTypes.size - _1.size | ||
val returnsUnit = field.meth.tpe.widenDealias.resultType.classSymbol == UnitClass | ||
if (returnsUnit) | ||
ctx.requiredClass(("scala.compat.java8.JProcedure" + arity).toTermName) | ||
else ctx.requiredClass(("scala.compat.java8.JFunction" + arity).toTermName) | ||
ctx.requiredClass("scala.compat.java8.JProcedure" + arity) | ||
else ctx.requiredClass("scala.compat.java8.JFunction" + arity) | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,7 +126,7 @@ object desugar { | |
val ValDef(name, tpt, rhs) = vdef | ||
val mods = vdef.mods | ||
val setterNeeded = | ||
(mods is Mutable) && ctx.owner.isClass && (!(mods is PrivateLocal) || (ctx.owner is Trait)) | ||
mods.is(Mutable) && ctx.owner.isClass && (!mods.isBoth(Private, and = Local) || ctx.owner.is(Trait)) | ||
if (setterNeeded) { | ||
// TODO: copy of vdef as getter needed? | ||
// val getter = ValDef(mods, name, tpt, rhs) withPos vdef.pos? | ||
|
@@ -149,7 +149,7 @@ object desugar { | |
|
||
def makeImplicitParameters(tpts: List[Tree], forPrimaryConstructor: Boolean = false)(implicit ctx: Context) = | ||
for (tpt <- tpts) yield { | ||
val paramFlags: FlagSet = if (forPrimaryConstructor) PrivateLocalParamAccessor else Param | ||
val paramFlags: FlagSet = if (forPrimaryConstructor) Private | Local | ParamAccessor else Param | ||
val epname = EvidenceParamName.fresh() | ||
ValDef(epname, tpt, EmptyTree).withFlags(paramFlags | Implicit) | ||
} | ||
|
@@ -260,9 +260,9 @@ object desugar { | |
@sharable private val synthetic = Modifiers(Synthetic) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this commit should not be merged. There is a reason for rawMods/modsDeco:
import, |
||
private def toDefParam(tparam: TypeDef): TypeDef = | ||
tparam.withMods(tparam.rawMods & EmptyFlags | Param) | ||
tparam.withMods(tparam.mods & EmptyFlags | Param) | ||
private def toDefParam(vparam: ValDef): ValDef = | ||
vparam.withMods(vparam.rawMods & (Implicit | Erased) | Param) | ||
vparam.withMods(vparam.mods & (Implicit | Erased) | Param) | ||
|
||
/** The expansion of a class definition. See inline comments for what is involved */ | ||
def classDef(cdef: TypeDef)(implicit ctx: Context): Tree = { | ||
|
@@ -314,7 +314,7 @@ object desugar { | |
lazy val derivedEnumParams = enumClass.typeParams.map(derivedTypeParam) | ||
val impliedTparams = | ||
if (isEnumCase && originalTparams.isEmpty) | ||
derivedEnumParams.map(tdef => tdef.withFlags(tdef.mods.flags | PrivateLocal)) | ||
derivedEnumParams.map(tdef => tdef.withFlags(tdef.mods.flags | Private | Local)) | ||
else | ||
originalTparams | ||
val constrTparams = impliedTparams.map(toDefParam) | ||
|
@@ -729,7 +729,7 @@ object desugar { | |
case _ => | ||
val tmpName = UniqueName.fresh() | ||
val patMods = | ||
mods & Lazy | Synthetic | (if (ctx.owner.isClass) PrivateLocal else EmptyFlags) | ||
mods & Lazy | Synthetic | (if (ctx.owner.isClass) Private | Local else EmptyFlags) | ||
val firstDef = | ||
ValDef(tmpName, TypeTree(), matchExpr) | ||
.withPos(pat.pos.union(rhs.pos)).withMods(patMods) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,12 @@ abstract class Positioned extends DotClass with Product { | |
|
||
setPos(initialPos) | ||
|
||
/** The item's position. | ||
*/ | ||
/** The item's position. */ | ||
def pos: Position = curPos | ||
|
||
/** The item's coord. */ | ||
def coord: Coord = curPos.toCoord | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is wrong. "Coord" is an optimization for Symbols, so that they can take alternatively a Position or an Index. It's wrong to pollute other data types with this distinction - they should have no idea what a |
||
|
||
/** Destructively update `curPos` to given position. Also, set any missing | ||
* positions in children. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,7 +78,7 @@ object Trees { | |
/** The type constructor at the root of the tree */ | ||
type ThisTree[T >: Untyped] <: Tree[T] | ||
|
||
private[this] var myTpe: T = _ | ||
protected var myTpe: T @uncheckedVariance = _ | ||
|
||
/** Destructively set the type of the tree. This should be called only when it is known that | ||
* it is safe under sharing to do so. One use-case is in the withType method below | ||
|
@@ -91,7 +91,7 @@ object Trees { | |
/** The type of the tree. In case of an untyped tree, | ||
* an UnAssignedTypeException is thrown. (Overridden by empty trees) | ||
*/ | ||
def tpe: T @uncheckedVariance = { | ||
final def tpe: T @uncheckedVariance = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This commit LGTM |
||
if (myTpe == null) | ||
throw new UnAssignedTypeException(this) | ||
myTpe | ||
|
@@ -310,7 +310,7 @@ object Trees { | |
|
||
private[this] var myMods: untpd.Modifiers = null | ||
|
||
private[dotc] def rawMods: untpd.Modifiers = | ||
def mods: untpd.Modifiers = | ||
if (myMods == null) untpd.EmptyModifiers else myMods | ||
|
||
def rawComment: Option[Comment] = getAttachment(DocComment) | ||
|
@@ -338,7 +338,7 @@ object Trees { | |
*/ | ||
def namePos = | ||
if (pos.exists) | ||
if (rawMods.is(Synthetic)) Position(pos.point, pos.point) | ||
if (mods.is(Synthetic)) Position(pos.point, pos.point) | ||
else Position(pos.point, pos.point + name.stripModuleClassSuffix.lastPart.length, pos.point) | ||
else pos | ||
} | ||
|
@@ -727,7 +727,6 @@ object Trees { | |
} | ||
|
||
trait WithoutTypeOrPos[-T >: Untyped] extends Tree[T] { | ||
override def tpe: T @uncheckedVariance = NoType.asInstanceOf[T] | ||
override def withTypeUnchecked(tpe: Type) = this.asInstanceOf[ThisTree[Type]] | ||
override def pos = NoPosition | ||
override def setPos(pos: Position) = {} | ||
|
@@ -740,6 +739,8 @@ object Trees { | |
*/ | ||
case class Thicket[-T >: Untyped](trees: List[Tree[T]]) | ||
extends Tree[T] with WithoutTypeOrPos[T] { | ||
myTpe = NoType.asInstanceOf[T] | ||
|
||
type ThisTree[-T >: Untyped] = Thicket[T] | ||
override def isEmpty: Boolean = trees.isEmpty | ||
override def toList: List[Tree[T]] = flatten(trees) | ||
|
@@ -758,8 +759,9 @@ object Trees { | |
|
||
class EmptyValDef[T >: Untyped] extends ValDef[T]( | ||
nme.WILDCARD, genericEmptyTree[T], genericEmptyTree[T]) with WithoutTypeOrPos[T] { | ||
myTpe = NoType.asInstanceOf[T] | ||
override def isEmpty: Boolean = true | ||
setMods(untpd.Modifiers(PrivateLocal)) | ||
setMods(untpd.Modifiers(Private | Local)) | ||
} | ||
|
||
@sharable val theEmptyTree: Thicket[Type] = Thicket(Nil) | ||
|
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.
This commit LGTM