Skip to content

Commit

Permalink
Simplify value class API handling and fix sbt/sbt#2497
Browse files Browse the repository at this point in the history 
The previous approach to value class API (introduced by sbt/sbt#2261 and
refined by sbt/sbt#2413 and sbt/sbt#2414) was to store both unerased and
erased signatures so that changes to value classes forced recompilations.
This is no longer necessary thanks to sbt#87: if a class type is
used, then it becomes a dependency of the current class and its name is
part of the used names of the current class. Since the name hash of a
class changes if it stops or start extending AnyVal, this is enough to
force recompilation of anything that uses a value class type. If the
underlying type of a value class change, its name hash doesn't change,
but the name hash of <init> change and since every class uses the name
<init>, we don't need to do anything special to trigger recompilations
either.
  • Loading branch information
@smarter
smarter committed Apr 9, 2016
1 parent 350afa7 commit 1e7e99e
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 157 deletions.
10 changes: 0 additions & 10 deletions internal/compiler-bridge/src-2.10/main/scala/xsbt/Compat.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,6 @@ abstract class Compat {
val Nullary = global.NullaryMethodType
val ScalaObjectClass = definitions.ScalaObjectClass

// `transformedType` doesn't exist in Scala < 2.10
implicit def withTransformedType(global: Global): WithTransformedType = new WithTransformedType(global)
class WithTransformedType(global: Global) {
def transformedType(tpe: Type): Type = tpe
}

private[this] final class MiscCompat {
// in 2.9, nme.LOCALCHILD was renamed to tpnme.LOCAL_CHILD
def tpnme = nme
Expand Down Expand Up @@ -105,10 +99,6 @@ abstract class Compat {
private class WithRootMirror(x: Any) {
def rootMirror: DummyMirror = new DummyMirror
}
lazy val AnyValClass = global.rootMirror.getClassIfDefined("scala.AnyVal")

def isDerivedValueClass(sym: Symbol): Boolean =
sym.isNonBottomSubClass(AnyValClass) && !definitions.ScalaValueClasses.contains(sym)
}

object MacroExpansionOf {
Expand Down
89 changes: 16 additions & 73 deletions internal/compiler-bridge/src-2.10/main/scala/xsbt/ExtractAPI.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,23 +217,9 @@ class ExtractAPI[GlobalType <: Global](

private def viewer(s: Symbol) = (if (s.isModule) s.moduleClass else s).thisType
private def printMember(label: String, in: Symbol, t: Type) = println(label + " in " + in + " : " + t + " (debug: " + debugString(t) + " )")
private def defDef(in: Symbol, s: Symbol): List[xsbti.api.Def] =
private def defDef(in: Symbol, s: Symbol): xsbti.api.Def =
{

val hasValueClassAsParameter: Boolean = {
s.asMethod.paramss.flatten map (_.info) exists (_.typeSymbol.isDerivedValueClass)
}

def hasValueClassAsReturnType(tpe: Type): Boolean = tpe match {
case PolyType(_, base) => hasValueClassAsReturnType(base)
case MethodType(_, resultType) => hasValueClassAsReturnType(resultType)
case NullaryMethodType(resultType) => hasValueClassAsReturnType(resultType)
case resultType => resultType.typeSymbol.isDerivedValueClass
}

val inspectPostErasure = hasValueClassAsParameter || hasValueClassAsReturnType(viewer(in).memberInfo(s))

def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): List[xsbti.api.Def] =
def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): xsbti.api.Def =
{
def parameterList(syms: List[Symbol], erase: Boolean = false): xsbti.api.ParameterList =
{
Expand All @@ -245,57 +231,14 @@ class ExtractAPI[GlobalType <: Global](
assert(typeParams.isEmpty)
assert(valueParameters.isEmpty)
build(base, typeParameters(in, typeParams0), Nil)
case mType @ MethodType(params, resultType) =>
// The types of a method's parameters change between phases: For instance, if a
// parameter is a subtype of AnyVal, then it won't have the same type before and after
// erasure. Therefore we record the type of parameters before AND after erasure to
// make sure that we don't miss some API changes.
// class A(val x: Int) extends AnyVal
// def foo(a: A): Int = A.x <- has type (LA)I before erasure
// <- has type (I)I after erasure
// If we change A from value class to normal class, we need to recompile all clients
// of def foo.
val beforeErasure =
build(resultType, typeParams, parameterList(params) :: valueParameters)
val afterErasure =
if (inspectPostErasure)
build(resultType, typeParams, parameterList(mType.params, erase = true) :: valueParameters)
else
Nil

beforeErasure ++ afterErasure
case MethodType(params, resultType) =>
build(resultType, typeParams, parameterList(params) :: valueParameters)
case NullaryMethodType(resultType) =>
build(resultType, typeParams, valueParameters)
case returnType =>
def makeDef(retTpe: xsbti.api.Type): xsbti.api.Def =
new xsbti.api.Def(
valueParameters.reverse.toArray,
retTpe,
typeParams,
simpleName(s),
getAccess(s),
getModifiers(s),
annotations(in, s)
)

// The return type of a method may change before and after erasure. Consider the
// following method:
// class A(val x: Int) extends AnyVal
// def foo(x: Int): A = new A(x) <- has type (I)LA before erasure
// <- has type (I)I after erasure
// If we change A from value class to normal class, we need to recompile all clients
// of def foo.
val beforeErasure = makeDef(processType(in, dropConst(returnType)))
val afterErasure =
if (inspectPostErasure) {
val erasedReturn = dropConst(global.transformedType(viewer(in).memberInfo(s))) map {
case MethodType(_, r) => r
case other => other
}
List(makeDef(processType(in, erasedReturn)))
} else Nil

beforeErasure :: afterErasure
val retType = processType(in, dropConst(returnType))
new xsbti.api.Def(valueParameters.reverse.toArray, retType, typeParams,
simpleName(s), getAccess(s), getModifiers(s), annotations(in, s))
}
}
def parameterS(erase: Boolean)(s: Symbol): xsbti.api.MethodParameter = {
Expand Down Expand Up @@ -420,22 +363,22 @@ class ExtractAPI[GlobalType <: Global](
defs
}

private def definition(in: Symbol, sym: Symbol): List[xsbti.api.Definition] =
private def definition(in: Symbol, sym: Symbol): Option[xsbti.api.Definition] =
{
def mkVar = List(fieldDef(in, sym, false, new xsbti.api.Var(_, _, _, _, _)))
def mkVal = List(fieldDef(in, sym, true, new xsbti.api.Val(_, _, _, _, _)))
def mkVar = Some(fieldDef(in, sym, false, new xsbti.api.Var(_, _, _, _, _)))
def mkVal = Some(fieldDef(in, sym, true, new xsbti.api.Val(_, _, _, _, _)))
if (isClass(sym))
if (ignoreClass(sym)) Nil else List(classLike(in, sym))
if (ignoreClass(sym)) None else Some(classLike(in, sym))
else if (sym.isNonClassType)
List(typeDef(in, sym))
Some(typeDef(in, sym))
else if (sym.isVariable)
if (isSourceField(sym)) mkVar else Nil
if (isSourceField(sym)) mkVar else None
else if (sym.isStable)
if (isSourceField(sym)) mkVal else Nil
if (isSourceField(sym)) mkVal else None
else if (sym.isSourceMethod && !sym.isSetter)
if (sym.isGetter) mkVar else defDef(in, sym)
if (sym.isGetter) mkVar else Some(defDef(in, sym))
else
Nil
None
}
private def ignoreClass(sym: Symbol): Boolean =
sym.isLocalClass || sym.isAnonymousClass || sym.fullName.endsWith(tpnme.LOCAL_CHILD.toString)
Expand Down
91 changes: 17 additions & 74 deletions internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,23 +217,9 @@ class ExtractAPI[GlobalType <: Global](

private def viewer(s: Symbol) = (if (s.isModule) s.moduleClass else s).thisType
private def printMember(label: String, in: Symbol, t: Type) = println(label + " in " + in + " : " + t + " (debug: " + debugString(t) + " )")
private def defDef(in: Symbol, s: Symbol): List[xsbti.api.Def] =
private def defDef(in: Symbol, s: Symbol): xsbti.api.Def =
{

val hasValueClassAsParameter: Boolean = {
s.asMethod.paramss.flatten map (_.info) exists (_.typeSymbol.isDerivedValueClass)
}

def hasValueClassAsReturnType(tpe: Type): Boolean = tpe match {
case PolyType(_, base) => hasValueClassAsReturnType(base)
case MethodType(_, resultType) => hasValueClassAsReturnType(resultType)
case NullaryMethodType(resultType) => hasValueClassAsReturnType(resultType)
case resultType => resultType.typeSymbol.isDerivedValueClass
}

val inspectPostErasure = hasValueClassAsParameter || hasValueClassAsReturnType(viewer(in).memberInfo(s))

def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): List[xsbti.api.Def] =
def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): xsbti.api.Def =
{
def parameterList(syms: List[Symbol], erase: Boolean = false): xsbti.api.ParameterList =
{
Expand All @@ -245,57 +231,14 @@ class ExtractAPI[GlobalType <: Global](
assert(typeParams.isEmpty)
assert(valueParameters.isEmpty)
build(base, typeParameters(in, typeParams0), Nil)
case mType @ MethodType(params, resultType) =>
// The types of a method's parameters change between phases: For instance, if a
// parameter is a subtype of AnyVal, then it won't have the same type before and after
// erasure. Therefore we record the type of parameters before AND after erasure to
// make sure that we don't miss some API changes.
// class A(val x: Int) extends AnyVal
// def foo(a: A): Int = A.x <- has type (LA)I before erasure
// <- has type (I)I after erasure
// If we change A from value class to normal class, we need to recompile all clients
// of def foo.
val beforeErasure =
build(resultType, typeParams, parameterList(params) :: valueParameters)
val afterErasure =
if (inspectPostErasure)
build(resultType, typeParams, parameterList(mType.params, erase = true) :: valueParameters)
else
Nil

beforeErasure ++ afterErasure
case MethodType(params, resultType) =>
build(resultType, typeParams, parameterList(params) :: valueParameters)
case NullaryMethodType(resultType) =>
build(resultType, typeParams, valueParameters)
case returnType =>
def makeDef(retTpe: xsbti.api.Type): xsbti.api.Def =
new xsbti.api.Def(
valueParameters.reverse.toArray,
retTpe,
typeParams,
simpleName(s),
getAccess(s),
getModifiers(s),
annotations(in, s)
)

// The return type of a method may change before and after erasure. Consider the
// following method:
// class A(val x: Int) extends AnyVal
// def foo(x: Int): A = new A(x) <- has type (I)LA before erasure
// <- has type (I)I after erasure
// If we change A from value class to normal class, we need to recompile all clients
// of def foo.
val beforeErasure = makeDef(processType(in, dropConst(returnType)))
val afterErasure =
if (inspectPostErasure) {
val erasedReturn = dropConst(global.transformedType(viewer(in).memberInfo(s))) map {
case MethodType(_, r) => r
case other => other
}
List(makeDef(processType(in, erasedReturn)))
} else Nil

beforeErasure :: afterErasure
val retType = processType(in, dropConst(returnType))
new xsbti.api.Def(valueParameters.reverse.toArray, retType, typeParams,
simpleName(s), getAccess(s), getModifiers(s), annotations(in, s))
}
}
def parameterS(erase: Boolean)(s: Symbol): xsbti.api.MethodParameter = {
Expand Down Expand Up @@ -420,22 +363,22 @@ class ExtractAPI[GlobalType <: Global](
defs
}

private def definition(in: Symbol, sym: Symbol): List[xsbti.api.Definition] =
private def definition(in: Symbol, sym: Symbol): Option[xsbti.api.Definition] =
{
def mkVar = List(fieldDef(in, sym, false, new xsbti.api.Var(_, _, _, _, _)))
def mkVal = List(fieldDef(in, sym, true, new xsbti.api.Val(_, _, _, _, _)))
def mkVar = Some(fieldDef(in, sym, false, new xsbti.api.Var(_, _, _, _, _)))
def mkVal = Some(fieldDef(in, sym, true, new xsbti.api.Val(_, _, _, _, _)))
if (isClass(sym))
if (ignoreClass(sym)) Nil else List(classLike(in, sym))
if (ignoreClass(sym)) None else Some(classLike(in, sym))
else if (sym.isNonClassType)
List(typeDef(in, sym))
Some(typeDef(in, sym))
else if (sym.isVariable)
if (isSourceField(sym)) mkVar else Nil
if (isSourceField(sym)) mkVar else None
else if (sym.isStable)
if (isSourceField(sym)) mkVal else Nil
if (isSourceField(sym)) mkVal else None
else if (sym.isSourceMethod && !sym.isSetter)
if (sym.isGetter) mkVar else defDef(in, sym)
if (sym.isGetter) mkVar else Some(defDef(in, sym))
else
Nil
None
}
private def ignoreClass(sym: Symbol): Boolean =
sym.isLocalClass || sym.isAnonymousClass || sym.fullName.endsWith(tpnme.LOCAL_CHILD.toString)
Expand Down Expand Up @@ -689,4 +632,4 @@ class ExtractAPI[GlobalType <: Global](
implicit def compat(ann: AnnotationInfo): IsStatic = new IsStatic(ann)
annotations.filter(_.isStatic)
}
}
}
1 change: 1 addition & 0 deletions zinc/src/sbt-test/source-dependencies/value-class-underlying/A.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class A(val x: Int) extends AnyVal
3 changes: 3 additions & 0 deletions zinc/src/sbt-test/source-dependencies/value-class-underlying/B.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object B {
def foo: A = new A(0)
}
5 changes: 5 additions & 0 deletions zinc/src/sbt-test/source-dependencies/value-class-underlying/C.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object C {
def main(args: Array[String]): Unit = {
val x = B.foo
}
}
1 change: 1 addition & 0 deletions zinc/src/sbt-test/source-dependencies/value-class-underlying/build.sbt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
logLevel := Level.Debug
1 change: 1 addition & 0 deletions zinc/src/sbt-test/source-dependencies/value-class-underlying/changes/A2.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class A(val x: Double) extends AnyVal
2 changes: 2 additions & 0 deletions zinc/src/sbt-test/source-dependencies/value-class-underlying/incOptions.properties
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
apiDebug = true
relationsDebug = true
3 changes: 3 additions & 0 deletions zinc/src/sbt-test/source-dependencies/value-class-underlying/test
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
> run
$ copy-file changes/A2.scala A.scala
> run

0 comments on commit 1e7e99e

Please sign in to comment.