Merge pull request #6486 from dotty-staging/given-last

Move Given Last in Instance Definitions

Author: biboudis

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

@@ -70,16 +70,21 @@ object NavigateAST {
    */
   def pathTo(span: Span, from: Positioned, skipZeroExtent: Boolean = false)(implicit ctx: Context): List[Positioned] = {
     def childPath(it: Iterator[Any], path: List[Positioned]): List[Positioned] = {
+      var bestFit: List[Positioned] = path
       while (it.hasNext) {
         val path1 = it.next() match {
           case p: Positioned => singlePath(p, path)
           case m: untpd.Modifiers => childPath(m.productIterator, path)
           case xs: List[_] => childPath(xs.iterator, path)
           case _ => path
         }
-        if (path1 ne path) return path1
+        if ((path1 ne path) &&
+            ((bestFit eq path) ||
+             bestFit.head.span != path1.head.span &&
+             bestFit.head.span.contains(path1.head.span)))
+          bestFit = path1
       }
-      path
+      bestFit
     }
     def singlePath(p: Positioned, path: List[Positioned]): List[Positioned] =
       if (p.span.exists && !(skipZeroExtent && p.span.isZeroExtent) && p.span.contains(span)) {

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

@@ -147,6 +147,11 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Pro
       }
   }
 
+  /** A hook that can be overridden if overlap checking in `checkPos` should be
+   *  disabled for this node.
+   */
+  def disableOverlapChecks = false
+
   /** Check that all positioned items in this tree satisfy the following conditions:
    *  - Parent spans contain child spans
    *  - If item is a non-empty tree, it has a position
@@ -169,7 +174,7 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Pro
               s"position error: position not set for $tree # ${tree.uniqueId}")
           case _ =>
         }
-        if (nonOverlapping) {
+        if (nonOverlapping && !disableOverlapChecks) {
           this match {
             case _: XMLBlock =>
               // FIXME: Trees generated by the XML parser do not satisfy `checkPos`

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

@@ -747,6 +747,10 @@ object Trees {
     assert(tpt != genericEmptyTree)
     def unforced: LazyTree = preRhs
     protected def force(x: AnyRef): Unit = preRhs = x
+
+    override def disableOverlapChecks = rawMods.is(Flags.Implied)
+      // disable order checks for implicit aliases since their given clause follows
+      // their for clause, but the two appear swapped in the DefDef.
   }
 
   class BackquotedDefDef[-T >: Untyped] private[ast] (name: TermName, tparams: List[TypeDef[T]],
@@ -784,6 +788,10 @@ object Trees {
 
     def parents: List[Tree[T]] = parentsOrDerived // overridden by DerivingTemplate
     def derived: List[untpd.Tree] = Nil           // overridden by DerivingTemplate
+
+    override def disableOverlapChecks = true
+      // disable overlaps checks since templates of instance definitions have their
+      // `given` clause come last, which means that the constructor span can contain the parent spans.
   }
 
 

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -2688,22 +2688,22 @@ object Parsers {
       Template(constr, parents, Nil, EmptyValDef, Nil)
     }
 
-    /** InstanceDef    ::=  [id] InstanceParams InstanceBody
+    /** InstanceDef    ::=  [id] [DefTypeParamClause] InstanceBody
      *  InstanceParams ::=  [DefTypeParamClause] {GivenParamClause}
-     *  InstanceBody   ::=  [‘for’ ConstrApp {‘,’ ConstrApp }] [TemplateBody]
-     *                   |  ‘for’ Type ‘=’ Expr
+     *  InstanceBody   ::=  [‘for’ ConstrApp {‘,’ ConstrApp }] {GivenParamClause} [TemplateBody]
+     *                   |  ‘for’ Type {GivenParamClause} ‘=’ Expr
      */
     def instanceDef(start: Offset, mods: Modifiers, instanceMod: Mod) = atSpan(start, nameStart) {
       var mods1 = addMod(mods, instanceMod)
       val name = if (isIdent) ident() else EmptyTermName
       val tparams = typeParamClauseOpt(ParamOwner.Def)
-      val vparamss = paramClauses(ofInstance = true)
       val parents =
         if (in.token == FOR) {
           in.nextToken()
           tokenSeparated(COMMA, constrApp)
         }
         else Nil
+      val vparamss = paramClauses(ofInstance = true)
       val instDef =
         if (in.token == EQUALS && parents.length == 1 && parents.head.isType) {
           in.nextToken()

docs/docs/internals/syntax.md

@@ -378,10 +378,10 @@ ClassConstr       ::=  [ClsTypeParamClause] [ConstrMods] ClsParamClauses
 ConstrMods        ::=  {Annotation} [AccessModifier]
 ObjectDef         ::=  id [Template]                                            ModuleDef(mods, name, template)  // no constructor
 EnumDef           ::=  id ClassConstr InheritClauses EnumBody                   EnumDef(mods, name, tparams, template)
-InstanceDef       ::=  [id] InstanceParams InstanceBody
+InstanceDef       ::=  [id] [DefTypeParamClause] InstanceBody
 InstanceParams    ::=  [DefTypeParamClause] {GivenParamClause}
-InstanceBody      ::=  [‘for’ ConstrApp {‘,’ ConstrApp }] [TemplateBody]
-                    |  ‘for’ Type ‘=’ Expr
+InstanceBody      ::=  [‘for’ ConstrApp {‘,’ ConstrApp }] {GivenParamClause} [TemplateBody]
+                    |  ‘for’ Type {GivenParamClause} ‘=’ Expr
 Template          ::=  InheritClauses [TemplateBody]                            Template(constr, parents, self, stats)
 InheritClauses    ::=  [‘extends’ ConstrApps] [‘derives’ QualId {‘,’ QualId}]
 ConstrApps        ::=  ConstrApp {‘with’ ConstrApp}

docs/docs/reference/contextual-implicit/instance-defs.md

@@ -71,10 +71,10 @@ An implicit instance without type parameters or given clause is created on-deman
 Here is the new syntax of implicit instances, seen as a delta from the [standard context free syntax of Scala 3](http://dotty.epfl.ch/docs/internals/syntax.html).
 ```
 TmplDef          ::=  ...
-                  |  ‘implicit’ InstanceDef
+                   |  ‘implicit’ InstanceDef
 InstanceDef      ::=  [id] [DefTypeParamClause] InstanceBody
-InstanceBody     ::=  [‘of’ ConstrApp {‘,’ ConstrApp }] {GivenParamClause} [TemplateBody]
-                   |  ‘of’ Type {GivenParamClause} ‘=’ Expr
+InstanceBody     ::=  [‘for’ ConstrApp {‘,’ ConstrApp }] {GivenParamClause} [TemplateBody]
+                   |  ‘for’ Type {GivenParamClause} ‘=’ Expr
 ConstrApp        ::=  AnnotType {ArgumentExprs}
                    |  ‘(’ ConstrApp {‘given’ (InfixExpr | ParArgumentExprs)} ‘)’
 GivenParamClause ::=  ‘given’ (‘(’ [DefParams] ‘)’ | GivenTypes)

tests/neg/i5978.scala

@@ -7,8 +7,8 @@ trait TokenParser[Token, R]
 object TextParser {
   implied TP for TokenParser[Char, Position[CharSequence]] {}
 
-  implied FromCharToken
-    given (T: TokenParser[Char, Position[CharSequence]]) for Conversion[Char, Position[CharSequence]] = ???
+  implied FromCharToken for Conversion[Char, Position[CharSequence]]
+    given (T: TokenParser[Char, Position[CharSequence]])  = ???
 }
 
 

tests/pos/i5978.scala

@@ -9,8 +9,8 @@ package p1 {
   object TextParser {
     implied TP for TokenParser[Char, Position[CharSequence]] {}
 
-    implied FromCharToken
-      given (T: TokenParser[Char, Position[CharSequence]]) for Conversion[Char, Position[CharSequence]] = ???
+    implied FromCharToken for Conversion[Char, Position[CharSequence]]
+      given (T: TokenParser[Char, Position[CharSequence]])= ???
   }
 
 

tests/pos/multiversal.scala

@@ -1,7 +1,7 @@
 object Test {
   import scala.Eql
 
-  implied [X, Y] given Eql[X, Y] for Eql[List[X], List[Y]] = Eql.derived
+  implied [X, Y] for Eql[List[X], List[Y]] given Eql[X, Y] = Eql.derived
 
   val b: Byte = 1
   val c: Char = 2

tests/pos/reference/instances.scala

@@ -37,7 +37,7 @@ object Instances extends Common {
       if (x < y) -1 else if (x > y) +1 else 0
   }
 
-  implied ListOrd[T] given Ord[T] for Ord[List[T]] {
+  implied ListOrd[T] for Ord[List[T]] given Ord[T] {
     def (xs: List[T]) compareTo (ys: List[T]): Int = (xs, ys) match {
       case (Nil, Nil) => 0
       case (Nil, _) => -1
@@ -132,7 +132,7 @@ object Instances extends Common {
         println(the[D[Int]])
       }
       locally {
-        implied given Context for D[Int]
+        implied for D[Int] given Context
         println(the[D[Int]])
       }
     }
@@ -195,7 +195,7 @@ object AnonymousInstances extends Common {
     def (xs: List[T]) second[T] = xs.tail.head
   }
 
-  implied [From, To] given (c: Convertible[From, To]) for Convertible[List[From], List[To]] {
+  implied [From, To] for Convertible[List[From], List[To]] given (c: Convertible[From, To]) {
     def (x: List[From]) convert: List[To] = x.map(c.convert)
   }
 

tests/run/implicit-specifity.scala

@@ -9,7 +9,7 @@ object Show {
 class Generic
 object Generic {
   implied gen for Generic = new Generic
-  implied showGen[T] given Generic for Show[T] = new Show[T](2)
+  implied showGen[T] for Show[T] given Generic = new Show[T](2)
 }
 
 class Generic2
@@ -25,9 +25,9 @@ object SubGen {
 object Contextual {
   trait Context
   implied ctx for Context
-  implied showGen[T] given Generic for Show[T] = new Show[T](2)
-  implied showGen[T] given Generic, Context for Show[T] = new Show[T](3)
-  implied showGen[T] given SubGen for Show[T] = new Show[T](4)
+  implied showGen[T] for Show[T] given Generic = new Show[T](2)
+  implied showGen[T] for Show[T] given Generic, Context = new Show[T](3)
+  implied showGen[T] for Show[T] given SubGen = new Show[T](4)
 }
 
 object Test extends App {

tests/run/implied-divergence.scala

@@ -6,7 +6,7 @@ implied e for E(null)
 
 object Test extends App {
 
-  implied f given (e: E) for E(e)
+  implied f for E(e) given (e: E)
 
   assert(the[E].toString == "E(E(null))")
 

tests/run/implied-priority.scala

@@ -15,7 +15,7 @@ class LowPriorityImplicits {
 }
 
 object NormalImplicits extends LowPriorityImplicits {
-  implied t2[T] given Arg[T] for E[T]("norm")
+  implied t2[T]for E[T]("norm") given Arg[T]
 }
 
 def test1 = {
@@ -38,8 +38,8 @@ object Priority {
 }
 
 object Impl2 {
-  implied t1[T] given Priority.Low for E[T]("low")
-  implied t2[T] given Priority.High given Arg[T] for E[T]("norm")
+  implied t1[T] for E[T]("low") given Priority.Low
+  implied t2[T] for E[T]("norm") given Priority.High given Arg[T]
 }
 
 def test2 = {
@@ -102,7 +102,7 @@ def test3 = {
  */
 object Impl4 {
   implied t1 for E[String]("string")
-  implied t2[T] given Arg[T] for E[T]("generic")
+  implied t2[T] for E[T]("generic") given Arg[T]
 }
 
 object fallback4 {
@@ -133,7 +133,7 @@ object HigherPriority {
 }
 
 object fallback5 {
-  implied [T] given (ev: E[T] = new E[T]("fallback")) for (E[T] & HigherPriority) = HigherPriority.inject(ev)
+  implied [T] for (E[T] & HigherPriority) given (ev: E[T] = new E[T]("fallback")) = HigherPriority.inject(ev)
 }
 
 def test5 = {

tests/run/implied-specifity-2.scala

@@ -15,36 +15,36 @@ class Foo[T](val i: Int)
 object Foo {
   def apply[T] given (fooT: Foo[T]): Int = fooT.i
 
-  implied foo[T]    given Low for Foo[T](0)
-  implied foobar[T] given Low for Foo[Bar[T]](1)
-  implied foobarbaz given Low for Foo[Bar[Baz]](2)
+  implied foo[T]    for Foo[T](0)        given Low
+  implied foobar[T] for Foo[Bar[T]](1)   given Low
+  implied foobarbaz for Foo[Bar[Baz]](2) given Low
 }
 class Bar[T]
 object Bar {
-  implied foobar[T] given Medium for Foo[Bar[T]](3)
-  implied foobarbaz given Medium for Foo[Bar[Baz]](4)
+  implied foobar[T] for Foo[Bar[T]](3)   given Medium
+  implied foobarbaz for Foo[Bar[Baz]](4) given Medium
 }
 class Baz
 object Baz {
-  implied baz given High for Foo[Bar[Baz]](5)
+  implied baz for Foo[Bar[Baz]](5) given High
 }
 
 class Arg
 implied for Arg
 
 class Bam(val str: String)
-implied lo given Low for Bam("lo")
-implied hi given High given Arg for Bam("hi")
+implied lo for Bam("lo") given Low
+implied hi for Bam("hi") given High given Arg
 
 class Bam2(val str: String)
-implied lo2 given Low for Bam2("lo")
-implied mid2 given High given Arg for Bam2("mid")
+implied lo2 for Bam2("lo") given Low
+implied mid2 for Bam2("mid") given High given Arg
 implied hi2 for Bam2("hi")
 
 class Arg2
 class Red(val str: String)
-implied normal given Arg2 for Red("normal")
-implied reduced given (ev: Arg2 | Low) for Red("reduced")
+implied normal for Red("normal") given Arg2
+implied reduced for Red("reduced") given (ev: Arg2 | Low)
 
 object Test extends App {
   assert(Foo[Int] == 0)

tests/run/tagless.scala

@@ -196,7 +196,7 @@ object Test extends App {
   // Added operation: negation pushdown
   enum NCtx { case Pos, Neg }
 
-  implied [T] given (e: Exp[T]) for Exp[NCtx => T] {
+  implied [T] for Exp[NCtx => T] given (e: Exp[T]) {
     import NCtx._
     def lit(i: Int) = {
       case Pos => e.lit(i)
@@ -216,7 +216,7 @@ object Test extends App {
   println(pushNeg(tf1[NCtx => String]))
   println(pushNeg(pushNeg(pushNeg(tf1))): String)
 
-  implied [T] given (e: Mult[T]) for Mult[NCtx => T] {
+  implied [T] for Mult[NCtx => T] given (e: Mult[T]) {
     import NCtx._
     def mul(l: NCtx => T, r: NCtx => T): NCtx => T = {
       case Pos => e.mul(l(Pos), r(Pos))