diff --git a/compile/interface/src/main/scala/xsbt/Compat.scala b/compile/interface/src/main/scala/xsbt/Compat.scala index 74116c0af..5b8c3983b 100644 --- a/compile/interface/src/main/scala/xsbt/Compat.scala +++ b/compile/interface/src/main/scala/xsbt/Compat.scala @@ -45,6 +45,17 @@ abstract class Compat { val Nullary = global.NullaryMethodType val ScalaObjectClass = definitions.ScalaObjectClass + // `afterPostErasure` doesn't exist in Scala < 2.10 + implicit def withAfterPostErasure(global: Global): WithAfterPostErasure = new WithAfterPostErasure(global) + class WithAfterPostErasure(global: Global) { + def afterPostErasure[T](op: => T): T = op + } + // `exitingPostErasure` was called `afterPostErasure` in 2.10 + implicit def withExitingPostErasure(global: Global): WithExitingPostErasure = new WithExitingPostErasure(global) + class WithExitingPostErasure(global: Global) { + def exitingPostErasure[T](op: => T): T = global afterPostErasure op + } + private[this] final class MiscCompat { // in 2.9, nme.LOCALCHILD was renamed to tpnme.LOCAL_CHILD def tpnme = nme @@ -75,6 +86,7 @@ abstract class Compat { def enclosingTopLevelClass: Symbol = sym.toplevelClass def toplevelClass: Symbol = sourceCompatibilityOnly + def asMethod: MethodSymbol = sym.asInstanceOf[MethodSymbol] } val DummyValue = 0 @@ -89,6 +101,21 @@ abstract class Compat { private[this] final implicit def miscCompat(n: AnyRef): MiscCompat = new MiscCompat + object MirrorHelper { + + private implicit def withRootMirror(x: Any): WithRootMirror = new WithRootMirror(x) + private class DummyMirror { + def getClassIfDefined(x: String): Symbol = NoSymbol + } + private class WithRootMirror(x: Any) { + def rootMirror: DummyMirror = new DummyMirror + } + lazy val AnyValClass = global.rootMirror.getClassIfDefined("scala.AnyVal") + + def isAnyValSubtype(sym: Symbol): Boolean = sym.isNonBottomSubClass(AnyValClass) + + } + object MacroExpansionOf { def unapply(tree: Tree): Option[Tree] = { diff --git a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala b/compile/interface/src/main/scala/xsbt/ExtractAPI.scala index 5c2cbf61b..7309aeeb9 100644 --- a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala +++ b/compile/interface/src/main/scala/xsbt/ExtractAPI.scala @@ -178,9 +178,32 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType, 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) = + + private def defDef(in: Symbol, s: Symbol): List[xsbti.api.Def] = { - def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): xsbti.api.Def = + import MirrorHelper._ + + val hasValueClassAsParameter: Boolean = { + import MirrorHelper._ + s.asMethod.paramss.flatten map (_.info) exists (t => isAnyValSubtype(t.typeSymbol)) + } + + // Note: We only inspect the "outermost type" (i.e. no recursion) because we don't need to + // inspect after erasure a function that would, for instance, return a function that returns + // a subtype of AnyVal. + val hasValueClassAsReturnType: Boolean = { + val tpe = viewer(in).memberInfo(s) + tpe match { + case PolyType(_, base) => isAnyValSubtype(base.typeSymbol) + case MethodType(_, resultType) => isAnyValSubtype(resultType.typeSymbol) + case Nullary(resultType) => isAnyValSubtype(resultType.typeSymbol) + case resultType => isAnyValSubtype(resultType.typeSymbol) + } + } + + val inspectPostErasure = hasValueClassAsParameter || hasValueClassAsReturnType + + def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): List[xsbti.api.Def] = { def parameterList(syms: List[Symbol]): xsbti.api.ParameterList = { @@ -192,13 +215,56 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType, assert(typeParams.isEmpty) assert(valueParameters.isEmpty) build(base, typeParameters(in, typeParams0), Nil) - case MethodType(params, resultType) => - build(resultType, typeParams, parameterList(params) :: valueParameters) + 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, global exitingPostErasure (parameterList(mType.params) :: valueParameters)) + else + Nil + + beforeErasure ++ afterErasure case Nullary(resultType) => // 2.9 and later build(resultType, typeParams, valueParameters) case returnType => - val t2 = processType(in, dropConst(returnType)) - new xsbti.api.Def(valueParameters.reverse.toArray, t2, typeParams, simpleName(s), getAccess(s), getModifiers(s), annotations(in, s)) + 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 exitingPostErasure viewer(in).memberInfo(s)) map { + case MethodType(_, r) => r + case other => other + } + List(makeDef(processType(in, erasedReturn))) + } else Nil + + beforeErasure :: afterErasure } } def parameterS(s: Symbol): xsbti.api.MethodParameter = @@ -292,22 +358,22 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType, defs } - private def definition(in: Symbol, sym: Symbol): Option[xsbti.api.Definition] = + private def definition(in: Symbol, sym: Symbol): List[xsbti.api.Definition] = { - def mkVar = Some(fieldDef(in, sym, false, new xsbti.api.Var(_, _, _, _, _))) - def mkVal = Some(fieldDef(in, sym, true, new xsbti.api.Val(_, _, _, _, _))) + def mkVar = List(fieldDef(in, sym, false, new xsbti.api.Var(_, _, _, _, _))) + def mkVal = List(fieldDef(in, sym, true, new xsbti.api.Val(_, _, _, _, _))) if (isClass(sym)) - if (ignoreClass(sym)) None else Some(classLike(in, sym)) + if (ignoreClass(sym)) Nil else List(classLike(in, sym)) else if (sym.isNonClassType) - Some(typeDef(in, sym)) + List(typeDef(in, sym)) else if (sym.isVariable) - if (isSourceField(sym)) mkVar else None + if (isSourceField(sym)) mkVar else Nil else if (sym.isStable) - if (isSourceField(sym)) mkVal else None + if (isSourceField(sym)) mkVal else Nil else if (sym.isSourceMethod && !sym.isSetter) - if (sym.isGetter) mkVar else Some(defDef(in, sym)) + if (sym.isGetter) mkVar else defDef(in, sym) else - None + Nil } private def ignoreClass(sym: Symbol): Boolean = sym.isLocalClass || sym.isAnonymousClass || sym.fullName.endsWith(LocalChild.toString) @@ -530,4 +596,4 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType, if (annots.isEmpty) processType(in, at.underlying) else annotated(in, annots, at.underlying) } -} \ No newline at end of file +} diff --git a/notes/0.13.10/consider-signatures-after-erasure.md b/notes/0.13.10/consider-signatures-after-erasure.md new file mode 100644 index 000000000..f511a5aba --- /dev/null +++ b/notes/0.13.10/consider-signatures-after-erasure.md @@ -0,0 +1,12 @@ + + [@Duhemm]: http://github.com/Duhemm + [1171]: https://github.com/sbt/sbt/issues/1171 + [2261]: https://github.com/sbt/sbt/pull/2261 + +### Fixes with compatibility implications + +### Improvements +- Register signatures of method before and after erasure if they involve value classes [#2261][2261] by [@Duhemm][@Duhemm] + +### Bug fixes +- Incremental compiler misses change to value class, and results to NoSuchMethodError at runtime [#1171][1171] \ No newline at end of file diff --git a/sbt/src/sbt-test/source-dependencies/value-class/build.sbt b/sbt/src/sbt-test/source-dependencies/value-class/build.sbt new file mode 100644 index 000000000..e850154ac --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/build.sbt @@ -0,0 +1,2 @@ +incOptions := incOptions.value.withRecompileAllFraction(1.0) + diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/A0.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/A0.scala new file mode 100644 index 000000000..ad5bf4c56 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/A0.scala @@ -0,0 +1 @@ +class A(val x: Int) diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/A1.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/A1.scala new file mode 100644 index 000000000..dbaa1c3f0 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/A1.scala @@ -0,0 +1 @@ +class A(val x: Int) extends AnyVal diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/B0.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/B0.scala new file mode 100644 index 000000000..0dba978c3 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/B0.scala @@ -0,0 +1,3 @@ +class B { + def foo(a: A): Int = 1 +} diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/B1.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/B1.scala new file mode 100644 index 000000000..c7b689c5e --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/B1.scala @@ -0,0 +1,3 @@ +class B { + def bar: A = new A(0) +} diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/C0.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/C0.scala new file mode 100644 index 000000000..c656b25a4 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/C0.scala @@ -0,0 +1,3 @@ +object C extends App { + println(new B().foo(null)) +} diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/C1.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/C1.scala new file mode 100644 index 000000000..ad8384312 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/C1.scala @@ -0,0 +1,3 @@ +object C extends App { + println(new B().bar.x) +} diff --git a/sbt/src/sbt-test/source-dependencies/value-class/test b/sbt/src/sbt-test/source-dependencies/value-class/test new file mode 100644 index 000000000..f78acb2d8 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/test @@ -0,0 +1,30 @@ +$ copy-file changes/A0.scala src/main/scala/A.scala +$ copy-file changes/B0.scala src/main/scala/B.scala +$ copy-file changes/C0.scala src/main/scala/C.scala + +# A is a normal class. B.foo accepts a parameter of type A. C calls B.foo, giving it `null`. +> compile +> run + +# Make A a value class. +$ copy-file changes/A1.scala src/main/scala/A.scala + +# The code no longer compiles because B.foo no longer accepts `null` as an argument. +# This means that we have invalidated C.scala, as expected! +-> compile + +$ copy-file changes/A0.scala src/main/scala/A.scala +$ copy-file changes/B1.scala src/main/scala/B.scala +$ copy-file changes/C1.scala src/main/scala/C.scala + +# A is a normal class. B.bar takes no arguments and returns an instance of A. C calls B.bar. +> compile +> run + +# Make A a value class. +$ copy-file changes/A1.scala src/main/scala/A.scala + +# The code compiles. It will run iff C is recompiled because the signature of B.bar has changed, +# because A is now a value class. +> run +