fix scala#11861, mix in nested inline def

Author: bishabosha

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

@@ -599,7 +599,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 +620,93 @@ 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
+
+    inline 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 =>
+          // 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
+    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 inlineRefHash(ref: Symbol, initHash: Int): Int =
+      var h = initHash
+      ref.defTree match // TODO: is this stable?
+        case tree: Trees.DefDef[?] =>
+          h = iteratorHash(tree.paramss.iterator, h)
+          h = positionedHash(tree.tpt, h)
+        case _ =>
+      positionedHash(Inliner.bodyToInline(ref), h)
+    end inlineRefHash
 
     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() =
+            internalError(i"Don't know how to produce a stable hash for inline reference `$ref`", ref.sourcePos)
+          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 = inlineRefHash(sym, h)
+              else
+                err()
+            else // inline val
+              sym.info.widenTermRefExpr.dealias.normalized match
+                case ConstantType(c) =>
+                  h = constantHash(c, h)
+                case _ =>
+                  err()
+        case _ =>
+
+      p match
+        case param: Trees.ValDef[?] if param.symbol.isAllOf(InlineParam) =>
+          h = MurmurHash3.mix(h, InlineParamHash)
+        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,21 +715,7 @@ 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)
@@ -691,6 +743,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,8 @@ import dotty.tools.dotc.core.Symbols.Symbol
 import dotty.tools.dotc.core.NameOps.stripModuleClassSuffix
 import dotty.tools.dotc.core.Names.Name
 
+inline val InlineParamHash = 1231 // aka true.##
+
 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 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-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
+}

sbt-test/source-dependencies/inline-rec-change-res-transparent/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-res-transparent/changes/B1.scala

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

sbt-test/source-dependencies/inline-rec-change-res-transparent/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-res-transparent/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-res-transparent/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/A.scala

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

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

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

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

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