From 1e7e99e7e19e1c45f5a52aa31c399bd33c007582 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 9 Apr 2016 20:13:47 +0200 Subject: [PATCH] Simplify value class API handling and fix sbt/sbt#2497 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 #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 change and since every class uses the name , we don't need to do anything special to trigger recompilations either. --- .../src-2.10/main/scala/xsbt/Compat.scala | 10 -- .../src-2.10/main/scala/xsbt/ExtractAPI.scala | 89 ++++-------------- .../src/main/scala/xsbt/ExtractAPI.scala | 91 ++++--------------- .../value-class-underlying/A.scala | 1 + .../value-class-underlying/B.scala | 3 + .../value-class-underlying/C.scala | 5 + .../value-class-underlying/build.sbt | 1 + .../value-class-underlying/changes/A2.scala | 1 + .../incOptions.properties | 2 + .../value-class-underlying/test | 3 + 10 files changed, 49 insertions(+), 157 deletions(-) create mode 100644 zinc/src/sbt-test/source-dependencies/value-class-underlying/A.scala create mode 100644 zinc/src/sbt-test/source-dependencies/value-class-underlying/B.scala create mode 100644 zinc/src/sbt-test/source-dependencies/value-class-underlying/C.scala create mode 100644 zinc/src/sbt-test/source-dependencies/value-class-underlying/build.sbt create mode 100644 zinc/src/sbt-test/source-dependencies/value-class-underlying/changes/A2.scala create mode 100644 zinc/src/sbt-test/source-dependencies/value-class-underlying/incOptions.properties create mode 100644 zinc/src/sbt-test/source-dependencies/value-class-underlying/test diff --git a/internal/compiler-bridge/src-2.10/main/scala/xsbt/Compat.scala b/internal/compiler-bridge/src-2.10/main/scala/xsbt/Compat.scala index a980628343..4ed9bef1ba 100644 --- a/internal/compiler-bridge/src-2.10/main/scala/xsbt/Compat.scala +++ b/internal/compiler-bridge/src-2.10/main/scala/xsbt/Compat.scala @@ -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 @@ -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 { diff --git a/internal/compiler-bridge/src-2.10/main/scala/xsbt/ExtractAPI.scala b/internal/compiler-bridge/src-2.10/main/scala/xsbt/ExtractAPI.scala index c4e3bb3067..dc0a455317 100644 --- a/internal/compiler-bridge/src-2.10/main/scala/xsbt/ExtractAPI.scala +++ b/internal/compiler-bridge/src-2.10/main/scala/xsbt/ExtractAPI.scala @@ -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 = { @@ -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 = { @@ -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) diff --git a/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala b/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala index 4a5346b70c..5fbfd2be01 100644 --- a/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala +++ b/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala @@ -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 = { @@ -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 = { @@ -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) @@ -689,4 +632,4 @@ class ExtractAPI[GlobalType <: Global]( implicit def compat(ann: AnnotationInfo): IsStatic = new IsStatic(ann) annotations.filter(_.isStatic) } -} \ No newline at end of file +} diff --git a/zinc/src/sbt-test/source-dependencies/value-class-underlying/A.scala b/zinc/src/sbt-test/source-dependencies/value-class-underlying/A.scala new file mode 100644 index 0000000000..dbaa1c3f0a --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/value-class-underlying/A.scala @@ -0,0 +1 @@ +class A(val x: Int) extends AnyVal diff --git a/zinc/src/sbt-test/source-dependencies/value-class-underlying/B.scala b/zinc/src/sbt-test/source-dependencies/value-class-underlying/B.scala new file mode 100644 index 0000000000..7d5a86a95f --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/value-class-underlying/B.scala @@ -0,0 +1,3 @@ +object B { + def foo: A = new A(0) +} diff --git a/zinc/src/sbt-test/source-dependencies/value-class-underlying/C.scala b/zinc/src/sbt-test/source-dependencies/value-class-underlying/C.scala new file mode 100644 index 0000000000..1a9a42bde9 --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/value-class-underlying/C.scala @@ -0,0 +1,5 @@ +object C { + def main(args: Array[String]): Unit = { + val x = B.foo + } +} diff --git a/zinc/src/sbt-test/source-dependencies/value-class-underlying/build.sbt b/zinc/src/sbt-test/source-dependencies/value-class-underlying/build.sbt new file mode 100644 index 0000000000..6448c246c2 --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/value-class-underlying/build.sbt @@ -0,0 +1 @@ +logLevel := Level.Debug diff --git a/zinc/src/sbt-test/source-dependencies/value-class-underlying/changes/A2.scala b/zinc/src/sbt-test/source-dependencies/value-class-underlying/changes/A2.scala new file mode 100644 index 0000000000..94d868a928 --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/value-class-underlying/changes/A2.scala @@ -0,0 +1 @@ +class A(val x: Double) extends AnyVal diff --git a/zinc/src/sbt-test/source-dependencies/value-class-underlying/incOptions.properties b/zinc/src/sbt-test/source-dependencies/value-class-underlying/incOptions.properties new file mode 100644 index 0000000000..17f05bf462 --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/value-class-underlying/incOptions.properties @@ -0,0 +1,2 @@ +apiDebug = true +relationsDebug = true diff --git a/zinc/src/sbt-test/source-dependencies/value-class-underlying/test b/zinc/src/sbt-test/source-dependencies/value-class-underlying/test new file mode 100644 index 0000000000..a42fd2d71c --- /dev/null +++ b/zinc/src/sbt-test/source-dependencies/value-class-underlying/test @@ -0,0 +1,3 @@ +> run +$ copy-file changes/A2.scala A.scala +> run