Make Modifiers not extend from Positioned

Modifiers was made a subclass of Positioned since it contains elements that
are Positioned: source modifiers and annotations. That way, we got the navigation
logic to get to the elements for free. However, a lot of modifiers are synthetic,
and seems artificial to give these a position. With the addition of a source
component to position the balance swings the other way. It now seems preferable
to have Modifiers not extend Positions and to consider modifier as a as a separate
case for navigation instead.

Author: odersky

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

@@ -72,6 +72,7 @@ object NavigateAST {
       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
         }

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

@@ -105,6 +105,8 @@ abstract class Positioned extends Product {
             end = p.pos.start
           }
           else outstanding = p :: outstanding
+        case m: untpd.Modifiers =>
+          elems = elems ::: m.mods.reverse ::: m.annotations.reverse
         case xs: List[_] =>
           elems = elems ::: xs.reverse
         case _ =>
@@ -121,6 +123,7 @@ abstract class Positioned extends Product {
       n -= 1
       productElement(n) match {
         case p: Positioned => pos = pos union p.pos
+        case m: untpd.Modifiers => pos = unionPos(unionPos(pos, m.mods), m.annotations)
         case xs: List[_] => pos = unionPos(pos, xs)
         case _ =>
       }
@@ -138,9 +141,11 @@ abstract class Positioned extends Product {
   def contains(that: Positioned): Boolean = {
     def isParent(x: Any): Boolean = x match {
       case x: Positioned =>
-        x contains that
+        x.contains(that)
+      case m: untpd.Modifiers =>
+        m.mods.exists(isParent) || m.annotations.exists(isParent)
       case xs: List[_] =>
-        xs exists isParent
+        xs.exists(isParent)
       case _ =>
         false
     }
@@ -198,6 +203,9 @@ abstract class Positioned extends Product {
           lastPos = p.pos
         }
         p.checkPos(nonOverlapping)
+      case m: untpd.Modifiers =>
+        m.annotations.foreach(check)
+        m.mods.foreach(check)
       case xs: List[_] =>
         xs.foreach(check)
       case _ =>

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

@@ -151,7 +151,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
     flags: FlagSet = EmptyFlags,
     privateWithin: TypeName = tpnme.EMPTY,
     annotations: List[Tree] = Nil,
-    mods: List[Mod] = Nil) extends Positioned with Cloneable {
+    mods: List[Mod] = Nil) extends Cloneable {
 
     def is(fs: FlagSet): Boolean = flags is fs
     def is(fc: FlagConjunction): Boolean = flags is fc

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

@@ -732,7 +732,7 @@ object JavaParsers {
       val members = new ListBuffer[Tree]
       while (in.token != RBRACE && in.token != EOF) {
         val start = in.offset
-        var mods = atPos(start) { modifiers(inInterface) }
+        var mods = modifiers(inInterface)
         if (in.token == LBRACE) {
           skipAhead() // skip init block, we just assume we have seen only static
           accept(RBRACE)
@@ -895,7 +895,7 @@ object JavaParsers {
         while (in.token == SEMI) in.nextToken()
         if (in.token != EOF) {
           val start = in.offset
-          val mods = atPos(start) { modifiers(inInterface = false) }
+          val mods = modifiers(inInterface = false)
           buf ++= typeDecl(start, mods)
         }
       }

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

@@ -1927,7 +1927,7 @@ object Parsers {
       def typeParam(): TypeDef = {
         val isConcreteOwner = ownerKind == ParamOwner.Class || ownerKind == ParamOwner.Def
         val start = in.offset
-        val mods = atPos(start) {
+        val mods =
           annotsAsMods() | {
             if (ownerKind == ParamOwner.Class) Param | PrivateLocal
             else Param
@@ -1938,7 +1938,6 @@ object Parsers {
               else EmptyFlags
             else EmptyFlags
           }
-        }
         atPos(start, nameStart) {
           val name =
             if (isConcreteOwner || in.token != USCORE) ident().toTypeName
@@ -1984,27 +1983,25 @@ object Parsers {
         if (ofClass) {
           mods = addFlag(modifiers(start = mods), ParamAccessor)
           mods =
-            atPos(start, in.offset) {
-              if (in.token == VAL) {
-                in.nextToken()
-                mods
-              }
-              else if (in.token == VAR) {
-                val mod = atPos(in.skipToken()) { Mod.Var() }
-                addMod(mods, mod)
-              }
-              else {
-                if (!(mods.flags &~ (ParamAccessor | Inline | impliedMods.flags)).isEmpty)
-                  syntaxError("`val' or `var' expected")
-                if (firstClause && ofCaseClass) mods
-                else mods | PrivateLocal
-              }
+            if (in.token == VAL) {
+              in.nextToken()
+              mods
+            }
+            else if (in.token == VAR) {
+              val mod = atPos(in.skipToken()) { Mod.Var() }
+              addMod(mods, mod)
+            }
+            else {
+              if (!(mods.flags &~ (ParamAccessor | Inline | impliedMods.flags)).isEmpty)
+                syntaxError("`val' or `var' expected")
+              if (firstClause && ofCaseClass) mods
+              else mods | PrivateLocal
             }
         }
         else {
           if (isIdent(nme.inline) && in.isSoftModifierInParamModifierPosition)
             mods = addModifier(mods)
-          mods = atPos(start) { mods | Param }
+          mods |= Param
         }
         atPos(start, nameStart) {
           val name = ident()
@@ -2016,7 +2013,7 @@ object Parsers {
             if (in.token == EQUALS) { in.nextToken(); expr() }
             else EmptyTree
           if (implicitOffset >= 0) {
-            mods = mods.withPos(mods.pos.union(Position(implicitOffset, implicitOffset)))
+            //mods = mods.withPos(mods.pos.union(Position(implicitOffset, implicitOffset)))
             implicitOffset = -1
           }
           ValDef(name, tpt, default).withMods(mods)
@@ -2157,9 +2154,8 @@ object Parsers {
     }
 
     def posMods(start: Int, mods: Modifiers): Modifiers = {
-      val mods1 = atPos(start)(mods)
       in.nextToken()
-      mods1
+      mods
     }
 
     /** Def      ::= val PatDef
@@ -2549,8 +2545,10 @@ object Parsers {
         setLastStatOffset()
         if (in.token == PACKAGE) {
           val start = in.skipToken()
-          if (in.token == OBJECT)
-            stats += objectDef(start, atPos(start, in.skipToken()) { Modifiers(Package) })
+          if (in.token == OBJECT) {
+            in.nextToken()
+            stats += objectDef(start, Modifiers(Package))
+          }
           else stats += packaging(start)
         }
         else if (in.token == IMPORT)
@@ -2710,7 +2708,8 @@ object Parsers {
         if (in.token == PACKAGE) {
           in.nextToken()
           if (in.token == OBJECT) {
-            ts += objectDef(start, atPos(start, in.skipToken()) { Modifiers(Package) })
+            in.nextToken()
+            ts += objectDef(start, Modifiers(Package))
             if (in.token != EOF) {
               acceptStatSep()
               ts ++= topStatSeq()

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

@@ -266,7 +266,7 @@ class Namer { typer: Typer =>
   def createSymbol(tree: Tree)(implicit ctx: Context): Symbol = {
 
     def privateWithinClass(mods: Modifiers) =
-      enclosingClassNamed(mods.privateWithin, mods.pos)
+      enclosingClassNamed(mods.privateWithin, tree.pos)
 
     /** Check that flags are OK for symbol. This is done early to avoid
      *  catastrophic failure when we create a TermSymbol with TypeFlags, or vice versa.