Skip to content

Commit fb71045

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 003ea0f commit fb71045

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
@@ -505,7 +505,7 @@ object desugar {
505505
val clsTmpl = cpy.Template(tmpl)(self = clsSelf, body = tmpl.body)
506506
val cls = TypeDef(clsName, clsTmpl)
507507
.withMods(mods.toTypeFlags & RetainedModuleClassFlags | ModuleClassCreationFlags)
508-
Thicket(modul, classDef(cls))
508+
Thicket(modul, classDef(cls).withPos(mdef.pos))
509509
}
510510
}
511511

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

522522
/** If `pat` is a variable pattern,
@@ -534,9 +534,9 @@ object desugar {
534534
* If the original pattern variable carries a type annotation, so does the corresponding
535535
* ValDef or DefDef.
536536
*/
537-
def makePatDef(mods: Modifiers, pat: Tree, rhs: Tree)(implicit ctx: Context): Tree = pat match {
537+
def makePatDef(original: Tree, mods: Modifiers, pat: Tree, rhs: Tree)(implicit ctx: Context): Tree = pat match {
538538
case VarPattern(named, tpt) =>
539-
derivedValDef(named, tpt, rhs, mods)
539+
derivedValDef(original, named, tpt, rhs, mods)
540540
case _ =>
541541
val rhsUnchecked = makeAnnotated(defn.UncheckedAnnot, rhs)
542542
val vars = getVariables(pat)
@@ -553,7 +553,7 @@ object desugar {
553553
case Nil =>
554554
matchExpr
555555
case (named, tpt) :: Nil =>
556-
derivedValDef(named, tpt, matchExpr, mods)
556+
derivedValDef(original, named, tpt, matchExpr, mods)
557557
case _ =>
558558
val tmpName = ctx.freshName().toTermName
559559
val patMods = mods & (AccessFlags | Lazy) | Synthetic
@@ -564,8 +564,8 @@ object desugar {
564564
val restDefs =
565565
for (((named, tpt), n) <- vars.zipWithIndex)
566566
yield
567-
if (mods is Lazy) derivedDefDef(named, tpt, selector(n), mods &~ Lazy)
568-
else derivedValDef(named, tpt, selector(n), mods)
567+
if (mods is Lazy) derivedDefDef(original, named, tpt, selector(n), mods &~ Lazy)
568+
else derivedValDef(original, named, tpt, selector(n), mods)
569569
flatTree(firstDef :: restDefs)
570570
}
571571
}
@@ -663,14 +663,18 @@ object desugar {
663663
def makeAnnotated(cls: Symbol, tree: Tree)(implicit ctx: Context) =
664664
Annotated(untpd.New(untpd.TypeTree(cls.typeRef), Nil), tree)
665665

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

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

675679
/** Main desugaring method */
676680
def apply(tree: Tree)(implicit ctx: Context): Tree = {
@@ -760,7 +764,7 @@ object desugar {
760764
*/
761765
def makeLambda(pat: Tree, body: Tree): Tree = pat match {
762766
case VarPattern(named, tpt) =>
763-
Function(derivedValDef(named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body)
767+
Function(derivedValDef(pat, named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body)
764768
case _ =>
765769
makeCaseLambda(CaseDef(pat, EmptyTree, body) :: Nil, unchecked = false)
766770
}
@@ -863,7 +867,7 @@ object desugar {
863867
val rhss = valeqs map { case GenAlias(_, rhs) => rhs }
864868
val (defpat0, id0) = makeIdPat(pat)
865869
val (defpats, ids) = (pats map makeIdPat).unzip
866-
val pdefs = (defpats, rhss).zipped map (makePatDef(Modifiers(), _, _))
870+
val pdefs = (valeqs, defpats, rhss).zipped.map(makePatDef(_, Modifiers(), _, _))
867871
val rhs1 = makeFor(nme.map, nme.flatMap, GenFrom(defpat0, rhs) :: Nil, Block(pdefs, makeTuple(id0 :: ids)))
868872
val allpats = pat :: pats
869873
val vfrom1 = new IrrefutableGenFrom(makeTuple(allpats), rhs1)
@@ -946,7 +950,7 @@ object desugar {
946950
makeFor(nme.map, nme.flatMap, enums, body) orElse tree
947951
case PatDef(mods, pats, tpt, rhs) =>
948952
val pats1 = if (tpt.isEmpty) pats else pats map (Typed(_, tpt))
949-
flatTree(pats1 map (makePatDef(mods, _, rhs)))
953+
flatTree(pats1 map (makePatDef(tree, mods, _, rhs)))
950954
case ParsedTry(body, handler, finalizer) =>
951955
handler match {
952956
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)