Skip to content

Backport "Fix overcompilation due to unstable context bound desugaring" to LTS #19132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -202,10 +202,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)
}

Expand Down Expand Up @@ -239,17 +243,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))
Expand Down Expand Up @@ -380,11 +394,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
}
Expand Down Expand Up @@ -1500,7 +1514,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)
}

Expand Down
24 changes: 23 additions & 1 deletion compiler/src/dotty/tools/dotc/core/NameKinds.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/semanticdb/Scala3.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -975,7 +975,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"
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1686,8 +1686,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)
Expand Down Expand Up @@ -1962,7 +1962,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)
Expand Down Expand Up @@ -3686,7 +3686,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
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/util/Signatures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ class DottyBytecodeTests extends DottyBytecodeTest {
}
}

@Test def freshNames = {
@Test def stableNames = {
val sourceA =
"""|class A {
| def a1[T: Ordering]: Unit = {}
Expand Down Expand Up @@ -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")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]] =
Expand Down
5 changes: 5 additions & 0 deletions sbt-test/source-dependencies/stable-ctx-bounds/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package database

object A {
def wrapper: B.Wrapper = ???
}
29 changes: 29 additions & 0 deletions sbt-test/source-dependencies/stable-ctx-bounds/B.scala
Original file line number Diff line number Diff line change
@@ -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)
}
8 changes: 8 additions & 0 deletions sbt-test/source-dependencies/stable-ctx-bounds/C.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package database

object C {
def foo: Unit = {
val rs: B.ResultSet = ???
rs.getV[String]
}
}
27 changes: 27 additions & 0 deletions sbt-test/source-dependencies/stable-ctx-bounds/build.sbt
Original file line number Diff line number Diff line change
@@ -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")
}
27 changes: 27 additions & 0 deletions sbt-test/source-dependencies/stable-ctx-bounds/changes/B.scala
Original file line number Diff line number Diff line change
@@ -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)
}
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
}
15 changes: 15 additions & 0 deletions sbt-test/source-dependencies/stable-ctx-bounds/test
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
8 changes: 4 additions & 4 deletions tests/neg/i10901.check
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading