From 0152dbe969520914ce1730c4a81597bc362c9c5b Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Tue, 15 May 2012 09:40:23 +0200 Subject: Fixed SI-5603. Early definitions now get transparent positions. This includes a fix for a minor problem described in #594 - ensureNonOverlapping still focuses on default position when outside of early defs. Review by @dragos, @odersky. --- src/compiler/scala/tools/nsc/ast/Trees.scala | 13 +++--- .../tools/nsc/interactive/RangePositions.scala | 48 ++++++++++------------ src/reflect/scala/reflect/internal/Positions.scala | 14 ++++--- test/files/run/t5603.check | 29 +++++++++++++ test/files/run/t5603.scala | 42 +++++++++++++++++++ 5 files changed, 107 insertions(+), 39 deletions(-) create mode 100644 test/files/run/t5603.check create mode 100644 test/files/run/t5603.scala diff --git a/src/compiler/scala/tools/nsc/ast/Trees.scala b/src/compiler/scala/tools/nsc/ast/Trees.scala index 1d29e33c50..9a6d32be01 100644 --- a/src/compiler/scala/tools/nsc/ast/Trees.scala +++ b/src/compiler/scala/tools/nsc/ast/Trees.scala @@ -95,11 +95,12 @@ trait Trees extends reflect.internal.Trees { self: Global => val (edefs, rest) = body span treeInfo.isEarlyDef val (evdefs, etdefs) = edefs partition treeInfo.isEarlyValDef val gvdefs = evdefs map { - case vdef @ ValDef(_, _, tpt, _) => copyValDef(vdef)( - // !!! I know "atPos in case" wasn't intentionally planted to - // add an air of mystery to this file, but it is the sort of - // comment which only its author could love. - tpt = atPos(vdef.pos.focus)(TypeTree() setOriginal tpt setPos tpt.pos.focus), // atPos in case + case vdef @ ValDef(_, _, tpt, _) => + copyValDef(vdef)( + // atPos for the new tpt is necessary, since the original tpt might have no position + // (when missing type annotation for ValDef for example), so even though setOriginal modifies the + // position of TypeTree, it would still be NoPosition. That's what the author meant. + tpt = atPos(vdef.pos.focus)(TypeTree() setOriginal tpt setPos tpt.pos.focus), rhs = EmptyTree ) } @@ -125,7 +126,7 @@ trait Trees extends reflect.internal.Trees { self: Global => DefDef(constrMods, nme.CONSTRUCTOR, List(), vparamss1, TypeTree(), Block(lvdefs ::: List(superCall), Literal(Constant()))))) } } - constrs foreach (ensureNonOverlapping(_, parents ::: gvdefs)) + constrs foreach (ensureNonOverlapping(_, parents ::: gvdefs, focus=false)) // Field definitions for the class - remove defaults. val fieldDefs = vparamss.flatten map (vd => copyValDef(vd)(mods = vd.mods &~ DEFAULTPARAM, rhs = EmptyTree)) diff --git a/src/compiler/scala/tools/nsc/interactive/RangePositions.scala b/src/compiler/scala/tools/nsc/interactive/RangePositions.scala index 06828f3a3a..b702d2787c 100644 --- a/src/compiler/scala/tools/nsc/interactive/RangePositions.scala +++ b/src/compiler/scala/tools/nsc/interactive/RangePositions.scala @@ -41,11 +41,11 @@ self: scala.tools.nsc.Global => /** A position that wraps a set of trees. * The point of the wrapping position is the point of the default position. * If some of the trees are ranges, returns a range position enclosing all ranges - * Otherwise returns default position. + * Otherwise returns default position that is either focused or not. */ - override def wrappingPos(default: Position, trees: List[Tree]): Position = { + override def wrappingPos(default: Position, trees: List[Tree], focus: Boolean): Position = { val ranged = trees filter (_.pos.isRange) - if (ranged.isEmpty) default.focus + if (ranged.isEmpty) if (focus) default.focus else default else new RangePosition(default.source, (ranged map (_.pos.start)).min, default.point, (ranged map (_.pos.end)).max) } @@ -59,13 +59,25 @@ self: scala.tools.nsc.Global => if (headpos.isDefined) wrappingPos(headpos, trees) else headpos } -/* - override def integratePos(tree: Tree, pos: Position) = - if (pos.isSynthetic && !tree.pos.isSynthetic) tree.syntheticDuplicate - else tree -*/ - // -------------- ensuring no overlaps ------------------------------- + + /** Ensure that given tree has no positions that overlap with + * any of the positions of `others`. This is done by + * shortening the range, assigning TransparentPositions + * to some of the nodes in `tree` or focusing on the position. + */ + override def ensureNonOverlapping(tree: Tree, others: List[Tree], focus: Boolean) { + def isOverlapping(pos: Position) = + pos.isRange && (others exists (pos overlaps _.pos)) + if (isOverlapping(tree.pos)) { + val children = tree.children + children foreach (ensureNonOverlapping(_, others, focus)) + if (tree.pos.isOpaqueRange) { + val wpos = wrappingPos(tree.pos, children, focus) + tree setPos (if (isOverlapping(wpos)) tree.pos.makeTransparent else wpos) + } + } + } def solidDescendants(tree: Tree): List[Tree] = if (tree.pos.isTransparent) tree.children flatMap solidDescendants @@ -106,24 +118,6 @@ self: scala.tools.nsc.Global => if (ts.head == t) replacement ::: ts.tail else ts.head :: replace(ts.tail, t, replacement) - /** Ensure that given tree has no positions that overlap with - * any of the positions of `others`. This is done by - * shortening the range or assigning TransparentPositions - * to some of the nodes in `tree`. - */ - override def ensureNonOverlapping(tree: Tree, others: List[Tree]) { - def isOverlapping(pos: Position) = - pos.isRange && (others exists (pos overlaps _.pos)) - if (isOverlapping(tree.pos)) { - val children = tree.children - children foreach (ensureNonOverlapping(_, others)) - if (tree.pos.isOpaqueRange) { - val wpos = wrappingPos(tree.pos.focus, children) - tree setPos (if (isOverlapping(wpos)) tree.pos.makeTransparent else wpos) - } - } - } - /** Does given list of trees have mutually non-overlapping positions? * pre: None of the trees is transparent */ diff --git a/src/reflect/scala/reflect/internal/Positions.scala b/src/reflect/scala/reflect/internal/Positions.scala index 6ae9b40fcb..faa161d6b1 100644 --- a/src/reflect/scala/reflect/internal/Positions.scala +++ b/src/reflect/scala/reflect/internal/Positions.scala @@ -10,23 +10,25 @@ trait Positions extends api.Positions { self: SymbolTable => /** A position that wraps a set of trees. * The point of the wrapping position is the point of the default position. * If some of the trees are ranges, returns a range position enclosing all ranges - * Otherwise returns default position. + * Otherwise returns default position that is either focused or not. */ - def wrappingPos(default: Position, trees: List[Tree]): Position = default + def wrappingPos(default: Position, trees: List[Tree]) = wrappingPos(default, trees, true) + def wrappingPos(default: Position, trees: List[Tree], focus: Boolean): Position = default /** A position that wraps the non-empty set of trees. * The point of the wrapping position is the point of the first trees' position. - * If all some the trees are non-synthetic, returns a range position enclosing the non-synthetic trees + * If some of the trees are non-synthetic, returns a range position enclosing the non-synthetic trees * Otherwise returns a synthetic offset position to point. */ def wrappingPos(trees: List[Tree]): Position = trees.head.pos /** Ensure that given tree has no positions that overlap with * any of the positions of `others`. This is done by - * shortening the range or assigning TransparentPositions - * to some of the nodes in `tree`. + * shortening the range, assigning TransparentPositions + * to some of the nodes in `tree` or focusing on the position. */ - def ensureNonOverlapping(tree: Tree, others: List[Tree]) {} + def ensureNonOverlapping(tree: Tree, others: List[Tree]){ ensureNonOverlapping(tree, others, true) } + def ensureNonOverlapping(tree: Tree, others: List[Tree], focus: Boolean) {} trait PosAssigner extends Traverser { var pos: Position diff --git a/test/files/run/t5603.check b/test/files/run/t5603.check new file mode 100644 index 0000000000..5127d3c1c7 --- /dev/null +++ b/test/files/run/t5603.check @@ -0,0 +1,29 @@ +[[syntax trees at end of parser]] // newSource1 +[0:241]package [0:0] { + [0:82]abstract trait Greeting extends [15:82][83]scala.AnyRef { + [15]def $init$() = [15]{ + [15]() + }; + [23:39]val name: [33:39]String; + [46:76]val msg = [56:76][56:72][56:71]"How are you, ".$plus([72:76]name) + }; + [87:209]class C extends [94:209][151:159]Greeting { + [119:139]val nameElse = _; + [95:101] private[this] val i: [98:101]Int = _; + <119:139>def ([95]i: [98]Int) = <119:139>{ + <119:139>val nameElse = <134:139>"Bob"; + [94][94][94]super.(); + [94]() + }; + [168:184]val name = [179:184]"avc"; + [191:203][191:198]println([199:202]msg) + }; + [215:241]object Test extends [227:241][235:238]App { + [227]def () = [227]{ + [227][227][227]super.(); + [227]() + }; + [NoPosition] + } +} + diff --git a/test/files/run/t5603.scala b/test/files/run/t5603.scala new file mode 100644 index 0000000000..60dfd01fee --- /dev/null +++ b/test/files/run/t5603.scala @@ -0,0 +1,42 @@ +import scala.tools.partest._ +import java.io._ +import scala.tools.nsc._ +import scala.tools.nsc.util.CommandLineParser +import scala.tools.nsc.{Global, Settings, CompilerCommand} +import scala.tools.nsc.reporters.ConsoleReporter + +object Test extends DirectTest { + + override def extraSettings: String = "-usejavacp -Xprint:parser -Ystop-after:parser -d " + testOutput.path + + override def code = """ + trait Greeting { + val name: String + val msg = "How are you, "+name + } + class C(i: Int) extends { + val nameElse = "Bob" + } with Greeting { + val name = "avc" + println(msg) + } + + object Test extends App {} + """.trim + + override def show(): Unit = { + // redirect err to out, for logging + val prevErr = System.err + System.setErr(System.out) + compile() + System.setErr(prevErr) + } + + override def newCompiler(args: String*): Global = { + + val settings = new Settings() + settings.Xprintpos.value = true + val command = new CompilerCommand((CommandLineParser tokenize extraSettings) ++ args.toList, settings) + new Global(command.settings, new ConsoleReporter(settings)) with interactive.RangePositions + } +} -- cgit v1.2.3