Merge pull request #12931 from dotty-staging/fix-11861

Fix #11861 - hash nested calls to inline definitions

Author: bishabosha

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

@@ -5,6 +5,10 @@ import dotty.tools.dotc.core.Symbols.Symbol
 import dotty.tools.dotc.core.NameOps.stripModuleClassSuffix
 import dotty.tools.dotc.core.Names.Name
 
+inline val TermNameHash = 1987 // 300th prime
+inline val TypeNameHash = 1993 // 301st prime
+inline val InlineParamHash = 1997 // 302nd prime
+
 extension (sym: Symbol)
 
   def constructorName(using Context) =

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

@@ -980,7 +980,7 @@ trait Implicits:
     trace(s"search implicit ${pt.show}, arg = ${argument.show}: ${argument.tpe.show}", implicits, show = true) {
       record("inferImplicit")
       assert(ctx.phase.allowsImplicitSearch,
-        if (argument.isEmpty) i"missing implicit parameter of type $pt after typer"
+        if (argument.isEmpty) i"missing implicit parameter of type $pt after typer at phase ${ctx.phase.phaseName}"
         else i"type error: ${argument.tpe} does not conform to $pt${err.whyNoMatchStr(argument.tpe, pt)}")
 
       if pt.unusableForInference

compiler/src/dotty/tools/dotc/typer/Implicits.scala

@@ -188,4 +188,3 @@ def byName[T](x: => T): T = x
   */
 extension [T](x: T)
   transparent inline def asMatchable: x.type & Matchable = x.asInstanceOf[x.type & Matchable]
-

library/src/scala/compiletime/package.scala

@@ -96,8 +96,8 @@ IFS=':=' read -ra versionProps < "$cwd/dist/target/pack/VERSION" # temporarily s
   [ -n ${versionProps[2]} ] || die "Expected non-empty 'version' property in $cwd/dist/target/pack/VERSION"
 scala_version=${versionProps[2]}
 
-echo "testing -sourcepath with inlining"
-# Here we will test that an inline method symbol loaded from the sourcepath (-sourcepath compiler option)
+echo "testing -sourcepath with incremental compile: inlining changed inline def into a def"
+# Here we will test that a changed inline method symbol loaded from the sourcepath (-sourcepath compiler option)
 # will have its `defTree` correctly set when its method body is required for inlining.
 # So far I have not found a way to replicate issue https://github.com/lampepfl/dotty/issues/13994
 # with sbt scripted tests, if a way is found, move this test there.
@@ -107,3 +107,17 @@ sbt_test_command="++${scala_version}!;clean;prepareSources;compile;copyChanges;c
 rm -rf "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/target"
 rm -rf "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/target"
 rm -f "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/src/main/scala/a/zz.scala"
+
+echo "testing -sourcepath with incremental compile: hashing reference to changed inline def from an inline def"
+# Here we will test that a changed inline method symbol loaded from the sourcepath (-sourcepath compiler option)
+# will have its `defTree` correctly set when its method body is hashed by extractAPI, when referenced from another
+# inline method.
+# So far I have not found a way to replicate https://github.com/lampepfl/dotty/pull/12931#discussion_r753212124
+# with sbt scripted tests, if a way is found, move this test there.
+cwd=$(pwd)
+sbt_test_dir="$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash"
+sbt_test_command="++${scala_version}!;clean;prepareSources;compile;copyChanges;compile"
+(cd "$sbt_test_dir" && "$SBT" "$sbt_test_command")
+rm -rf "$sbt_test_dir/target"
+rm -rf "$sbt_test_dir/project/target"
+rm -f "$sbt_test_dir/src/main/scala/a/zz.scala"

project/scripts/bootstrapCmdTests

@@ -0,0 +1,7 @@
+object A {
+
+  class Box[T](val value: T)
+
+  extension (box: Box[Int]) def map[I <: Int](f: Int => I): Box[I] = new Box(f(box.value))
+
+}

sbt-test/source-dependencies/extension-change-second-tparams/A.scala

@@ -0,0 +1,6 @@
+import A.Box
+import A.map
+
+class B {
+  val n = Box(5).map(x => x * 3)
+}

sbt-test/source-dependencies/extension-change-second-tparams/B.scala

@@ -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/extension-change-second-tparams/build.sbt

@@ -0,0 +1,7 @@
+object A {
+
+  class Box[T](val value: T)
+
+  extension (box: Box[Int]) def map[I](f: Int => I): Box[I] = new Box(f(box.value))
+
+}

sbt-test/source-dependencies/extension-change-second-tparams/changes/A1.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/extension-change-second-tparams/project/CompileState.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/extension-change-second-tparams/project/DottyInjectedPlugin.scala

@@ -0,0 +1,7 @@
+> compile
+> recordPreviousIterations
+# Force recompilation of B because A.map, called by B.n, has changed
+$ copy-file changes/A1.scala A.scala
+> compile
+# 1 to recompile A, then 1 more to recompile B due to A.map change
+> checkIterations 2

sbt-test/source-dependencies/extension-change-second-tparams/test

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

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

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

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

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

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

@@ -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/build.sbt

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

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

@@ -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-inline/test

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

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

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

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

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

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

@@ -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/build.sbt

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

sbt-test/source-dependencies/inline-rec-change-param/changes/B1.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/CompileState.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/project/DottyInjectedPlugin.scala

@@ -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/test

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

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

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

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

@@ -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/build.sbt

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

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

@@ -0,0 +1,9 @@
+> compile
+> recordPreviousIterations
+# Force recompilation of A because B.inlinedAny, called by A.callInline, has changed
+# the type of its result, while being an inline method.
+$ 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/test

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

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

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

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

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

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

@@ -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")
+}