Fix overcompilation due to unstable context bound desugaring (#18280)

Context bounds are desugared into term parameters `evidence$N` and
before this
commit, the `N` was chosen to be unique in the current compilation unit.
This
isn't great because it means that adding a new definition with a context
bound
in the middle of a file would change the desugaring of subsequent
definitions
in the same file.

Even worse, when using incremental compilation we could end up with the
same
context bound desugared with a different value of `N` on different
compilation
runs because the order in which a compilation unit is traversed during
Typer is
not fixed but depends on the how the units that are jointly compiled
depend on
each other (as demonstrated by the `stable-ctx-bounds` test). This issue
affects all fresh names generated during Typer, but it is especially
problematic for context bounds because they're part of the API and
renaming
a method parameter forces the recompilation of all files calling that
method.

To fix this, we now only require context bounds parameters to have
unique names
among all the parameters of the method. This matches how we already
desugar
`def foo(using A, B)` into `def foo(using x$1: A, x$2: B)` regardless of
the
context.

Note that fresh names used in other situations are still problematic for
deterministic compilation. Most of the time they're not part of the API
checked
by Zinc, but they can still lead to overcompilation if they appear in an
`inline def` since the entire body of the `inline def` constitutes its
API. In
the future, we should follow Scala 2's lead and only require names to be
fresh
at the method level: scala/scala#6300 (The Scala
2
logic is slightly more complex to handle macros, but I don't think that
applies
to Scala 3 macros), see #7661.

Fixes #18080.

Author: smarter

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -7,7 +7,7 @@ import util.Spans._, Types._, Contexts._, Constants._, Names._, NameOps._, Flags
 import Symbols._, StdNames._, Trees._, ContextOps._
 import Decorators._, transform.SymUtils._
 import Annotations.Annotation
-import NameKinds.{UniqueName, EvidenceParamName, DefaultGetterName, WildcardParamName}
+import NameKinds.{UniqueName, ContextBoundParamName, ContextFunctionParamName, DefaultGetterName, WildcardParamName}
 import typer.{Namer, Checking}
 import util.{Property, SourceFile, SourcePosition, Chars}
 import config.Feature.{sourceVersion, migrateTo3, enabled}
@@ -203,10 +203,14 @@ object desugar {
     else vdef1
   end valDef
 
-  def makeImplicitParameters(tpts: List[Tree], implicitFlag: FlagSet, forPrimaryConstructor: Boolean = false)(using Context): List[ValDef] =
-    for (tpt <- tpts) yield {
+  def makeImplicitParameters(
+      tpts: List[Tree], implicitFlag: FlagSet,
+      mkParamName: () => TermName,
+      forPrimaryConstructor: Boolean = false
+  )(using Context): List[ValDef] =
+    for (tpt, i) <- tpts.zipWithIndex yield {
        val paramFlags: FlagSet = if (forPrimaryConstructor) LocalParamAccessor else Param
-       val epname = EvidenceParamName.fresh()
+       val epname = mkParamName()
        ValDef(epname, tpt, EmptyTree).withFlags(paramFlags | implicitFlag)
     }
 
@@ -240,17 +244,27 @@ object desugar {
     val DefDef(_, paramss, tpt, rhs) = meth
     val evidenceParamBuf = ListBuffer[ValDef]()
 
+    var seenContextBounds: Int = 0
     def desugarContextBounds(rhs: Tree): Tree = rhs match
       case ContextBounds(tbounds, cxbounds) =>
         val iflag = if sourceVersion.isAtLeast(`future`) then Given else Implicit
         evidenceParamBuf ++= makeImplicitParameters(
-          cxbounds, iflag, forPrimaryConstructor = isPrimaryConstructor)
+          cxbounds, iflag,
+          // Just like with `makeSyntheticParameter` on nameless parameters of
+          // using clauses, we only need names that are unique among the
+          // parameters of the method since shadowing does not affect
+          // implicit resolution in Scala 3.
+          mkParamName = () =>
+            val index = seenContextBounds + 1 // Start at 1 like FreshNameCreator.
+            val ret = ContextBoundParamName(EmptyTermName, index)
+            seenContextBounds += 1
+            ret,
+          forPrimaryConstructor = isPrimaryConstructor)
         tbounds
       case LambdaTypeTree(tparams, body) =>
         cpy.LambdaTypeTree(rhs)(tparams, desugarContextBounds(body))
       case _ =>
         rhs
-
     val paramssNoContextBounds =
       mapParamss(paramss) {
         tparam => cpy.TypeDef(tparam)(rhs = desugarContextBounds(tparam.rhs))
@@ -409,11 +423,11 @@ object desugar {
             meth.paramss :+ evidenceParams
         cpy.DefDef(meth)(paramss = paramss1)
 
-  /** The implicit evidence parameters of `meth`, as generated by `desugar.defDef` */
+  /** The parameters generated from the contextual bounds of `meth`, as generated by `desugar.defDef` */
   private def evidenceParams(meth: DefDef)(using Context): List[ValDef] =
     meth.paramss.reverse match {
       case ValDefs(vparams @ (vparam :: _)) :: _ if vparam.mods.isOneOf(GivenOrImplicit) =>
-        vparams.takeWhile(_.name.is(EvidenceParamName))
+        vparams.takeWhile(_.name.is(ContextBoundParamName))
       case _ =>
         Nil
     }
@@ -1588,7 +1602,7 @@ object desugar {
 
   def makeContextualFunction(formals: List[Tree], body: Tree, erasedParams: List[Boolean])(using Context): Function = {
     val mods = Given
-    val params = makeImplicitParameters(formals, mods)
+    val params = makeImplicitParameters(formals, mods, mkParamName = () => ContextFunctionParamName.fresh())
     FunctionWithMods(params, body, Modifiers(mods), erasedParams)
   }
 

compiler/src/dotty/tools/dotc/core/NameKinds.scala

@@ -278,9 +278,31 @@ object NameKinds {
       if (underlying.isEmpty) "$" + info.num + "$" else super.mkString(underlying, info)
   }
 
+  /** The name of the term parameter generated for a context bound:
+   *
+   *      def foo[T: A](...): ...
+   *
+   *  becomes:
+   *
+   *      def foo[T](...)(using evidence$1: A[T]): ...
+   *
+   *  The "evidence$" prefix is a convention copied from Scala 2.
+   */
+  val ContextBoundParamName: UniqueNameKind    = new UniqueNameKind("evidence$")
+
+  /** The name of an inferred contextual function parameter:
+   *
+   *      val x: A ?=> B = b
+   *
+   *  becomes:
+   *
+   *      val x: A ?=> B = (contextual$1: A) ?=> b
+   */
+  val ContextFunctionParamName: UniqueNameKind = new UniqueNameKind("contextual$")
+
   /** Other unique names */
+  val CanThrowEvidenceName: UniqueNameKind   = new UniqueNameKind("canThrow$")
   val TempResultName: UniqueNameKind         = new UniqueNameKind("ev$")
-  val EvidenceParamName: UniqueNameKind      = new UniqueNameKind("evidence$")
   val DepParamName: UniqueNameKind           = new UniqueNameKind("(param)")
   val LazyImplicitName: UniqueNameKind       = new UniqueNameKind("$_lazy_implicit_$")
   val LazyLocalName: UniqueNameKind          = new UniqueNameKind("$lzy")

compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala

@@ -216,7 +216,8 @@ object Scala3:
 
       def isEmptyNumbered: Boolean =
         !name.is(NameKinds.WildcardParamName)
-        && !name.is(NameKinds.EvidenceParamName)
+        && !name.is(NameKinds.ContextBoundParamName)
+        && !name.is(NameKinds.ContextFunctionParamName)
         && { name match
           case NameKinds.AnyNumberedName(nme.EMPTY, _) => true
           case _                                       => false

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

@@ -13,7 +13,7 @@ import Contexts._
 import Types._
 import Flags._
 import Mode.ImplicitsEnabled
-import NameKinds.{LazyImplicitName, EvidenceParamName}
+import NameKinds.{LazyImplicitName, ContextBoundParamName}
 import Symbols._
 import Types._
 import Decorators._
@@ -993,7 +993,7 @@ trait Implicits:
         def addendum = if (qt1 eq qt) "" else (i"\nWhere $qt is an alias of: $qt1")
         i"parameter of ${qual.tpe.widen}$addendum"
       case _ =>
-        i"${ if paramName.is(EvidenceParamName) then "an implicit parameter"
+        i"${ if paramName.is(ContextBoundParamName) then "a context parameter"
              else s"parameter $paramName" } of $methodStr"
     }
 

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

@@ -1734,8 +1734,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
           checkInInlineContext("summonFrom", tree.srcPos)
           val cases1 = tree.cases.mapconserve {
             case cdef @ CaseDef(pat @ Typed(Ident(nme.WILDCARD), _), _, _) =>
-              // case _ : T  -->  case evidence$n : T
-              cpy.CaseDef(cdef)(pat = untpd.Bind(EvidenceParamName.fresh(), pat))
+              // case _ : T  -->  case _$n : T
+              cpy.CaseDef(cdef)(pat = untpd.Bind(WildcardParamName.fresh(), pat))
             case cdef => cdef
           }
           typedMatchFinish(tree, tpd.EmptyTree, defn.ImplicitScrutineeTypeRef, cases1, pt)
@@ -2010,7 +2010,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
   def addCanThrowCapabilities(expr: untpd.Tree, cases: List[CaseDef])(using Context): untpd.Tree =
     def makeCanThrow(tp: Type): untpd.Tree =
       untpd.ValDef(
-          EvidenceParamName.fresh(),
+          CanThrowEvidenceName.fresh(),
           untpd.TypeTree(defn.CanThrowClass.typeRef.appliedTo(tp)),
           untpd.ref(defn.Compiletime_erasedValue))
         .withFlags(Given | Final | Erased)
@@ -3756,7 +3756,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
           else tree
         else if wtp.isContextualMethod then
           def isContextBoundParams = wtp.stripPoly match
-            case MethodType(EvidenceParamName(_) :: _) => true
+            case MethodType(ContextBoundParamName(_) :: _) => true
             case _ => false
           if sourceVersion == `future-migration` && isContextBoundParams && pt.args.nonEmpty
           then // Under future-migration, don't infer implicit arguments yet for parameters

compiler/src/dotty/tools/dotc/util/Signatures.scala

@@ -407,7 +407,7 @@ object Signatures {
       (params :: rest)
 
     def isSyntheticEvidence(name: String) =
-      if !name.startsWith(NameKinds.EvidenceParamName.separator) then false else
+      if !name.startsWith(NameKinds.ContextBoundParamName.separator) then false else
         symbol.paramSymss.flatten.find(_.name.show == name).exists(_.flags.is(Flags.Implicit))
 
     denot.info.stripPoly match

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

@@ -874,7 +874,7 @@ class DottyBytecodeTests extends DottyBytecodeTest {
     }
   }
 
-  @Test def freshNames = {
+  @Test def stableNames = {
     val sourceA =
       """|class A {
          |  def a1[T: Ordering]: Unit = {}
@@ -902,11 +902,11 @@ class DottyBytecodeTests extends DottyBytecodeTest {
           s"Method ${mn.name} has parameter $actualName but expected $expectedName")
       }
 
-      // The fresh name counter should be reset for every compilation unit
+      // Each definition should get the same names since there's no possible clashes.
       assertParamName(a1, "evidence$1")
-      assertParamName(a2, "evidence$2")
+      assertParamName(a2, "evidence$1")
       assertParamName(b1, "evidence$1")
-      assertParamName(b2, "evidence$2")
+      assertParamName(b2, "evidence$1")
     }
   }
 

presentation-compiler/src/main/dotty/tools/pc/completions/ScaladocCompletions.scala

@@ -112,7 +112,7 @@ object ScaladocCompletions:
           defdef.trailingParamss.flatten.collect {
             case param
                 if !param.symbol.isOneOf(Synthetic) &&
-                  !param.name.is(EvidenceParamName) &&
+                  !param.name.is(ContextBoundParamName) &&
                   param.symbol != extensionParam =>
               param.name.show
           }
@@ -121,7 +121,7 @@ object ScaladocCompletions:
             case param
                 if !param.is(Synthetic) &&
                   !param.isTypeParam &&
-                  !param.name.is(EvidenceParamName) =>
+                  !param.name.is(ContextBoundParamName) =>
               param.name.show
           }
         case other =>

presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala

@@ -9,7 +9,7 @@ import scala.meta.pc.SymbolSearch
 import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.core.Flags
 import dotty.tools.dotc.core.Flags.*
-import dotty.tools.dotc.core.NameKinds.EvidenceParamName
+import dotty.tools.dotc.core.NameKinds.ContextBoundParamName
 import dotty.tools.dotc.core.NameOps.*
 import dotty.tools.dotc.core.Names
 import dotty.tools.dotc.core.Names.Name
@@ -270,7 +270,7 @@ class ShortenedTypePrinter(
 
     lazy val implicitEvidenceParams: Set[Symbol] =
       implicitParams
-        .filter(p => p.name.toString.startsWith(EvidenceParamName.separator))
+        .filter(p => p.name.toString.startsWith(ContextBoundParamName.separator))
         .toSet
 
     lazy val implicitEvidencesByTypeParam: Map[Symbol, List[String]] =

sbt-test/source-dependencies/stable-ctx-bounds/A.scala

@@ -0,0 +1,5 @@
+package database
+
+object A {
+  def wrapper: B.Wrapper = ???
+}

sbt-test/source-dependencies/stable-ctx-bounds/B.scala

@@ -0,0 +1,29 @@
+package database
+
+object B {
+  trait GetValue[T]
+
+  object GetValue {
+    implicit def inst[T]: GetValue[T] = ???
+  }
+
+  class ResultSet {
+    def getV[A: GetValue]: A = ???
+  }
+
+  trait DBParse[T] {
+    def apply(rs: ResultSet): T
+  }
+
+  class AVG() {
+    def call: String = "AVG"
+  }
+
+  object ClientOwnerId {
+    class CompanyId
+
+    def parseClientOwnerId[T: DBParse]: Unit = {}
+  }
+
+  class Wrapper(companyId: ClientOwnerId.CompanyId)
+}

sbt-test/source-dependencies/stable-ctx-bounds/C.scala

@@ -0,0 +1,8 @@
+package database
+
+object C {
+  def foo: Unit = {
+    val rs: B.ResultSet = ???
+    rs.getV[String]
+  }
+}

sbt-test/source-dependencies/stable-ctx-bounds/build.sbt

@@ -0,0 +1,27 @@
+scalaVersion := sys.props("plugin.scalaVersion")
+
+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/stable-ctx-bounds/changes/B.scala

@@ -0,0 +1,27 @@
+package database
+
+object B {
+  trait GetValue[T]
+
+  object GetValue {
+    implicit def inst[T]: GetValue[T] = ???
+  }
+
+  class ResultSet {
+    def getV[A: GetValue]: A = ???
+  }
+
+  trait DBParse[T]
+
+  class AVG() {
+    def call: String = "AVG2"
+  }
+
+  object ClientOwnerId {
+    class CompanyId
+
+    def parseClientOwnerId[T: DBParse]: Unit = {}
+  }
+
+  class Wrapper(companyId: ClientOwnerId.CompanyId)
+}

sbt-test/source-dependencies/stable-ctx-bounds/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/stable-ctx-bounds/test

@@ -0,0 +1,15 @@
+> compile
+> recordPreviousIterations
+
+# change only the body of a method
+$ copy-file changes/B.scala B.scala
+
+# Only B.scala should be recompiled. Previously, this lead to a subsequent
+# compilation round because context bounds were desugared into names unique to
+# the whole compilation unit, and in the first `compile` the two context bounds
+# of B.scala were desugared into `evidence$2` and `evidence$1` in this order
+# (because the definitions were visited out of order), but in the second call
+# to `compile` we traverse them in order as we typecheck B.scala and ended up
+# with `evidence$1` and `evidence$2` instead.
+> compile
+> checkIterations 1

scaladoc/src/dotty/tools/scaladoc/tasty/ClassLikeSupport.scala

@@ -578,7 +578,7 @@ trait ClassLikeSupport:
     val baseTypeRepr = typeForClass(c).memberType(symbol)
 
     def isSyntheticEvidence(name: String) =
-      if !name.startsWith(NameKinds.EvidenceParamName.separator) then false else
+      if !name.startsWith(NameKinds.ContextBoundParamName.separator) then false else
         // This assumes that every parameter that starts with `evidence$` and is implicit is generated by compiler to desugar context bound.
         // Howrever, this is just a heuristic, so
         // `def foo[A](evidence$1: ClassTag[A]) = 1`

tests/neg/i10901.check

@@ -12,11 +12,11 @@
    |                [T1, T2]
    |                 (x: BugExp4Point2D.ColumnType[T1])
    |                   (y: BugExp4Point2D.ColumnType[T2])
-   |                     (implicit evidence$7: Numeric[T1], evidence$8: Numeric[T2]): BugExp4Point2D.Point2D[T1, T2]
+   |                     (implicit evidence$1: Numeric[T1], evidence$2: Numeric[T2]): BugExp4Point2D.Point2D[T1, T2]
    |                [T1, T2]
    |                 (x: T1)
    |                   (y: BugExp4Point2D.ColumnType[T2])
-   |                     (implicit evidence$5: Numeric[T1], evidence$6: Numeric[T2]): BugExp4Point2D.Point2D[T1, T2]
+   |                     (implicit evidence$1: Numeric[T1], evidence$2: Numeric[T2]): BugExp4Point2D.Point2D[T1, T2]
    |               both match arguments ((x : BugExp4Point2D.IntT.type))((y : BugExp4Point2D.DoubleT.type))
 -- [E008] Not Found Error: tests/neg/i10901.scala:48:38 ----------------------------------------------------------------
 48 |    val pos4: Point2D[Int,Double] = x º 201.1   // error
@@ -31,8 +31,8 @@
    |        Ambiguous overload. The overloaded alternatives of method º in object dsl with types
    |         [T1, T2]
    |          (x: BugExp4Point2D.ColumnType[T1])
-   |            (y: T2)(implicit evidence$9: Numeric[T1], evidence$10: Numeric[T2]): BugExp4Point2D.Point2D[T1, T2]
-   |         [T1, T2](x: T1)(y: T2)(implicit evidence$3: Numeric[T1], evidence$4: Numeric[T2]): BugExp4Point2D.Point2D[T1, T2]
+   |            (y: T2)(implicit evidence$1: Numeric[T1], evidence$2: Numeric[T2]): BugExp4Point2D.Point2D[T1, T2]
+   |         [T1, T2](x: T1)(y: T2)(implicit evidence$1: Numeric[T1], evidence$2: Numeric[T2]): BugExp4Point2D.Point2D[T1, T2]
    |        both match arguments ((x : BugExp4Point2D.IntT.type))((201.1d : Double))
 -- [E008] Not Found Error: tests/neg/i10901.scala:62:16 ----------------------------------------------------------------
 62 |  val y = "abc".foo  // error