fix scala#11861, mix in nested inline def

Author: bishabosha

compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala

@@ -24,6 +24,7 @@ import java.io.PrintWriter
 import xsbti.api.DefinitionType
 
 import scala.collection.mutable
+import java.time.chrono.Era
 
 /** This phase sends a representation of the API of classes to sbt via callbacks.
  *
@@ -599,7 +600,7 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
       // an inline def in every class that extends its owner. To avoid this we
       // could store the hash as an annotation when pickling an inline def
       // and retrieve it here instead of computing it on the fly.
-      val inlineBodyHash = treeHash(inlineBody)
+      val inlineBodyHash = treeHash(inlineBody, inlineSym = s)
       annots += marker(inlineBodyHash.toString)
     }
 
@@ -620,27 +621,129 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
    *  it should stay the same across compiler runs, compiler instances,
    *  JVMs, etc.
    */
-  def treeHash(tree: Tree): Int =
+  def treeHash(tree: Tree, inlineSym: Symbol): Int =
     import scala.util.hashing.MurmurHash3
+    import core.Constants.*
+
+    val seenInlines = mutable.HashSet.empty[Symbol]
+
+    if inlineSym ne NoSymbol then
+      seenInlines += inlineSym // do not hash twice a recursive def
+
+    def nameHash(n: Name, initHash: Int): Int =
+      val h =
+        if n.isTermName then
+          MurmurHash3.mix(initHash, TermNameHash)
+        else
+          MurmurHash3.mix(initHash, TypeNameHash)
+
+      // The hashCode of the name itself is not stable across compiler instances
+      MurmurHash3.mix(h, n.toString.hashCode)
+    end nameHash
+
+    def typeHash(tp: Type, initHash: Int): Int =
+      // Go through `apiType` to get a value with a stable hash, it'd
+      // be better to use Murmur here too instead of relying on
+      // `hashCode`, but that would essentially mean duplicating
+      // https://github.com/sbt/zinc/blob/develop/internal/zinc-apiinfo/src/main/scala/xsbt/api/HashAPI.scala
+      // and at that point we might as well do type hashing on our own
+      // representation.
+      var h = initHash
+      tp match
+        case TypeBounds(lo, hi) =>
+          h = MurmurHash3.mix(h, apiType(lo).hashCode)
+          h = MurmurHash3.mix(h, apiType(hi).hashCode)
+        case tp: Type =>
+          h = MurmurHash3.mix(h, apiType(tp).hashCode)
+      h
+    end typeHash
+
+    def constantHash(c: Constant, initHash: Int): Int =
+      var h = MurmurHash3.mix(initHash, c.tag)
+      c.tag match
+        case NullTag =>
+          // No value to hash, the tag is enough.
+        case ClazzTag =>
+          h = typeHash(c.typeValue, h)
+        case _ =>
+          h = MurmurHash3.mix(h, c.value.hashCode)
+      h
+    end constantHash
+
+    /**An inline method that calls another inline method will eventually inline the call
+     * at a non-inline callsite, in this case if the implementation of the nested call
+     * changes, then the callsite will have a different API, we should hash the definition
+     */
+    def inlineDefHash(ref: Symbol, initHash: Int): Int =
+      var h = initHash
+
+      def paramssHash(paramss: List[List[Symbol]], initHash: Int): Int = paramss match
+        case Nil :: paramss1 =>
+          paramssHash(paramss1, MurmurHash3.mix(initHash, EmptyParamHash))
+        case params :: paramss1 =>
+          var h = initHash
+          val paramsIt = params.iterator
+          while paramsIt.hasNext do
+            val param = paramsIt.next
+            h = nameHash(param.name, h)
+            h = typeHash(param.info, h)
+            if param.is(Inline) then
+              h = MurmurHash3.mix(h, InlineParamHash) // inline would change the generated code
+            if param.isType then
+              val variance = param.paramVarianceSign
+              if variance != 0 then
+                h = MurmurHash3.mix(h, variance)
+          paramssHash(paramss1, h)
+        case Nil =>
+          initHash
+      end paramssHash
+
+      h = paramssHash(ref.paramSymss, h)
+      h = typeHash(ref.info.finalResultType, h)
+      positionedHash(Inliner.bodyToInline(ref), h)
+    end inlineDefHash
 
     def positionedHash(p: ast.Positioned, initHash: Int): Int =
+      var h = initHash
+
       p match
         case p: WithLazyField[?] =>
           p.forceIfLazy
         case _ =>
+
+      p match
+        case ref: RefTree @unchecked =>
+          val sym = ref.symbol
+          def err(): Int =
+            internalError(i"Don't know how to produce a stable hash for inline reference `$ref`", ref.sourcePos)
+            ref.name.toString.hashCode
+          if sym.is(Inline, butNot = Param) && !seenInlines.contains(sym) then
+            seenInlines += sym // dont re-enter hashing this ref
+            if sym.is(Method) then // inline def
+              if Inliner.hasBodyToInline(sym) then
+                h = inlineDefHash(sym, h)
+              else
+                h = err()
+            else // inline val
+              sym.info.widenTermRefExpr.dealias.normalized match
+                case ConstantType(c) =>
+                  h = constantHash(c, h)
+                case tpe =>
+                  h = err()
+        case _ =>
+
       // FIXME: If `p` is a tree we should probably take its type into account
       // when hashing it, but producing a stable hash for a type is not trivial
       // since the same type might have multiple representations, for method
       // signatures this is already handled by `computeType` and the machinery
       // in Zinc that generates hashes from that, if we can reliably produce
       // stable hashes for types ourselves then we could bypass all that and
       // send Zinc hashes directly.
-      val h = MurmurHash3.mix(initHash, p.productPrefix.hashCode)
+      h = MurmurHash3.mix(h, p.productPrefix.hashCode)
       iteratorHash(p.productIterator, h)
     end positionedHash
 
     def iteratorHash(it: Iterator[Any], initHash: Int): Int =
-      import core.Constants._
       var h = initHash
       while it.hasNext do
         it.next() match
@@ -649,24 +752,9 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
           case xs: List[?] =>
             h = iteratorHash(xs.iterator, h)
           case c: Constant =>
-            h = MurmurHash3.mix(h, c.tag)
-            c.tag match
-              case NullTag =>
-                // No value to hash, the tag is enough.
-              case ClazzTag =>
-                // Go through `apiType` to get a value with a stable hash, it'd
-                // be better to use Murmur here too instead of relying on
-                // `hashCode`, but that would essentially mean duplicating
-                // https://github.com/sbt/zinc/blob/develop/internal/zinc-apiinfo/src/main/scala/xsbt/api/HashAPI.scala
-                // and at that point we might as well do type hashing on our own
-                // representation.
-                val apiValue = apiType(c.typeValue)
-                h = MurmurHash3.mix(h, apiValue.hashCode)
-              case _ =>
-                h = MurmurHash3.mix(h, c.value.hashCode)
+            h = constantHash(c, h)
           case n: Name =>
-            // The hashCode of the name itself is not stable across compiler instances
-            h = MurmurHash3.mix(h, n.toString.hashCode)
+            h = nameHash(n, h)
           case elem =>
             internalError(
               i"Don't know how to produce a stable hash for `$elem` of unknown class ${elem.getClass}",
@@ -676,6 +764,14 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
       h
     end iteratorHash
 
+    def typeIteratorHash(it: Iterator[Type], initHash: Int): Int =
+      var h = initHash
+      while it.hasNext do
+        val apiValue = apiType(it.next) // see comment in constantHash for ClazzTag hashing
+        h = MurmurHash3.mix(h, apiValue.hashCode)
+      h
+    end typeIteratorHash
+
     val seed = 4 // https://xkcd.com/221
     val h = positionedHash(tree, seed)
     MurmurHash3.finalizeHash(h, 0)
@@ -691,6 +787,6 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
     // annotated @org.junit.Test).
     api.Annotation.of(
       apiType(annot.tree.tpe), // Used by sbt to find tests to run
-      Array(api.AnnotationArgument.of("TREE_HASH", treeHash(annot.tree).toString)))
+      Array(api.AnnotationArgument.of("TREE_HASH", treeHash(annot.tree, inlineSym = NoSymbol).toString)))
   }
 }

compiler/src/dotty/tools/dotc/sbt/package.scala

@@ -5,6 +5,11 @@ import dotty.tools.dotc.core.Symbols.Symbol
 import dotty.tools.dotc.core.NameOps.stripModuleClassSuffix
 import dotty.tools.dotc.core.Names.Name
 
+inline val TermNameHash = 1229 // 201st prime
+inline val TypeNameHash = 1231 // 202nd prime
+inline val EmptyParamHash = 1237 // 203rd prime
+inline val InlineParamHash = 1249 // 204th prime
+
 extension (sym: Symbol)
 
   def constructorName(using Context) =

sbt-test/source-dependencies/inline-rec-change-inline/A.scala

@@ -0,0 +1,5 @@
+object A {
+
+  inline def callInline: Any = B.inlinedAny("yyy")
+
+}

sbt-test/source-dependencies/inline-rec-change-inline/B.scala

@@ -0,0 +1,5 @@
+object B {
+
+  inline def inlinedAny(x: String): x.type = x
+
+}

sbt-test/source-dependencies/inline-rec-change-inline/C.scala

@@ -0,0 +1,3 @@
+class C {
+  val n = A.callInline
+}

sbt-test/source-dependencies/inline-rec-change-inline/build.sbt

@@ -0,0 +1,25 @@
+import sbt.internal.inc.Analysis
+import complete.DefaultParsers._
+
+// Reset compiler iterations, necessary because tests run in batch mode
+val recordPreviousIterations = taskKey[Unit]("Record previous iterations.")
+recordPreviousIterations := {
+  val log = streams.value.log
+  CompileState.previousIterations = {
+    val previousAnalysis = (previousCompile in Compile).value.analysis.asScala
+    previousAnalysis match {
+      case None =>
+        log.info("No previous analysis detected")
+        0
+      case Some(a: Analysis) => a.compilations.allCompilations.size
+    }
+  }
+}
+
+val checkIterations = inputKey[Unit]("Verifies the accumulated number of iterations of incremental compilation.")
+
+checkIterations := {
+  val expected: Int = (Space ~> NatBasic).parsed
+  val actual: Int = ((compile in Compile).value match { case a: Analysis => a.compilations.allCompilations.size }) - CompileState.previousIterations
+  assert(expected == actual, s"Expected $expected compilations, got $actual")
+}

sbt-test/source-dependencies/inline-rec-change-inline/changes/B1.scala

@@ -0,0 +1,5 @@
+object B {
+
+  inline def inlinedAny(inline x: String): x.type = x
+
+}

sbt-test/source-dependencies/inline-rec-change-inline/project/CompileState.scala

@@ -0,0 +1,4 @@
+// This is necessary because tests are run in batch mode
+object CompileState {
+  @volatile var previousIterations: Int = -1
+}

sbt-test/source-dependencies/inline-rec-change-inline/project/DottyInjectedPlugin.scala

@@ -0,0 +1,11 @@
+import sbt._
+import Keys._
+
+object DottyInjectedPlugin extends AutoPlugin {
+  override def requires = plugins.JvmPlugin
+  override def trigger = allRequirements
+
+  override val projectSettings = Seq(
+    scalaVersion := sys.props("plugin.scalaVersion")
+  )
+}

sbt-test/source-dependencies/inline-rec-change-inline/test

@@ -0,0 +1,9 @@
+> compile
+> recordPreviousIterations
+# Force recompilation of A because B.inlinedAny, called by A.callInline, has added
+# the inline flag to one of its parameters.
+$ copy-file changes/B1.scala B.scala
+> compile
+# 1 to recompile B, then 1 more to recompile A due to B.inlinedAny change,
+# then 1 final compilation to recompile C due to A.callInline change
+> checkIterations 3

sbt-test/source-dependencies/inline-rec-change-param/A.scala

@@ -0,0 +1,5 @@
+object A {
+
+  inline def callInline: Any = B.inlinedAny("yyy")
+
+}

sbt-test/source-dependencies/inline-rec-change-param/B.scala

@@ -0,0 +1,5 @@
+object B {
+
+  inline def inlinedAny(x: String): x.type = x
+
+}

sbt-test/source-dependencies/inline-rec-change-param/C.scala

@@ -0,0 +1,3 @@
+class C {
+  val n = A.callInline
+}

sbt-test/source-dependencies/inline-rec-change-param/build.sbt

@@ -0,0 +1,25 @@
+import sbt.internal.inc.Analysis
+import complete.DefaultParsers._
+
+// Reset compiler iterations, necessary because tests run in batch mode
+val recordPreviousIterations = taskKey[Unit]("Record previous iterations.")
+recordPreviousIterations := {
+  val log = streams.value.log
+  CompileState.previousIterations = {
+    val previousAnalysis = (previousCompile in Compile).value.analysis.asScala
+    previousAnalysis match {
+      case None =>
+        log.info("No previous analysis detected")
+        0
+      case Some(a: Analysis) => a.compilations.allCompilations.size
+    }
+  }
+}
+
+val checkIterations = inputKey[Unit]("Verifies the accumulated number of iterations of incremental compilation.")
+
+checkIterations := {
+  val expected: Int = (Space ~> NatBasic).parsed
+  val actual: Int = ((compile in Compile).value match { case a: Analysis => a.compilations.allCompilations.size }) - CompileState.previousIterations
+  assert(expected == actual, s"Expected $expected compilations, got $actual")
+}

sbt-test/source-dependencies/inline-rec-change-param/changes/B1.scala

@@ -0,0 +1,5 @@
+object B {
+
+  inline def inlinedAny(x: Any): x.type = x
+
+}

sbt-test/source-dependencies/inline-rec-change-param/project/CompileState.scala

@@ -0,0 +1,4 @@
+// This is necessary because tests are run in batch mode
+object CompileState {
+  @volatile var previousIterations: Int = -1
+}

sbt-test/source-dependencies/inline-rec-change-param/project/DottyInjectedPlugin.scala

@@ -0,0 +1,11 @@
+import sbt._
+import Keys._
+
+object DottyInjectedPlugin extends AutoPlugin {
+  override def requires = plugins.JvmPlugin
+  override def trigger = allRequirements
+
+  override val projectSettings = Seq(
+    scalaVersion := sys.props("plugin.scalaVersion")
+  )
+}

sbt-test/source-dependencies/inline-rec-change-param/test

@@ -0,0 +1,9 @@
+> compile
+> recordPreviousIterations
+# Force recompilation of A because B.inlinedAny, called by A.callInline, has changed
+# the type of its parameters.
+$ copy-file changes/B1.scala B.scala
+> compile
+# 1 to recompile B, then 1 more to recompile A due to B.inlinedAny change,
+# then 1 final compilation to recompile C due to A.callInline change
+> checkIterations 3

sbt-test/source-dependencies/inline-rec-change-res-transparent/A.scala

@@ -0,0 +1,5 @@
+object A {
+
+  inline def callInline: Any = B.inlinedAny("yyy")
+
+}

sbt-test/source-dependencies/inline-rec-change-res-transparent/B.scala

@@ -0,0 +1,5 @@
+object B {
+
+  transparent inline def inlinedAny(x: String): x.type = x
+
+}

sbt-test/source-dependencies/inline-rec-change-res-transparent/C.scala

@@ -0,0 +1,3 @@
+class C {
+  val n = A.callInline
+}