Skip to content

Commit dc742a8

Browse files
committed
Make positions fit for meta
In particular: - get rid of envelope, it's too complicated and hides too many errors - check that everywhere in parsed trees the position range of a parent node contains the position ranges of its children. - check that all non-empty trees coming from parser have positions. The commit contains lots of fixes to make these checks pass. In particular, it changes the scheme how definitions are positioned. Previously the position of a definition was the token range of the name defined by it. That does obviously not work with the parent/children invariant. Now, the position is the whole definition range, with the point at the defined name (care is taken to not count backticks). Namer is changed to still use the token range of defined name as the position of the symbol.
1 parent e5509a0 commit dc742a8

15 files changed

+243
-163
lines changed

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ object desugar {
506506
val clsTmpl = cpy.Template(tmpl)(self = clsSelf, body = tmpl.body)
507507
val cls = TypeDef(clsName, clsTmpl)
508508
.withMods(mods.toTypeFlags & RetainedModuleClassFlags | ModuleClassCreationFlags)
509-
Thicket(modul, classDef(cls))
509+
Thicket(modul, classDef(cls).withPos(mdef.pos))
510510
}
511511
}
512512

@@ -517,7 +517,7 @@ object desugar {
517517
def patDef(pdef: PatDef)(implicit ctx: Context): Tree = {
518518
val PatDef(mods, pats, tpt, rhs) = pdef
519519
val pats1 = if (tpt.isEmpty) pats else pats map (Typed(_, tpt))
520-
flatTree(pats1 map (makePatDef(mods, _, rhs)))
520+
flatTree(pats1 map (makePatDef(pdef, mods, _, rhs)))
521521
}
522522

523523
/** If `pat` is a variable pattern,
@@ -535,9 +535,9 @@ object desugar {
535535
* If the original pattern variable carries a type annotation, so does the corresponding
536536
* ValDef or DefDef.
537537
*/
538-
def makePatDef(mods: Modifiers, pat: Tree, rhs: Tree)(implicit ctx: Context): Tree = pat match {
538+
def makePatDef(original: Tree, mods: Modifiers, pat: Tree, rhs: Tree)(implicit ctx: Context): Tree = pat match {
539539
case VarPattern(named, tpt) =>
540-
derivedValDef(named, tpt, rhs, mods)
540+
derivedValDef(original, named, tpt, rhs, mods)
541541
case _ =>
542542
val rhsUnchecked = makeAnnotated(defn.UncheckedAnnot, rhs)
543543
val vars = getVariables(pat)
@@ -554,7 +554,7 @@ object desugar {
554554
case Nil =>
555555
matchExpr
556556
case (named, tpt) :: Nil =>
557-
derivedValDef(named, tpt, matchExpr, mods)
557+
derivedValDef(original, named, tpt, matchExpr, mods)
558558
case _ =>
559559
val tmpName = ctx.freshName().toTermName
560560
val patMods = mods & (AccessFlags | Lazy) | Synthetic
@@ -565,8 +565,8 @@ object desugar {
565565
val restDefs =
566566
for (((named, tpt), n) <- vars.zipWithIndex)
567567
yield
568-
if (mods is Lazy) derivedDefDef(named, tpt, selector(n), mods &~ Lazy)
569-
else derivedValDef(named, tpt, selector(n), mods)
568+
if (mods is Lazy) derivedDefDef(original, named, tpt, selector(n), mods &~ Lazy)
569+
else derivedValDef(original, named, tpt, selector(n), mods)
570570
flatTree(firstDef :: restDefs)
571571
}
572572
}
@@ -664,14 +664,18 @@ object desugar {
664664
def makeAnnotated(cls: Symbol, tree: Tree)(implicit ctx: Context) =
665665
Annotated(untpd.New(untpd.TypeTree(cls.typeRef), Nil), tree)
666666

667-
private def derivedValDef(named: NameTree, tpt: Tree, rhs: Tree, mods: Modifiers)(implicit ctx: Context) = {
668-
val vdef = ValDef(named.name.asTermName, tpt, rhs).withMods(mods).withPos(named.pos)
667+
private def derivedValDef(original: Tree, named: NameTree, tpt: Tree, rhs: Tree, mods: Modifiers)(implicit ctx: Context) = {
668+
val vdef = ValDef(named.name.asTermName, tpt, rhs)
669+
.withMods(mods)
670+
.withPos(original.pos.withPoint(named.pos.start))
669671
val mayNeedSetter = valDef(vdef)
670672
mayNeedSetter
671673
}
672674

673-
private def derivedDefDef(named: NameTree, tpt: Tree, rhs: Tree, mods: Modifiers) =
674-
DefDef(named.name.asTermName, Nil, Nil, tpt, rhs).withMods(mods).withPos(named.pos)
675+
private def derivedDefDef(original: Tree, named: NameTree, tpt: Tree, rhs: Tree, mods: Modifiers) =
676+
DefDef(named.name.asTermName, Nil, Nil, tpt, rhs)
677+
.withMods(mods)
678+
.withPos(original.pos.withPoint(named.pos.start))
675679

676680
/** Main desugaring method */
677681
def apply(tree: Tree)(implicit ctx: Context): Tree = {
@@ -761,7 +765,7 @@ object desugar {
761765
*/
762766
def makeLambda(pat: Tree, body: Tree): Tree = pat match {
763767
case VarPattern(named, tpt) =>
764-
Function(derivedValDef(named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body)
768+
Function(derivedValDef(pat, named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body)
765769
case _ =>
766770
makeCaseLambda(CaseDef(pat, EmptyTree, body) :: Nil, unchecked = false)
767771
}
@@ -864,7 +868,7 @@ object desugar {
864868
val rhss = valeqs map { case GenAlias(_, rhs) => rhs }
865869
val (defpat0, id0) = makeIdPat(pat)
866870
val (defpats, ids) = (pats map makeIdPat).unzip
867-
val pdefs = (defpats, rhss).zipped map (makePatDef(Modifiers(), _, _))
871+
val pdefs = (valeqs, defpats, rhss).zipped.map(makePatDef(_, Modifiers(), _, _))
868872
val rhs1 = makeFor(nme.map, nme.flatMap, GenFrom(defpat0, rhs) :: Nil, Block(pdefs, makeTuple(id0 :: ids)))
869873
val allpats = pat :: pats
870874
val vfrom1 = new IrrefutableGenFrom(makeTuple(allpats), rhs1)
@@ -947,7 +951,7 @@ object desugar {
947951
makeFor(nme.map, nme.flatMap, enums, body) orElse tree
948952
case PatDef(mods, pats, tpt, rhs) =>
949953
val pats1 = if (tpt.isEmpty) pats else pats map (Typed(_, tpt))
950-
flatTree(pats1 map (makePatDef(mods, _, rhs)))
954+
flatTree(pats1 map (makePatDef(tree, mods, _, rhs)))
951955
case ParsedTry(body, handler, finalizer) =>
952956
handler match {
953957
case Match(EmptyTree, cases) => Try(body, cases, finalizer)

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,9 @@ object NavigateAST {
1919
case _ =>
2020
val loosePath = untypedPath(tree, exactMatch = false)
2121
throw new
22-
Error(i"""no untyped tree for $tree, pos = ${tree.pos}, envelope = ${tree.envelope}
22+
Error(i"""no untyped tree for $tree, pos = ${tree.pos}
2323
|best matching path =\n$loosePath%\n====\n%
24-
|path positions = ${loosePath.map(_.pos)}
25-
|path envelopes = ${loosePath.map(_.envelope)}""")
24+
|path positions = ${loosePath.map(_.pos)}""")
2625
}
2726

2827
/** The reverse path of untyped trees starting with a tree that closest matches
@@ -40,7 +39,7 @@ object NavigateAST {
4039
def untypedPath(tree: tpd.Tree, exactMatch: Boolean = false)(implicit ctx: Context): List[Positioned] =
4140
tree match {
4241
case tree: MemberDef[_] =>
43-
untypedPath(tree.envelope) match {
42+
untypedPath(tree.pos) match {
4443
case path @ (last: DefTree[_]) :: _ => path
4544
case path if !exactMatch => path
4645
case _ => Nil
@@ -76,7 +75,7 @@ object NavigateAST {
7675
path
7776
}
7877
def singlePath(p: Positioned, path: List[Positioned]): List[Positioned] =
79-
if (p.envelope contains pos) childPath(p.productIterator, p :: path)
78+
if (p.pos contains pos) childPath(p.productIterator, p :: path)
8079
else path
8180
singlePath(from, Nil)
8281
}

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

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ package ast
33

44
import util.Positions._
55
import util.DotClass
6+
import core.Contexts.Context
7+
import core.Decorators._
8+
import core.Flags.JavaDefined
9+
import core.StdNames.nme
610

711
/** A base class for things that have positions (currently: modifiers and trees)
812
*/
@@ -16,19 +20,14 @@ abstract class Positioned extends DotClass with Product {
1620
*/
1721
def pos: Position = curPos
1822

19-
/** Destructively update `curPos` to given position. Also, set any missing
23+
/** Destructively update `curPos` to given position. Also, set any missing
2024
* positions in children.
2125
*/
2226
protected def setPos(pos: Position): Unit = {
2327
curPos = pos
2428
if (pos.exists) setChildPositions(pos.toSynthetic)
2529
}
2630

27-
/** The envelope containing the item in its entirety. Envelope is different from
28-
* `pos` for definitions (instances of MemberDef).
29-
*/
30-
def envelope: Position = pos.toSynthetic
31-
3231
/** A positioned item like this one with the position set to `pos`.
3332
* if the positioned item is source-derived, a clone is returned.
3433
* If the positioned item is synthetic, the position is updated
@@ -106,16 +105,15 @@ abstract class Positioned extends DotClass with Product {
106105
}
107106
}
108107

109-
/** The initial, synthetic position. This is usually the union of all positioned children's
110-
* envelopes.
108+
/** The initial, synthetic position. This is usually the union of all positioned children's positions.
111109
*/
112110
protected def initialPos: Position = {
113111
var n = productArity
114112
var pos = NoPosition
115113
while (n > 0) {
116114
n -= 1
117115
productElement(n) match {
118-
case p: Positioned => pos = pos union p.envelope
116+
case p: Positioned => pos = pos union p.pos
119117
case xs: List[_] => pos = unionPos(pos, xs)
120118
case _ =>
121119
}
@@ -124,7 +122,7 @@ abstract class Positioned extends DotClass with Product {
124122
}
125123

126124
private def unionPos(pos: Position, xs: List[_]): Position = xs match {
127-
case (p: Positioned) :: xs1 => unionPos(pos union p.envelope, xs1)
125+
case (p: Positioned) :: xs1 => unionPos(pos union p.pos, xs1)
128126
case _ => pos
129127
}
130128

@@ -138,7 +136,7 @@ abstract class Positioned extends DotClass with Product {
138136
false
139137
}
140138
(this eq that) ||
141-
(this.envelope contains that.pos) && {
139+
(this.pos contains that.pos) && {
142140
var n = productArity
143141
var found = false
144142
while (n > 0 && !found) {
@@ -148,4 +146,48 @@ abstract class Positioned extends DotClass with Product {
148146
found
149147
}
150148
}
149+
150+
/** Check that all positioned items in this tree satisfy the following conditions:
151+
* - Parent positions contain child positions
152+
* - If item is a non-empty tree, it has a position
153+
*/
154+
def checkPos(complete: Boolean)(implicit ctx: Context): Unit = try {
155+
import untpd._
156+
def check(p: Any): Unit = p match {
157+
case p: Positioned =>
158+
assert(pos contains p.pos,
159+
s"""position error, parent position does not contain child positon
160+
|parent = $this,
161+
|parent position = $pos,
162+
|child = $p,
163+
|child position = ${p.pos}""".stripMargin)
164+
p match {
165+
case tree: Tree if !tree.isEmpty =>
166+
assert(tree.pos.exists,
167+
s"position error: position not set for $tree # ${tree.uniqueId}")
168+
case _ =>
169+
}
170+
p.checkPos(complete)
171+
case xs: List[_] =>
172+
xs.foreach(check)
173+
case _ =>
174+
}
175+
this match {
176+
case tree: DefDef if tree.name == nme.CONSTRUCTOR && tree.mods.is(JavaDefined) =>
177+
// Special treatment for constructors coming from Java:
178+
// Leave out tparams, they are copied with wrong positions from parent class
179+
check(tree.mods)
180+
check(tree.vparamss)
181+
case _ =>
182+
var n = productArity
183+
while (n > 0) {
184+
n -= 1
185+
check(productElement(n))
186+
}
187+
}
188+
} catch {
189+
case ex: AssertionError =>
190+
println(i"error while checking $this")
191+
throw ex
192+
}
151193
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
480480
require(sym.pos.exists)
481481
object accum extends TreeAccumulator[List[Tree]] {
482482
def apply(x: List[Tree], tree: Tree)(implicit ctx: Context): List[Tree] = {
483-
if (tree.envelope.contains(sym.pos))
483+
if (tree.pos.contains(sym.pos))
484484
if (definedSym(tree) == sym) tree :: x
485485
else {
486486
val x1 = foldOver(x, tree)

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,6 @@ object Trees {
341341
}
342342

343343
protected def setMods(mods: Modifiers[T @uncheckedVariance]) = myMods = mods
344-
345-
override def envelope: Position = rawMods.pos.union(pos).union(initialPos)
346344
}
347345

348346
/** A ValDef or DefDef tree */
@@ -619,7 +617,6 @@ object Trees {
619617
type ThisTree[-T >: Untyped] = Bind[T]
620618
override def isType = name.isTypeName
621619
override def isTerm = name.isTermName
622-
override def envelope: Position = pos union initialPos
623620
}
624621

625622
/** tree_1 | ... | tree_n */
@@ -740,6 +737,7 @@ object Trees {
740737
val newTrees = trees.map(_.withPos(pos))
741738
new Thicket[T](newTrees).asInstanceOf[this.type]
742739
}
740+
override def pos = (NoPosition /: trees) ((pos, t) => pos union t.pos)
743741
override def foreachInThicket(op: Tree[T] => Unit): Unit =
744742
trees foreach (_.foreachInThicket(op))
745743
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
258258
implicit class UntypedTreeDecorator(val self: Tree) extends AnyVal {
259259
def locateEnclosing(base: List[Tree], pos: Position): List[Tree] = {
260260
def encloses(elem: Any) = elem match {
261-
case t: Tree => t.envelope contains pos
261+
case t: Tree => t.pos contains pos
262262
case _ => false
263263
}
264264
base.productIterator find encloses match {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ object Config {
6363
*/
6464
final val checkNoDoubleBindings = true
6565

66+
/** Check positions for consistency after parsing */
67+
final val checkPositions = true
68+
6669
/** Show subtype traces for all deep subtype recursions */
6770
final val traceDeepSubTypeRecursions = false
6871

0 commit comments

Comments
 (0)