Tweaks after review

Author: odersky

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

@@ -226,29 +226,40 @@ object desugar {
   private def defDef(meth: DefDef, isPrimaryConstructor: Boolean = false)(using Context): Tree =
     addDefaultGetters(elimContextBounds(meth, isPrimaryConstructor))
 
+  /** Drop context bounds in given TypeDef, replacing them with evidence ValDefs that
+   *  get added to a buffer.
+   *  @param tdef          The given TypeDef
+   *  @param evidenceBuf   The buffer to which evidence gets added. This buffer
+   *                       is shared between desugarings of different type parameters
+   *                       of the same method.
+   *  @param evidenceFlags The flags to use for evidence definitions
+   *  @param freshName     A function to generate fresh names for evidence definitions
+   *  @param allParams     If `tdef` is a type paramter, all parameters of the owning method,
+   *                       otherwise the empty list.
+   */
   private def desugarContextBounds(
       tdef: TypeDef,
       evidenceBuf: mutable.ListBuffer[ValDef],
-      flags: FlagSet,
+      evidenceFlags: FlagSet,
       freshName: untpd.Tree => TermName,
       allParamss: List[ParamClause])(using Context): TypeDef =
 
     val evidenceNames = mutable.ListBuffer[TermName]()
 
     def desugarRhs(rhs: Tree): Tree = rhs match
       case ContextBounds(tbounds, cxbounds) =>
-        val isMember = flags.isAllOf(DeferredGivenFlags)
+        val isMember = evidenceFlags.isAllOf(DeferredGivenFlags)
         for bound <- cxbounds do
           val evidenceName = bound match
             case ContextBoundTypeTree(_, _, ownName) if !ownName.isEmpty =>
-              ownName
+              ownName // if there is an explicitly given name, use it.
             case _ if Config.nameSingleContextBounds && !isMember
                 && cxbounds.tail.isEmpty && Feature.enabled(Feature.modularity) =>
               tdef.name.toTermName
             case _ =>
               freshName(bound)
           evidenceNames += evidenceName
-          val evidenceParam = ValDef(evidenceName, bound, EmptyTree).withFlags(flags)
+          val evidenceParam = ValDef(evidenceName, bound, EmptyTree).withFlags(evidenceFlags)
           evidenceParam.pushAttachment(ContextBoundParam, ())
           evidenceBuf += evidenceParam
         tbounds
@@ -258,9 +269,13 @@ object desugar {
         rhs
 
     val tdef1 = cpy.TypeDef(tdef)(rhs = desugarRhs(tdef.rhs))
+    // Under x.modularity, if there was a context bound, and `tdef`s name as a term name is
+    // neither a name of an existing parameter nor a name of generated evidence for
+    // the same method, add a WitnessAnnotation with all generated evidence names to `tdef`.
+    // This means a context bound proxy will be created later.
     if Feature.enabled(Feature.modularity)
         && evidenceNames.nonEmpty
-        && !evidenceNames.contains(tdef.name.toTermName)
+        && !evidenceBuf.exists(_.name == tdef.name.toTermName)
         && !allParamss.nestedExists(_.name == tdef.name.toTermName)
     then
       tdef1.withAddedAnnotation:
@@ -332,9 +347,9 @@ object desugar {
 
     def getterParamss(n: Int): List[ParamClause] =
       mapParamss(takeUpTo(paramssNoRHS, n)) {
-        tparam => dropContextBounds(toDefParam(tparam, KeepAnnotations.All))
+        tparam => dropContextBounds(toMethParam(tparam, KeepAnnotations.All))
       } {
-        vparam => toDefParam(vparam, KeepAnnotations.All, keepDefault = false)
+        vparam => toMethParam(vparam, KeepAnnotations.All, keepDefault = false)
       }
 
     def defaultGetters(paramss: List[ParamClause], n: Int): List[DefDef] = paramss match
@@ -429,32 +444,30 @@ object desugar {
    *   The position of the added parameters is determined as follows:
    *
    *     - If there is an existing parameter list that refers to one of the added
-   *       parameters in one of its parameter types, add the new parameters
-   *       in front of the first such parameter list.
-   *     - Otherwise, if the last parameter list consists implicit or using parameters,
+   *       parameters or their future context bound proxies in one of its parameter
+   *       types, add the new parameters in front of the first such parameter list.
+   *     - Otherwise, if the last parameter list consists of implicit or using parameters,
    *       join the new parameters in front of this parameter list, creating one
-   *       parameter list (this is equilavent to Scala 2's scheme).
+   *       parameter list (this is equivalent to Scala 2's scheme).
    *     - Otherwise, add the new parameter list at the end as a separate parameter clause.
    */
   private def addEvidenceParams(meth: DefDef, params: List[ValDef])(using Context): DefDef =
     if params.isEmpty then return meth
 
-    var boundNames = params.map(_.name).toSet
+    var boundNames = params.map(_.name).toSet // all evidence parameter + context bound proxy names
     for mparams <- meth.paramss; mparam <- mparams do
       mparam match
         case tparam: TypeDef if tparam.mods.annotations.exists(WitnessNamesAnnot.unapply(_).isDefined) =>
           boundNames += tparam.name.toTermName
         case _ =>
 
-    //println(i"add ev params ${meth.name}, ${boundNames.toList}")
-
-    def references(vdef: ValDef): Boolean =
+    def referencesBoundName(vdef: ValDef): Boolean =
       vdef.tpt.existsSubTree:
         case Ident(name: TermName) => boundNames.contains(name)
         case _ => false
 
     def recur(mparamss: List[ParamClause]): List[ParamClause] = mparamss match
-      case ValDefs(mparams) :: _ if mparams.exists(references) =>
+      case ValDefs(mparams) :: _ if mparams.exists(referencesBoundName) =>
         params :: mparamss
       case ValDefs(mparams @ (mparam :: _)) :: Nil if mparam.mods.isOneOf(GivenOrImplicit) =>
         (params ++ mparams) :: Nil
@@ -468,12 +481,12 @@ object desugar {
 
   /** 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(_.hasAttachment(ContextBoundParam))
-      case _ =>
-        Nil
-    }
+    for
+      case ValDefs(vparams @ (vparam :: _)) <- meth.paramss
+      if vparam.mods.isOneOf(GivenOrImplicit)
+      param <- vparams.takeWhile(_.hasAttachment(ContextBoundParam))
+    yield
+      param
 
   @sharable private val synthetic = Modifiers(Synthetic)
 
@@ -491,11 +504,13 @@ object desugar {
           case WitnessNamesAnnot(_) => true
           case _ => false
 
-  private def toDefParam(tparam: TypeDef, keep: KeepAnnotations)(using Context): TypeDef =
+  /** Map type parameter accessor to corresponding method (i.e. constructor) parameter */
+  private def toMethParam(tparam: TypeDef, keep: KeepAnnotations)(using Context): TypeDef =
     val mods = filterAnnots(tparam.rawMods, keep)
     tparam.withMods(mods & EmptyFlags | Param)
 
-  private def toDefParam(vparam: ValDef, keep: KeepAnnotations, keepDefault: Boolean)(using Context): ValDef = {
+  /** Map term parameter accessor to corresponding method (i.e. constructor) parameter */
+  private def toMethParam(vparam: ValDef, keep: KeepAnnotations, keepDefault: Boolean)(using Context): ValDef = {
     val mods = filterAnnots(vparam.rawMods, keep)
     val hasDefault = if keepDefault then HasDefault else EmptyFlags
     // Need to ensure that tree is duplicated since term parameters can be watched
@@ -507,22 +522,16 @@ object desugar {
       .withMods(mods & (GivenOrImplicit | Erased | hasDefault | Tracked) | Param)
   }
 
-  def mkApply(fn: Tree, paramss: List[ParamClause])(using Context): Tree =
-    paramss.foldLeft(fn) { (fn, params) => params match
-      case TypeDefs(params) =>
-        TypeApply(fn, params.map(refOfDef))
-      case (vparam: ValDef) :: _ if vparam.mods.is(Given) =>
-        Apply(fn, params.map(refOfDef)).setApplyKind(ApplyKind.Using)
-      case _ =>
-        Apply(fn, params.map(refOfDef))
-    }
-
+  /** Desugar type def (not param): Under x.moduliity this can expand
+   *  context bounds, which are expanded to evidence ValDefs. These will
+   *  ultimately map to deferred givens.
+   */
   def typeDef(tdef: TypeDef)(using Context): Tree =
     val evidenceBuf = new mutable.ListBuffer[ValDef]
     val result = desugarContextBounds(
         tdef, evidenceBuf,
         (tdef.mods.flags.toTermFlags & AccessFlags) | Lazy | DeferredGivenFlags,
-        inventGivenOrExtensionName, Nil)
+        inventGivenName, Nil)
     if evidenceBuf.isEmpty then result else Thicket(result :: evidenceBuf.toList)
 
   /** The expansion of a class definition. See inline comments for what is involved */
@@ -597,7 +606,7 @@ object desugar {
     // Annotations on class _type_ parameters are set on the derived parameters
     // but not on the constructor parameters. The reverse is true for
     // annotations on class _value_ parameters.
-    val constrTparams = impliedTparams.map(toDefParam(_, KeepAnnotations.WitnessOnly))
+    val constrTparams = impliedTparams.map(toMethParam(_, KeepAnnotations.WitnessOnly))
     val constrVparamss =
       if (originalVparamss.isEmpty) { // ensure parameter list is non-empty
         if (isCaseClass)
@@ -608,7 +617,7 @@ object desugar {
         report.error(CaseClassMissingNonImplicitParamList(cdef), namePos)
         ListOfNil
       }
-      else originalVparamss.nestedMap(toDefParam(_, KeepAnnotations.All, keepDefault = true))
+      else originalVparamss.nestedMap(toMethParam(_, KeepAnnotations.All, keepDefault = true))
     val derivedTparams =
       constrTparams.zipWithConserve(impliedTparams)((tparam, impliedParam) =>
         derivedTypeParam(tparam).withAnnotations(impliedParam.mods.annotations))
@@ -630,7 +639,7 @@ object desugar {
             defDef(
               addEvidenceParams(
                 cpy.DefDef(ddef)(paramss = joinParams(constrTparams, ddef.paramss)),
-                evidenceParams(constr1).map(toDefParam(_, KeepAnnotations.None, keepDefault = false)))))
+                evidenceParams(constr1).map(toMethParam(_, KeepAnnotations.None, keepDefault = false)))))
         case stat =>
           stat
       }
@@ -1148,7 +1157,7 @@ object desugar {
    */
   def normalizeName(mdef: MemberDef, impl: Tree)(using Context): Name = {
     var name = mdef.name
-    if (name.isEmpty) name = name.likeSpaced(inventGivenOrExtensionName(impl))
+    if (name.isEmpty) name = name.likeSpaced(inventGivenName(impl))
     def errPos = mdef.source.atSpan(mdef.nameSpan)
     if (ctx.owner == defn.ScalaPackageClass && defn.reservedScalaClassNames.contains(name.toTypeName)) {
       val kind = if (name.isTypeName) "class" else "object"
@@ -1195,7 +1204,7 @@ object desugar {
   end makePolyFunctionType
 
   /** Invent a name for an anonympus given of type or template `impl`. */
-  def inventGivenOrExtensionName(impl: Tree)(using Context): SimpleName =
+  def inventGivenName(impl: Tree)(using Context): SimpleName =
     val str = impl match
       case impl: Template =>
         if impl.parents.isEmpty then
@@ -1207,6 +1216,10 @@ object desugar {
         "given_" ++ inventTypeName(impl)
     str.toTermName.asSimpleName
 
+  /** Extract a synthesized given name from a type tree. This is used for
+   *  both anonymous givens and (under x.modularity) deferred givens.
+   *  @param followArgs   If true include argument types in the name
+   */
   private class NameExtractor(followArgs: Boolean) extends UntypedTreeAccumulator[String] {
     private def extractArgs(args: List[Tree])(using Context): String =
       args.map(argNameExtractor.apply("", _)).mkString("_")

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

@@ -382,15 +382,15 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] =>
     case _ =>
       tree.tpe.isInstanceOf[ThisType]
   }
-  
-  /** Extractor for annotation.internal.WitnessNames(name_1, ..., name_n)`
+
+  /** Under x.modularity: Extractor for `annotation.internal.WitnessNames(name_1, ..., name_n)`
    *  represented as an untyped or typed tree.
    */
   object WitnessNamesAnnot:
-    def apply(names0: List[TermName])(using Context): untpd.Tree =
+    def apply(names: List[TermName])(using Context): untpd.Tree =
       untpd.TypedSplice(tpd.New(
           defn.WitnessNamesAnnot.typeRef,
-          tpd.SeqLiteral(names0.map(n => tpd.Literal(Constant(n.toString))), tpd.TypeTree(defn.StringType)) :: Nil
+          tpd.SeqLiteral(names.map(n => tpd.Literal(Constant(n.toString))), tpd.TypeTree(defn.StringType)) :: Nil
         ))
 
     def unapply(tree: Tree)(using Context): Option[List[TermName]] =

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

@@ -119,6 +119,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
   case class PatDef(mods: Modifiers, pats: List[Tree], tpt: Tree, rhs: Tree)(implicit @constructorOnly src: SourceFile) extends DefTree
   case class ExtMethods(paramss: List[ParamClause], methods: List[Tree])(implicit @constructorOnly src: SourceFile) extends Tree
   case class ContextBoundTypeTree(tycon: Tree, paramName: TypeName, ownName: TermName)(implicit @constructorOnly src: SourceFile) extends Tree
+    // `paramName: tycon as ownName`, ownName != EmptyTermName only under x.modularity
   case class MacroTree(expr: Tree)(implicit @constructorOnly src: SourceFile) extends Tree
 
   case class ImportSelector(imported: Ident, renamed: Tree = EmptyTree, bound: Tree = EmptyTree)(implicit @constructorOnly src: SourceFile) extends Tree {

compiler/src/dotty/tools/dotc/config/Config.scala

@@ -236,7 +236,8 @@ object Config {
   inline val checkLevelsOnConstraints = false
   inline val checkLevelsOnInstantiation = true
 
-  /** If a type parameter `X` has a single context bound `X: C`, should the
+  /** Under x.modularity:
+   *  If a type parameter `X` has a single context bound `X: C`, should the
    *  witness parameter be named `X`? This would prevent the creation of a
    *  context bound companion.
    */

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

@@ -104,8 +104,8 @@ object Mode {
   val CheckBoundsOrSelfType: Mode = newMode(14, "CheckBoundsOrSelfType")
 
   /** Use previous Scheme for implicit resolution. Currently significant
-   *  in 3.0-migration where we use Scala-2's scheme instead and in 3.5-migration
-   *  where we use the previous scheme up to 3.4 instead.
+   *  in 3.0-migration where we use Scala-2's scheme instead and in 3.5 and 3.6-migration
+   *  where we use the previous scheme up to 3.4 for comparison with the new scheme. 
    */
   val OldImplicitResolution: Mode = newMode(15, "OldImplicitResolution")
 

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

@@ -24,9 +24,9 @@ object NamerOps:
         addParamRefinements(ctor.owner.typeRef, paramss)
 
   /** Given a method with tracked term-parameters `p1, ..., pn`, and result type `R`, add the
-   *  refinements R { p1 = p1' } ... { pn = pn' }, where pi' is the term parameter ref
+   *  refinements R { p1 = p1' } ... { pn = pn' }, where pi' is the TermParamRef
    *  of the parameter and pi is its name. This matters only under experimental.modularity,
-   *  since wothout it there are no tracked parameters. Parameter refinements are added for
+   *  since without it there are no tracked parameters. Parameter refinements are added for
    *  constructors and given companion methods.
    */
   def addParamRefinements(resType: Type, paramss: List[List[Symbol]])(using Context): Type =
@@ -261,7 +261,7 @@ object NamerOps:
   /** Create a context-bound companion for type symbol `tsym`, which has a context
    *  bound that defines a set of witnesses with names `witnessNames`.
    *
-   *  @param parans  If `tsym` is a type parameter, a list of parameter symbols
+   *  @param params  If `tsym` is a type parameter, a list of parameter symbols
    *                 that include all witnesses, otherwise the empty list.
    *
    *  The context-bound companion has as name the name of `tsym` translated to
@@ -299,7 +299,7 @@ object NamerOps:
    *  this class. This assumes that these types already have their
    *  WitnessNames annotation set even before they are completed. This is
    *  the case for unpickling but currently not for Namer. So the method
-   *  is only called during unpickling, and is not part of NamerOps.
+   *  is only called during unpickling.
    */
   def addContextBoundCompanions(cls: ClassSymbol)(using Context): Unit =
     for sym <- cls.info.decls do

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

@@ -1194,7 +1194,7 @@ object SymDenotations {
       || is(JavaDefinedVal, butNot = Method)
       || isConstructor
       || !owner.isExtensibleClass && !is(Deferred)
-      	// Deferred symbols can arise through parent refinements.
+      	// Deferred symbols can arise through parent refinements under x.modularity.
       	// For them, the overriding relationship reverses anyway, so
       	// being in a final class does not mean the symbol cannot be
       	// implemented concretely in a superclass.

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

@@ -1655,7 +1655,7 @@ object Types extends TypeUtils {
      *
      *    P { ... type T = / += / -= U ... } # T
      *
-     *  to just U. Analogously, `P { val x: S} # x` is reduced tp `S` is `S`
+     *  to just U. Analogously, `P { val x: S} # x` is reduced tp `S` if `S`
      *  is a singleton type.
      *
      *  Does not perform the reduction if the resulting type would contain
@@ -4936,6 +4936,7 @@ object Types extends TypeUtils {
    *  @param  origin           the parameter that's tracked by the type variable.
    *  @param  creatorState     the typer state in which the variable was created.
    *  @param  initNestingLevel the initial nesting level of the type variable. (c.f. nestingLevel)
+   *  @param  precise          whether we should use instantiation without widening for this TypeVar.
    */
   final class TypeVar private(
       initOrigin: TypeParamRef,
@@ -5045,6 +5046,9 @@ object Types extends TypeUtils {
       else
         instantiateWith(tp)
 
+    /** Should we suppress widening? True if this TypeVar is precise
+     *  or if it has as an upper bound a precise TypeVar.
+     */
     def isPrecise(using Context) =
       precise
       || {
@@ -5055,7 +5059,9 @@ object Types extends TypeUtils {
             case _ => false
       }
 
-    /** Widen unions when instantiating this variable in the current context? */
+    /** The policy used for widening singletons or unions when instantiating
+     *  this variable in the current context.
+     */
     def widenPolicy(using Context): Widen =
       if isPrecise then Widen.None
       else if ctx.typerState.constraint.isHard(this) then Widen.Singletons
@@ -5107,6 +5113,7 @@ object Types extends TypeUtils {
         precise: Boolean = false) =
       new TypeVar(initOrigin, creatorState, nestingLevel, precise)
 
+  /** The three possible widening policies */
   enum Widen:
     case None        // no widening
     case Singletons  // widen singletons but not unions