Merge pull request #9730 from dotty-staging/tree-hashing

Author: smarter

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

@@ -1,7 +1,8 @@
 package dotty.tools.dotc
 package sbt
 
-import ast.{Trees, tpd}
+import ExtractDependencies.internalError
+import ast.{Positioned, Trees, tpd, untpd}
 import core._
 import core.Decorators._
 import Annotations._
@@ -11,6 +12,7 @@ import Phases._
 import Trees._
 import Types._
 import Symbols._
+import Names._
 import NameOps._
 import NameKinds.DefaultGetterName
 import typer.Inliner
@@ -29,7 +31,7 @@ import scala.collection.mutable
  *
  *  See the documentation of `ExtractAPICollector`, `ExtractDependencies`,
  *  `ExtractDependenciesCollector` and
- *  http://www.scala-sbt.org/0.13/docs/Understanding-Recompilation.html for more
+ *  http://www.scala-sbt.org/1.x/docs/Understanding-Recompilation.html for more
  *  information on incremental recompilation.
  *
  *  The following flags affect this phase:
@@ -171,6 +173,7 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
   private val orMarker = marker("Or")
   private val byNameMarker = marker("ByName")
   private val matchMarker = marker("Match")
+  private val superMarker = marker("Super")
 
   /** Extract the API representation of a source file */
   def apiSource(tree: Tree): Seq[api.ClassLike] = {
@@ -513,8 +516,11 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
         apiType(tp.ref)
       case tp: TypeVar =>
         apiType(tp.underlying)
+      case SuperType(thistpe, supertpe) =>
+        val s = combineApiTypes(apiType(thistpe), apiType(supertpe))
+        withMarker(s, superMarker)
       case _ => {
-        report.warning(i"sbt-api: Unhandled type ${tp.getClass} : $tp")
+        internalError(i"Unhandled type $tp of class ${tp.getClass}")
         Constants.emptyType
       }
     }
@@ -582,13 +588,18 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
     val annots = new mutable.ListBuffer[api.Annotation]
     val inlineBody = Inliner.bodyToInline(s)
     if (!inlineBody.isEmpty) {
-      // FIXME: If the body of an inlineable method changes, all the reverse
-      // dependencies of this method need to be recompiled. sbt has no way
-      // of tracking method bodies, so as a hack we include the printed
-      // tree of the method as part of the signature we send to sbt.
-      // To do this properly we would need a way to hash trees and types in
-      // dotty itself.
-      annots += marker(inlineBody.toString)
+      // If the body of an inline def changes, all the reverse dependencies of
+      // this method need to be recompiled. sbt has no way of tracking method
+      // bodies, so we include the hash of the body of the method as part of the
+      // signature we send to sbt.
+      //
+      // FIXME: The API of a class we send to Zinc includes the signatures of
+      // inherited methods, which means that we repeatedly compute the hash of
+      // 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)
+      annots += marker(inlineBodyHash.toString)
     }
 
     // In the Scala2 ExtractAPI phase we only extract annotations that extend
@@ -602,16 +613,81 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
     annots.toList
   }
 
+  /** Produce a hash for a tree that is as stable as possible:
+   *  it should stay the same across compiler runs, compiler instances,
+   *  JVMs, etc.
+   */
+  def treeHash(tree: Tree): Int =
+    import scala.util.hashing.MurmurHash3
+
+    def positionedHash(p: ast.Positioned, initHash: Int): Int =
+      p match
+        case p: WithLazyField[?] =>
+          p.forceIfLazy
+        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)
+      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
+          case p: Positioned =>
+            h = positionedHash(p, h)
+          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)
+          case n: Name =>
+            // The hashCode of the name itself is not stable across compiler instances
+            h = MurmurHash3.mix(h, n.toString.hashCode)
+          case elem =>
+            internalError(
+              i"Don't know how to produce a stable hash for `$elem` of unknown class ${elem.getClass}",
+              tree.sourcePos)
+
+            h = MurmurHash3.mix(h, elem.toString.hashCode)
+      h
+    end iteratorHash
+
+    val seed = 4 // https://xkcd.com/221
+    val h = positionedHash(tree, seed)
+    MurmurHash3.finalizeHash(h, 0)
+  end treeHash
+
   def apiAnnotation(annot: Annotation): api.Annotation = {
-    // FIXME: To faithfully extract an API we should extract the annotation tree,
-    // sbt instead wants us to extract the annotation type and its arguments,
-    // to do this properly we would need a way to hash trees and types in dotty itself,
-    // instead we use the raw string representation of the annotation tree.
-    // However, we still need to extract the annotation type in the way sbt expect
-    // because sbt uses this information to find tests to run (for example
-    // junit tests are annotated @org.junit.Test).
+    // Like with inline defs, the whole body of the annotation and not just its
+    // type is part of its API so we need to store its hash, but Zinc wants us
+    // to extract the annotation type and its arguments, so we use a dummy
+    // annotation argument to store the hash of the tree. We still need to
+    // extract the annotation type in the way Zinc expects because sbt uses this
+    // information to find tests to run (for example junit tests are
+    // annotated @org.junit.Test).
     api.Annotation.of(
       apiType(annot.tree.tpe), // Used by sbt to find tests to run
-      Array(api.AnnotationArgument.of("FULLTREE", annot.tree.toString)))
+      Array(api.AnnotationArgument.of("TREE_HASH", treeHash(annot.tree).toString)))
   }
 }

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

@@ -16,6 +16,7 @@ import dotty.tools.dotc.core.StdNames._
 import dotty.tools.dotc.core.Symbols._
 import dotty.tools.dotc.core.Types._
 import dotty.tools.dotc.transform.SymUtils._
+import dotty.tools.dotc.util.{SrcPos, NoSourcePosition}
 import dotty.tools.io
 import dotty.tools.io.{AbstractFile, PlainFile, ZipArchive}
 import xsbti.UseScope
@@ -129,7 +130,7 @@ class ExtractDependencies extends Phase {
           if pf.file != null then
             binaryDependency(pf.file, binaryClassName)
         case _ =>
-          report.warning(s"sbt-deps: Ignoring dependency $depFile of class ${depFile.getClass}}")
+          internalError(s"Ignoring dependency $depFile of unknown class ${depFile.getClass}}", dep.from.srcPos)
       }
     }
 
@@ -171,6 +172,10 @@ class ExtractDependencies extends Phase {
 object ExtractDependencies {
   def classNameAsString(sym: Symbol)(using Context): String =
     sym.fullName.stripModuleClassSuffix.toString
+
+  /** Report an internal error in incremental compilation. */
+  def internalError(msg: => String, pos: SrcPos = NoSourcePosition)(using Context): Unit =
+    report.error(s"Internal error in the incremental compiler while compiling ${ctx.compilationUnit.source}: $msg", pos)
 }
 
 private case class ClassDependency(from: Symbol, to: Symbol, context: DependencyContext)

sbt-dotty/sbt-test/source-dependencies/inline-inherited/A.scala

@@ -0,0 +1,11 @@
+class A {
+  inline def getInline: String = {
+    class Local {
+      private val y: Int = 1
+    }
+    val a = scala.reflect.classTag[Integer]
+    println(new Local)
+    val x = 1
+    x.toString
+  }
+}

sbt-dotty/sbt-test/source-dependencies/inline-inherited/B.scala

@@ -0,0 +1 @@
+class B extends A

sbt-dotty/sbt-test/source-dependencies/inline-inherited/C.scala

@@ -0,0 +1,4 @@
+class C {
+  val b = new B
+  b.getInline
+}

sbt-dotty/sbt-test/source-dependencies/inline-inherited/build.sbt

@@ -0,0 +1,12 @@
+import complete.DefaultParsers._
+
+val checkIterations = inputKey[Unit]("Verifies the accumlated number of iterations of incremental compilation.")
+
+checkIterations := {
+  val analysis = (compile in Compile).value.asInstanceOf[sbt.internal.inc.Analysis]
+
+  val expected: Int = (Space ~> NatBasic).parsed
+  val actual: Int = analysis.compilations.allCompilations.size
+  assert(expected == actual, s"Expected $expected compilations, got $actual")
+}
+

sbt-dotty/sbt-test/source-dependencies/inline-inherited/changes/B1.scala

@@ -0,0 +1,3 @@
+class B extends A {
+  val unrelatedChange: Int = 1
+}

sbt-dotty/sbt-test/source-dependencies/inline-inherited/dbg.sbt

@@ -0,0 +1,3 @@
+logLevel := Level.Debug
+incOptions in ThisBuild ~= { _.withApiDebug(true) }
+incOptions in ThisBuild ~= { _.withRelationsDebug(true) }

sbt-dotty/sbt-test/source-dependencies/inline-inherited/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-dotty/sbt-test/source-dependencies/inline-inherited/project/plugins.sbt

@@ -0,0 +1 @@
+addSbtPlugin("ch.epfl.lamp" % "sbt-dotty" % sys.props("plugin.version"))

sbt-dotty/sbt-test/source-dependencies/inline-inherited/test

@@ -0,0 +1,6 @@
+> compile
+# Force recompilation of B, B.getInline hasn't changed so C shouldn't be recompiled.
+$ copy-file changes/B1.scala B.scala
+> compile
+# One iteration for each call to `compile`
+> checkIterations 2