From 1146be7f53fa51d5c2d18d7b57abedc1fa76119e Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Tue, 27 Sep 2016 09:39:32 +0200 Subject: [PATCH 1/5] Add test of pickling positions under -Ytest-pickler --- .../tools/dotc/config/ScalaSettings.scala | 1 - src/dotty/tools/dotc/core/Phases.scala | 9 +++++-- src/dotty/tools/dotc/transform/Pickler.scala | 24 +++++++++++++------ src/dotty/tools/dotc/util/Positions.scala | 2 +- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/dotty/tools/dotc/config/ScalaSettings.scala b/src/dotty/tools/dotc/config/ScalaSettings.scala index ff17a993991a..b2e9f8c24e99 100644 --- a/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -164,7 +164,6 @@ class ScalaSettings extends Settings.SettingGroup { val YforceSbtPhases = BooleanSetting("-Yforce-sbt-phases", "Run the phases used by sbt for incremental compilation (ExtractDependencies and ExtractAPI) even if the compiler is ran outside of sbt, for debugging.") val YdumpSbtInc = BooleanSetting("-Ydump-sbt-inc", "For every compiled foo.scala, output the API representation and dependencies used for sbt incremental compilation in foo.inc, implies -Yforce-sbt-phases.") val YcheckAllPatmat = BooleanSetting("-Ycheck-all-patmat", "Check exhaustivity and redundancy of all pattern matching (used for testing the algorithm)") - def stop = YstopAfter /** Area-specific debug output. */ diff --git a/src/dotty/tools/dotc/core/Phases.scala b/src/dotty/tools/dotc/core/Phases.scala index 222e2235d6c6..de4f092cc32a 100644 --- a/src/dotty/tools/dotc/core/Phases.scala +++ b/src/dotty/tools/dotc/core/Phases.scala @@ -79,8 +79,13 @@ object Phases { * Each TreeTransform gets own period, * whereas a combined TreeTransformer gets period equal to union of periods of it's TreeTransforms */ - def squashPhases(phasess: List[List[Phase]], - phasesToSkip: List[String], stopBeforePhases: List[String], stopAfterPhases: List[String], YCheckAfter: List[String]): List[Phase] = { + def squashPhases( + phasess: List[List[Phase]], + phasesToSkip: List[String], + stopBeforePhases: List[String], + stopAfterPhases: List[String], + YCheckAfter: List[String] + ): List[Phase] = { val squashedPhases = ListBuffer[Phase]() var prevPhases: Set[Class[_ <: Phase]] = Set.empty val YCheckAll = YCheckAfter.contains("all") diff --git a/src/dotty/tools/dotc/transform/Pickler.scala b/src/dotty/tools/dotc/transform/Pickler.scala index 90e62b65c82e..21c11e1ebd78 100644 --- a/src/dotty/tools/dotc/transform/Pickler.scala +++ b/src/dotty/tools/dotc/transform/Pickler.scala @@ -12,6 +12,7 @@ import Phases._ import Symbols._ import Flags.Module import collection.mutable +import util.Positions.Position /** This phase pickles trees */ class Pickler extends Phase { @@ -25,7 +26,7 @@ class Pickler extends Phase { s.close } - private val beforePickling = new mutable.HashMap[ClassSymbol, String] + private val beforePickling = new mutable.HashMap[ClassSymbol, (String, Position)] /** Drop any elements of this list that are linked module classes of other elements in the list */ private def dropCompanionModuleClasses(clss: List[ClassSymbol])(implicit ctx: Context): List[ClassSymbol] = { @@ -40,7 +41,7 @@ class Pickler extends Phase { for { cls <- dropCompanionModuleClasses(topLevelClasses(unit.tpdTree)) tree <- sliceTopLevel(unit.tpdTree, cls) } { - if (ctx.settings.YtestPickler.value) beforePickling(cls) = tree.show + if (ctx.settings.YtestPickler.value) beforePickling(cls) = (tree.show, tree.pos) val pickler = new TastyPickler() unit.picklers += (cls -> pickler) val treePkl = pickler.treePkl @@ -73,20 +74,22 @@ class Pickler extends Phase { private def testUnpickler(units: List[CompilationUnit])(implicit ctx: Context): Unit = { pickling.println(i"testing unpickler at run ${ctx.runId}") ctx.initialize() - val unpicklers = + val unpicklers: List[(ClassSymbol, DottyUnpickler)] = for (unit <- units; (cls, pickler) <- unit.picklers) yield { val unpickler = new DottyUnpickler(pickler.assembleParts()) unpickler.enter(roots = Set()) cls -> unpickler } - pickling.println("************* entered toplevel ***********") + pickling.println("*********** Entered toplevel ***********") for ((cls, unpickler) <- unpicklers) { - val unpickled = unpickler.body(ctx.addMode(Mode.ReadPositions)) - testSame(i"$unpickled%\n%", beforePickling(cls), cls) + val ((unpickled: Tree) :: Nil) = unpickler.body(ctx.addMode(Mode.ReadPositions)) + val (beforeShow, beforePos) = beforePickling(cls) + testSameShow(unpickled.show, beforeShow, cls) + testSamePos(unpickled.pos, beforePos, cls) } } - private def testSame(unpickled: String, previous: String, cls: ClassSymbol)(implicit ctx: Context) = + private def testSameShow(unpickled: String, previous: String, cls: ClassSymbol)(implicit ctx: Context): Unit = if (previous != unpickled) { output("before-pickling.txt", previous) output("after-pickling.txt", unpickled) @@ -94,4 +97,11 @@ class Pickler extends Phase { | | diff before-pickling.txt after-pickling.txt""") } + + // Ignoring Position#point which are not pickled. + private def testSamePos(unpickled: Position, previous: Position, cls: ClassSymbol)(implicit ctx: Context): Unit = + if (unpickled.start != previous.start || unpickled.end != previous.end) { + ctx.error(i"""pickling difference in ${cls.fullName} in ${cls.sourceFile} positions: + |previous positions $previous unpickled as $unpickled""") + } } diff --git a/src/dotty/tools/dotc/util/Positions.scala b/src/dotty/tools/dotc/util/Positions.scala index c3890cc9a463..6da3c1151b1c 100644 --- a/src/dotty/tools/dotc/util/Positions.scala +++ b/src/dotty/tools/dotc/util/Positions.scala @@ -12,7 +12,7 @@ import language.implicitConversions object Positions { private val StartEndBits = 26 - val StartEndMask: Long = (1L << StartEndBits) - 1 + val StartEndMask: Long = (1L << StartEndBits) - 1 private val SyntheticPointDelta = (1 << (64 - StartEndBits * 2)) - 1 /** The maximal representable offset in a position */ From 39cf8054f2421eaca9a37b29a4e2cfbf6750c563 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Fri, 7 Oct 2016 16:40:42 +0200 Subject: [PATCH 2/5] Revert "Add test of pickling positions under -Ytest-pickler" This reverts commit ed7aeebd1425eddc0d0bf5bfab5340675cabb51b. Conflicts: src/dotty/tools/dotc/transform/Pickler.scala --- .../tools/dotc/config/ScalaSettings.scala | 1 + src/dotty/tools/dotc/core/Phases.scala | 9 ++----- src/dotty/tools/dotc/transform/Pickler.scala | 24 ++++++------------- src/dotty/tools/dotc/util/Positions.scala | 2 +- 4 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/dotty/tools/dotc/config/ScalaSettings.scala b/src/dotty/tools/dotc/config/ScalaSettings.scala index b2e9f8c24e99..ff17a993991a 100644 --- a/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -164,6 +164,7 @@ class ScalaSettings extends Settings.SettingGroup { val YforceSbtPhases = BooleanSetting("-Yforce-sbt-phases", "Run the phases used by sbt for incremental compilation (ExtractDependencies and ExtractAPI) even if the compiler is ran outside of sbt, for debugging.") val YdumpSbtInc = BooleanSetting("-Ydump-sbt-inc", "For every compiled foo.scala, output the API representation and dependencies used for sbt incremental compilation in foo.inc, implies -Yforce-sbt-phases.") val YcheckAllPatmat = BooleanSetting("-Ycheck-all-patmat", "Check exhaustivity and redundancy of all pattern matching (used for testing the algorithm)") + def stop = YstopAfter /** Area-specific debug output. */ diff --git a/src/dotty/tools/dotc/core/Phases.scala b/src/dotty/tools/dotc/core/Phases.scala index de4f092cc32a..222e2235d6c6 100644 --- a/src/dotty/tools/dotc/core/Phases.scala +++ b/src/dotty/tools/dotc/core/Phases.scala @@ -79,13 +79,8 @@ object Phases { * Each TreeTransform gets own period, * whereas a combined TreeTransformer gets period equal to union of periods of it's TreeTransforms */ - def squashPhases( - phasess: List[List[Phase]], - phasesToSkip: List[String], - stopBeforePhases: List[String], - stopAfterPhases: List[String], - YCheckAfter: List[String] - ): List[Phase] = { + def squashPhases(phasess: List[List[Phase]], + phasesToSkip: List[String], stopBeforePhases: List[String], stopAfterPhases: List[String], YCheckAfter: List[String]): List[Phase] = { val squashedPhases = ListBuffer[Phase]() var prevPhases: Set[Class[_ <: Phase]] = Set.empty val YCheckAll = YCheckAfter.contains("all") diff --git a/src/dotty/tools/dotc/transform/Pickler.scala b/src/dotty/tools/dotc/transform/Pickler.scala index 21c11e1ebd78..90e62b65c82e 100644 --- a/src/dotty/tools/dotc/transform/Pickler.scala +++ b/src/dotty/tools/dotc/transform/Pickler.scala @@ -12,7 +12,6 @@ import Phases._ import Symbols._ import Flags.Module import collection.mutable -import util.Positions.Position /** This phase pickles trees */ class Pickler extends Phase { @@ -26,7 +25,7 @@ class Pickler extends Phase { s.close } - private val beforePickling = new mutable.HashMap[ClassSymbol, (String, Position)] + private val beforePickling = new mutable.HashMap[ClassSymbol, String] /** Drop any elements of this list that are linked module classes of other elements in the list */ private def dropCompanionModuleClasses(clss: List[ClassSymbol])(implicit ctx: Context): List[ClassSymbol] = { @@ -41,7 +40,7 @@ class Pickler extends Phase { for { cls <- dropCompanionModuleClasses(topLevelClasses(unit.tpdTree)) tree <- sliceTopLevel(unit.tpdTree, cls) } { - if (ctx.settings.YtestPickler.value) beforePickling(cls) = (tree.show, tree.pos) + if (ctx.settings.YtestPickler.value) beforePickling(cls) = tree.show val pickler = new TastyPickler() unit.picklers += (cls -> pickler) val treePkl = pickler.treePkl @@ -74,22 +73,20 @@ class Pickler extends Phase { private def testUnpickler(units: List[CompilationUnit])(implicit ctx: Context): Unit = { pickling.println(i"testing unpickler at run ${ctx.runId}") ctx.initialize() - val unpicklers: List[(ClassSymbol, DottyUnpickler)] = + val unpicklers = for (unit <- units; (cls, pickler) <- unit.picklers) yield { val unpickler = new DottyUnpickler(pickler.assembleParts()) unpickler.enter(roots = Set()) cls -> unpickler } - pickling.println("*********** Entered toplevel ***********") + pickling.println("************* entered toplevel ***********") for ((cls, unpickler) <- unpicklers) { - val ((unpickled: Tree) :: Nil) = unpickler.body(ctx.addMode(Mode.ReadPositions)) - val (beforeShow, beforePos) = beforePickling(cls) - testSameShow(unpickled.show, beforeShow, cls) - testSamePos(unpickled.pos, beforePos, cls) + val unpickled = unpickler.body(ctx.addMode(Mode.ReadPositions)) + testSame(i"$unpickled%\n%", beforePickling(cls), cls) } } - private def testSameShow(unpickled: String, previous: String, cls: ClassSymbol)(implicit ctx: Context): Unit = + private def testSame(unpickled: String, previous: String, cls: ClassSymbol)(implicit ctx: Context) = if (previous != unpickled) { output("before-pickling.txt", previous) output("after-pickling.txt", unpickled) @@ -97,11 +94,4 @@ class Pickler extends Phase { | | diff before-pickling.txt after-pickling.txt""") } - - // Ignoring Position#point which are not pickled. - private def testSamePos(unpickled: Position, previous: Position, cls: ClassSymbol)(implicit ctx: Context): Unit = - if (unpickled.start != previous.start || unpickled.end != previous.end) { - ctx.error(i"""pickling difference in ${cls.fullName} in ${cls.sourceFile} positions: - |previous positions $previous unpickled as $unpickled""") - } } diff --git a/src/dotty/tools/dotc/util/Positions.scala b/src/dotty/tools/dotc/util/Positions.scala index 6da3c1151b1c..c3890cc9a463 100644 --- a/src/dotty/tools/dotc/util/Positions.scala +++ b/src/dotty/tools/dotc/util/Positions.scala @@ -12,7 +12,7 @@ import language.implicitConversions object Positions { private val StartEndBits = 26 - val StartEndMask: Long = (1L << StartEndBits) - 1 + val StartEndMask: Long = (1L << StartEndBits) - 1 private val SyntheticPointDelta = (1 << (64 - StartEndBits * 2)) - 1 /** The maximal representable offset in a position */ From 236a0f8741c05cc57b98f3f4c4cd54d5b07cede1 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Thu, 6 Oct 2016 16:04:34 +0200 Subject: [PATCH 3/5] Document and clean up returns in PlainPrinter/RefinedPrinter --- .../tools/dotc/printing/PlainPrinter.scala | 4 ++ .../tools/dotc/printing/RefinedPrinter.scala | 48 ++++++++++--------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/dotty/tools/dotc/printing/PlainPrinter.scala b/src/dotty/tools/dotc/printing/PlainPrinter.scala index a92095d9bd15..3701e33e9ffd 100644 --- a/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -11,6 +11,10 @@ import java.lang.Integer.toOctalString import config.Config.summarizeDepth import scala.annotation.switch +/** + * Pretty printer with minimal formating. Similar to `.toString` but with proper indentation, + * position and a few more details. Accessible under `-Yplain-printer`. + */ class PlainPrinter(_ctx: Context) extends Printer { protected[this] implicit def ctx: Context = _ctx.addMode(Mode.Printing) diff --git a/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 4f3a8d272597..ee846bea97ba 100644 --- a/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -16,6 +16,7 @@ import config.Config import scala.annotation.switch import language.implicitConversions +/** Pretty printer used by default with `.show`, generate strings very close to scala source code. */ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { /** A stack of enclosing DefDef, TypeDef, or ClassDef, or ModuleDefs nodes */ @@ -64,12 +65,13 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { override def toTextRef(tp: SingletonType): Text = controlled { tp match { - case tp: ThisType => - if (tp.cls.isAnonymousClass) return "this" - if (tp.cls is ModuleClass) return fullNameString(tp.cls.sourceModule) + case tp: ThisType if tp.cls.isAnonymousClass => + "this" + case tp: ThisType if tp.cls is ModuleClass => + fullNameString(tp.cls.sourceModule) case _ => + super.toTextRef(tp) } - super.toTextRef(tp) } override def toTextPrefix(tp: Type): Text = controlled { @@ -113,44 +115,46 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { homogenize(tp) match { case AppliedType(tycon, args) => val cls = tycon.typeSymbol - if (tycon.isRepeatedParam) return toTextLocal(args.head) ~ "*" - if (defn.isFunctionClass(cls)) return toTextFunction(args) - if (defn.isTupleClass(cls)) return toTextTuple(args) - return (toTextLocal(tycon) ~ "[" ~ Text(args map argText, ", ") ~ "]").close + if (tycon.isRepeatedParam) toTextLocal(args.head) ~ "*" + else if (defn.isFunctionClass(cls)) toTextFunction(args) + else if (defn.isTupleClass(cls)) toTextTuple(args) + else (toTextLocal(tycon) ~ "[" ~ Text(args map argText, ", ") ~ "]").close case tp: TypeRef => val hideType = tp.symbol is AliasPreferred if (hideType && !ctx.phase.erasedTypes && !tp.symbol.isCompleting) { tp.info match { - case TypeAlias(alias) => return toText(alias) - case _ => if (tp.prefix.isInstanceOf[ThisType]) return nameString(tp.symbol) + case TypeAlias(alias) => toText(alias) + case _ if (tp.prefix.isInstanceOf[ThisType]) => nameString(tp.symbol) + case _ => super.toText(tp) } - } - else if (tp.symbol.isAnonymousClass && !ctx.settings.uniqid.value) - return toText(tp.info) + } else if (tp.symbol.isAnonymousClass && !ctx.settings.uniqid.value) + toText(tp.info) + else + super.toText(tp) case ExprType(result) => - return "=> " ~ toText(result) + "=> " ~ toText(result) case ErasedValueType(tycon, underlying) => - return "ErasedValueType(" ~ toText(tycon) ~ ", " ~ toText(underlying) ~ ")" + "ErasedValueType(" ~ toText(tycon) ~ ", " ~ toText(underlying) ~ ")" case tp: ClassInfo => - return toTextParents(tp.parentsWithArgs) ~ "{...}" + toTextParents(tp.parentsWithArgs) ~ "{...}" case JavaArrayType(elemtp) => - return toText(elemtp) ~ "[]" + toText(elemtp) ~ "[]" case tp: SelectionProto => - return "?{ " ~ toText(tp.name) ~ (" " provided !tp.name.decode.last.isLetterOrDigit) ~ + "?{ " ~ toText(tp.name) ~ (" " provided !tp.name.decode.last.isLetterOrDigit) ~ ": " ~ toText(tp.memberProto) ~ " }" case tp: ViewProto => - return toText(tp.argType) ~ " ?=>? " ~ toText(tp.resultType) + toText(tp.argType) ~ " ?=>? " ~ toText(tp.resultType) case tp @ FunProto(args, resultType, _) => val argsText = args match { case dummyTreeOfType(tp) :: Nil if !(tp isRef defn.NullClass) => "null: " ~ toText(tp) case _ => toTextGlobal(args, ", ") } - return "FunProto(" ~ argsText ~ "):" ~ toText(resultType) + "FunProto(" ~ argsText ~ "):" ~ toText(resultType) case tp: IgnoredProto => - return "?" + "?" case _ => + super.toText(tp) } - super.toText(tp) } def blockText[T >: Untyped](trees: List[Tree[T]]): Text = From 1954b8b09d6378687d7b8e6985f8dbc0dadc61dd Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Fri, 7 Oct 2016 16:49:27 +0200 Subject: [PATCH 4/5] Add simplified toString on Positions which matches pickled data --- src/dotty/tools/dotc/printing/RefinedPrinter.scala | 2 +- src/dotty/tools/dotc/util/Positions.scala | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/src/dotty/tools/dotc/printing/RefinedPrinter.scala index ee846bea97ba..5ec823276b12 100644 --- a/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -531,7 +531,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { else if (!tree.isDef) txt = ("<" ~ txt ~ ":" ~ toText(tp) ~ ">").close } if (ctx.settings.Yprintpos.value && !tree.isInstanceOf[WithoutTypeOrPos[_]]) - txt = txt ~ "@" ~ tree.pos.toString + txt = txt ~ "@" ~ tree.pos.toString(simplified = true) tree match { case Block(_, _) | Template(_, _, _, _) => txt case _ => txt.close diff --git a/src/dotty/tools/dotc/util/Positions.scala b/src/dotty/tools/dotc/util/Positions.scala index c3890cc9a463..c718db02568a 100644 --- a/src/dotty/tools/dotc/util/Positions.scala +++ b/src/dotty/tools/dotc/util/Positions.scala @@ -108,10 +108,12 @@ object Positions { /** A synthetic copy of this position */ def toSynthetic = if (isSynthetic) this else Position(start, end) - override def toString = { - val (left, right) = if (isSynthetic) ("<", ">") else ("[", "]") + override def toString: String = toString(simplified = false) + + def toString(simplified: Boolean): String = { + val (left, right) = if (simplified || isSynthetic) ("<", ">") else ("[", "]") if (exists) - s"$left$start..${if (point == start) "" else s"$point.."}$end$right" + s"$left$start..${if (simplified || point == start) "" else s"$point.."}$end$right" else s"${left}no position${right}" } From b6df675cf161e17e48426af4c3fe95c214440fd1 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Fri, 7 Oct 2016 16:49:25 +0200 Subject: [PATCH 5/5] Add -Yprintpos in pickling tests --- test/dotc/tests.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index f161fefe3b4b..4975003e64c8 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -28,7 +28,7 @@ class tests extends CompilerTest { else List("-Ycheck:tailrec,resolveSuper,mixin,restoreScopes,labelDef") } - val testPickling = List("-Xprint-types", "-Ytest-pickler", "-Ystop-after:pickler") + val testPickling = List("-Xprint-types", "-Yprintpos", "-Ytest-pickler", "-Ystop-after:pickler") val twice = List("#runs", "2") val staleSymbolError: List[String] = List()