Skip to content

Catch binary-incompatible method overriding in traits #541

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

Merged
merged 1 commit into from
Sep 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions core/src/main/scala/com/typesafe/tools/mima/core/ClassInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,36 @@ private[mima] sealed abstract class ClassInfo(val owner: PackageInfo) extends In
case _: SyntheticClassInfo => false
case impl: ConcreteClassInfo =>
assert(impl.isImplClass, impl)
impl.methods.get(m.bytecodeName).exists(im => hasImplSig(im.descriptor, m.descriptor))
impl.methods.get(m.bytecodeName).exists { im =>
val isig = im.descriptor
val tsig = m.descriptor
assert(isig(0) == '(' && isig(1) == 'L' && tsig(0) == '(', s"isig=[$isig] tsig=[$tsig]")
hasMatchingSig(isig, tsig)
}
}
}

// Does `isig` correspond to `tsig` if seen as the signature of the static
// implementation method of a trait method with signature `tsig`?
private def hasImplSig(isig: String, tsig: String): Boolean = {
assert(isig(0) == '(' && isig(1) == 'L' && tsig(0) == '(')
val ilen = isig.length
/** Does the given method have a static mixin forwarder? */
final def hasMixinForwarder(m: MethodInfo): Boolean = {
methods.get(m.bytecodeName + "$").exists { fm =>
val fsig = fm.descriptor
val tsig = m.descriptor
assert(fsig(0) == '(' && tsig(0) == '(', s"fsig=[$fsig] tsig=[$tsig]")
hasMatchingSig(fsig, tsig)
}
}

// Does `sig` correspond to `tsig` if seen as the signature of the static
// implementation method or the mixin forwarder method of a trait method with signature `tsig`?
private def hasMatchingSig(sig: String, tsig: String): Boolean = {
val ilen = sig.length
val tlen = tsig.length
var i = 2
while (isig(i) != ';')
while (sig(i) != ';')
i += 1
i += 1
var j = 1
while (i < ilen && j < tlen && isig(i) == tsig(j)) {
while (i < ilen && j < tlen && sig(i) == tsig(j)) {
i += 1
j += 1
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ sealed abstract class Problem extends ProblemRef {
case DirectAbstractMethodProblem(ref) => s"${ref.methodString} does not have a correspondent in $affectedVersion version"
case ReversedAbstractMethodProblem(ref) => s"in $affectedVersion version there is ${ref.methodString}, which does not have a correspondent"
case UpdateForwarderBodyProblem(ref) => s"in $affectedVersion version, classes mixing ${ref.owner.fullName} needs to update body of ${ref.shortMethodString}"
case NewMixinForwarderProblem(ref) => s"in $affectedVersion version, classes mixing ${ref.owner.fullName} need be recompiled to wire to the new static mixin forwarder method all super calls to ${ref.shortMethodString}"
case InheritedNewAbstractMethodProblem(absmeth, ref) => s"${absmeth.methodString} is inherited by class ${ref.owner.bytecodeName} in $affectedVersion version."
}
}
Expand Down Expand Up @@ -84,4 +85,5 @@ sealed abstract class AbstractMethodProblem(memb: MemberInfo)
final case class DirectAbstractMethodProblem(newmeth: MethodInfo) extends AbstractMethodProblem(newmeth)
final case class ReversedAbstractMethodProblem(newmeth: MethodInfo) extends MemberProblem(newmeth)
final case class UpdateForwarderBodyProblem(oldmeth: MethodInfo) extends MemberProblem(oldmeth)
final case class NewMixinForwarderProblem(oldmeth: MethodInfo) extends MemberProblem(oldmeth)
final case class InheritedNewAbstractMethodProblem(absmeth: MethodInfo, newmeth: MethodInfo) extends AbstractMethodProblem(newmeth)
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ private[analyze] object MethodChecker {
} else {
if (oldmeth.owner.hasStaticImpl(oldmeth))
checkStaticImplMethod(oldmeth, newclazz)
else if (oldmeth.owner.hasMixinForwarder(oldmeth))
checkStaticMixinForwarderMethod(oldmeth, newclazz)
else
checkExisting1Impl(oldmeth, newclazz, _.lookupMethods(oldmeth))
}
Expand Down Expand Up @@ -79,6 +81,19 @@ private[analyze] object MethodChecker {
}
}

private def checkStaticMixinForwarderMethod(oldmeth: MethodInfo, newclazz: ClassInfo) = {
if (newclazz.hasMixinForwarder(oldmeth)) {
None // then it's ok, the method it is still there
} else {
if (newclazz.allTraits.exists(_.hasMixinForwarder(oldmeth))) {
Some(NewMixinForwarderProblem(oldmeth))
} else {
val methsLookup = (_: ClassInfo).lookupConcreteTraitMethods(oldmeth)
Some(missingOrIncompatible(oldmeth, methsLookup(newclazz).toList, methsLookup))
}
}
}

private def missingOrIncompatible(oldmeth: MethodInfo, newmeths: List[MethodInfo], methsLookup: ClassInfo => Iterator[MethodInfo]) = {
val newmethAndBridges = newmeths.filter(oldmeth.matchesType(_)) // _the_ corresponding new method + its bridges
newmethAndBridges.find(_.tpe.resultType == oldmeth.tpe.resultType) match {
Expand Down
1 change: 1 addition & 0 deletions functional-tests/src/it/scala-library-2-13/problems.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
in new version, classes mixing scala.collection.MapView need be recompiled to wire to the new static mixin forwarder method all super calls to method className()java.lang.String
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ object CollectProblemsTest {
() <- testCase.compileBoth
problems = collectProblems(direction.lhs(testCase).jfile, direction.rhs(testCase).jfile)
expected = readOracleFile(testCase.versionedFile(direction.oracleFile).jfile)
() <- diffProblems(problems, expected)
() <- direction match {
case Backwards => diffProblems(problems, expected)
case Forwards => diffProblems(problems, expected, "other")
}
} yield ()

val mimaLib: MiMaLib = new MiMaLib(cp = Nil)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object App {
def main(args: Array[String]): Unit = {
println(new B {}.n)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
in other version, classes mixing B need be recompiled to wire to the new static mixin forwarder method all super calls to method n()java.lang.String
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
trait A {
def m = "a"
def n = "a"
}

trait B extends A {
override def m = "b"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
trait A {
def m = "a"
def n = "a"
}

trait B extends A {
override def m = "b"
override def n = "b"
}
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
synthetic static method $init$(B)Unit in interface B does not have a correspondent in new version
in new version, classes mixing B need be recompiled to wire to the new static mixin forwarder method all super calls to method foo()Int
Original file line number Diff line number Diff line change
@@ -1 +1 @@
abstract method foo(Int)Int in interface A does not have a correspondent in new version
method foo(Int)Int in interface A does not have a correspondent in new version
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
synthetic static method $init$(B)Unit in interface B does not have a correspondent in new version
synthetic static method $init$(B)Unit in interface B does not have a correspondent in new version
in new version, classes mixing B need be recompiled to wire to the new static mixin forwarder method all super calls to method foo()Int