Skip to content

Commit

Permalink
fix scala#11861, mix in nested inline def
Browse files Browse the repository at this point in the history
  • Loading branch information
bishabosha committed Jun 22, 2021
1 parent a0fb338 commit 540cb50
Show file tree
Hide file tree
Showing 46 changed files with 394 additions and 16 deletions.
48 changes: 44 additions & 4 deletions compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -620,22 +620,62 @@ 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

val seenInlines = mutable.HashSet.empty[Symbol]

if inlineSym ne NoSymbol then
seenInlines += inlineSym // do not hash twice a recursive def

def positionedHash(p: ast.Positioned, initHash: Int): Int =
var h = initHash

p match
case p: WithLazyField[?] =>
p.forceIfLazy
case _ =>

p match
case ref: Trees.RefTree[?] if Inliner.hasBodyToInline(ref.symbol) && !seenInlines.contains(ref.symbol) =>
// 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 mixParam(initHash: Int, param: Symbol): Int =
var h = initHash
h = MurmurHash3.mix(h, param.name.toString.hashCode)
param.defTree match // todo: is this stable?
case tree: Trees.ValOrDefDef[?] =>
h = positionedHash(tree.tpt, h)
case tree: Trees.TypeDef[?] =>
h = positionedHash(tree.rhs, h)
case _ =>
if param.isTerm && param.isAllOf(InlineParam) then
h = MurmurHash3.mix(h, InlineParamHash)
h
end mixParam

val inlined = ref.symbol

// dont re-enter hashing a symbol
seenInlines += inlined
// hash the body of the inline def
h = positionedHash(Inliner.bodyToInline(inlined), h)
// track when params are inline + their name/type
h = inlined.paramSymss.foldLeft(h) { (h, paramSyms) =>
paramSyms.foldLeft(h)(mixParam)
}
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

Expand Down Expand Up @@ -691,6 +731,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)))
}
}
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/sbt/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down
5 changes: 5 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-inline/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object A {

inline def callInline: Any = B.inlinedAny("yyy")

}
5 changes: 5 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-inline/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object B {

inline def inlinedAny(x: String): x.type = x

}
3 changes: 3 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-inline/C.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class C {
val n = A.callInline
}
25 changes: 25 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-inline/build.sbt
Original file line number Diff line number Diff line change
@@ -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")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object B {

inline def inlinedAny(inline x: String): x.type = x

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This is necessary because tests are run in batch mode
object CompileState {
@volatile var previousIterations: Int = -1
}
Original file line number Diff line number Diff line change
@@ -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")
)
}
9 changes: 9 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-inline/test
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-param/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object A {

inline def callInline: Any = B.inlinedAny("yyy")

}
5 changes: 5 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-param/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object B {

inline def inlinedAny(x: String): x.type = x

}
3 changes: 3 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-param/C.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class C {
val n = A.callInline
}
25 changes: 25 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-param/build.sbt
Original file line number Diff line number Diff line change
@@ -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")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object B {

inline def inlinedAny(x: Any): x.type = x

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This is necessary because tests are run in batch mode
object CompileState {
@volatile var previousIterations: Int = -1
}
Original file line number Diff line number Diff line change
@@ -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")
)
}
9 changes: 9 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-param/test
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-res/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object A {

inline def callInline: Any = B.inlinedAny("yyy")

}
5 changes: 5 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-res/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object B {

inline def inlinedAny(x: String): x.type = x

}
3 changes: 3 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-res/C.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class C {
val n = A.callInline
}
25 changes: 25 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-res/build.sbt
Original file line number Diff line number Diff line change
@@ -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")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object B {

inline def inlinedAny(x: String): String = x

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This is necessary because tests are run in batch mode
object CompileState {
@volatile var previousIterations: Int = -1
}
Original file line number Diff line number Diff line change
@@ -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")
)
}
9 changes: 9 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-res/test
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object A {

inline def callInline: Any = B.inlinedAny("yyy")

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object B {

inline def inlinedAny[T](x: T): x.type = x

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class C {
val n = A.callInline
}
25 changes: 25 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-typaram/build.sbt
Original file line number Diff line number Diff line change
@@ -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")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object B {

inline def inlinedAny[T <: String](x: T): x.type = x

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This is necessary because tests are run in batch mode
object CompileState {
@volatile var previousIterations: Int = -1
}
Original file line number Diff line number Diff line change
@@ -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")
)
}
9 changes: 9 additions & 0 deletions sbt-test/source-dependencies/inline-rec-change-typaram/test
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions sbt-test/source-dependencies/inline-rec-mut/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
object A {
inline def isEven(inline x: Int): Boolean = {
inline if x % 2 == 0 then
true
else
!B.isOdd(x)
}
}
Loading

0 comments on commit 540cb50

Please sign in to comment.