From 586527d5855b56bc7e1719762169adb2c466c79d Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 1 Aug 2018 20:49:09 +0200 Subject: [PATCH 01/13] Seal Texts.Text --- compiler/src/dotty/tools/dotc/printing/Texts.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/printing/Texts.scala b/compiler/src/dotty/tools/dotc/printing/Texts.scala index 41e91a5957a1..d1758847ca20 100644 --- a/compiler/src/dotty/tools/dotc/printing/Texts.scala +++ b/compiler/src/dotty/tools/dotc/printing/Texts.scala @@ -4,7 +4,7 @@ import language.implicitConversions object Texts { - abstract class Text { + sealed abstract class Text { protected def indentMargin = 2 @@ -46,9 +46,11 @@ object Texts { case Str(s1, lines2) => Str(s1 + s2, lines1 union lines2) case Fluid(Str(s1, lines2) :: prev) => Fluid(Str(s1 + s2, lines1 union lines2) :: prev) case Fluid(relems) => Fluid(that :: relems) + case Vertical(_) => throw new IllegalArgumentException("Unexpected Vertical.appendToLastLine") } case Fluid(relems) => (this /: relems.reverse)(_ appendToLastLine _) + case Vertical(_) => throw new IllegalArgumentException("Unexpected Text.appendToLastLine(Vertical(...))") } private def appendIndented(that: Text)(width: Int): Text = From dac426543cecfbc6c2452755951dc6b98e7a8a6f Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Sat, 4 Aug 2018 11:27:55 +0200 Subject: [PATCH 02/13] Bugfix: missing return --- compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index cc6416df3f5e..169b023232c9 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -201,7 +201,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { // (they don't need to because we keep the original type tree with // the original annotation anyway. Therefore, there will always be // one version of the annotation tree that has the correct positions). - withoutPos(super.toText(tp)) + return withoutPos(super.toText(tp)) case tp: SelectionProto => return "?{ " ~ toText(tp.name) ~ (Str(" ") provided !tp.name.toSimpleName.last.isLetterOrDigit) ~ From df2189bd4107102887d197a7e1aa8981af079580 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 30 Jul 2018 23:17:18 +0200 Subject: [PATCH 03/13] Refactor argText `argText` uses `toTextGlobal` to change priority; instead, call `toText(arg)` and leave the priority up to the caller. - call-sites where `GlobalPrec` makes sense use now `argsText`. - to ensure this is a refactoring, other call-sites now use `atPrec(GlobalPrec)` explicitly; all those calls will be corrected in subsequent commits. --- .../src/dotty/tools/dotc/printing/PlainPrinter.scala | 11 ++++++++--- .../dotty/tools/dotc/printing/RefinedPrinter.scala | 8 ++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 51f95d94f794..85d44a11b5a9 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -102,10 +102,15 @@ class PlainPrinter(_ctx: Context) extends Printer { (refinementNameString(rt) ~ toTextRHS(rt.refinedInfo)).close protected def argText(arg: Type): Text = homogenizeArg(arg) match { - case arg: TypeBounds => "_" ~ toTextGlobal(arg) - case arg => toTextGlobal(arg) + case arg: TypeBounds => "_" ~ toText(arg) + case arg => toText(arg) } + /** Pretty-print comma-separated type arguments for a constructor to be inserted among parentheses or brackets + * (hence with `GlobalPrec` precedence). + */ + protected def argsText(args: List[Type]): Text = atPrec(GlobalPrec) { Text(args.map(arg => argText(arg) ), ", ") } + /** The longest sequence of refinement types, starting at given type * and following parents. */ @@ -144,7 +149,7 @@ class PlainPrinter(_ctx: Context) extends Printer { case tp: SingletonType => toTextLocal(tp.underlying) ~ "(" ~ toTextRef(tp) ~ ")" case AppliedType(tycon, args) => - (toTextLocal(tycon) ~ "[" ~ Text(args map argText, ", ") ~ "]").close + (toTextLocal(tycon) ~ "[" ~ argsText(args) ~ "]").close case tp: RefinedType => val parent :: (refined: List[RefinedType @unchecked]) = refinementChain(tp).reverse diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 169b023232c9..3895c9fc7c85 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -130,13 +130,13 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { override def toText(tp: Type): Text = controlled { def toTextTuple(args: List[Type]): Text = - "(" ~ Text(args.map(argText), ", ") ~ ")" + "(" ~ argsText(args) ~ ")" def toTextFunction(args: List[Type], isImplicit: Boolean, isErased: Boolean): Text = changePrec(GlobalPrec) { val argStr: Text = if (args.length == 2 && !defn.isTupleType(args.head)) - atPrec(InfixPrec) { argText(args.head) } + atPrec(InfixPrec) { atPrec(GlobalPrec) { argText(args.head) } } else toTextTuple(args.init) (keywordText("erased ") provided isErased) ~ (keywordText("implicit ") provided isImplicit) ~ argStr ~ " => " ~ argText(args.last) @@ -162,8 +162,8 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { * needs to be parenthesized if it's an infix type, and vice versa. */ val l :: r :: Nil = args val isRightAssoc = op.typeSymbol.name.endsWith(":") - val leftArg = if (isRightAssoc && isInfixType(l)) "(" ~ argText(l) ~ ")" else argText(l) - val rightArg = if (!isRightAssoc && isInfixType(r)) "(" ~ argText(r) ~ ")" else argText(r) + val leftArg = if (isRightAssoc && isInfixType(l)) "(" ~ atPrec(GlobalPrec) { argText(l) } ~ ")" else atPrec(GlobalPrec) { argText(l) } + val rightArg = if (!isRightAssoc && isInfixType(r)) "(" ~ atPrec(GlobalPrec) { argText(r) } ~ ")" else atPrec(GlobalPrec) { argText(r) } leftArg ~ " " ~ simpleNameString(op.classSymbol) ~ " " ~ rightArg } From 8d8dfffd732841fe63d62f5b281a9553c5b7392e Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 9 Aug 2018 17:37:46 +0200 Subject: [PATCH 04/13] Fix #4068: Fix pretty-printing precedence for function types Using `atPrec(GlobalPrec)` ensures that `argText(args.head)` never adds parentheses around its output, but that's inappropriate for a function type `T1 => T2`. The nested calls to `atPrec` make no sense, but `argText` hid that before it was refactored. --- .../src/dotty/tools/dotc/printing/RefinedPrinter.scala | 2 +- compiler/test-resources/type-printer/functions | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 3895c9fc7c85..a2f316c4c81c 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -136,7 +136,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { changePrec(GlobalPrec) { val argStr: Text = if (args.length == 2 && !defn.isTupleType(args.head)) - atPrec(InfixPrec) { atPrec(GlobalPrec) { argText(args.head) } } + atPrec(InfixPrec) { argText(args.head) } else toTextTuple(args.init) (keywordText("erased ") provided isErased) ~ (keywordText("implicit ") provided isImplicit) ~ argStr ~ " => " ~ argText(args.last) diff --git a/compiler/test-resources/type-printer/functions b/compiler/test-resources/type-printer/functions index 6d0bcd7e92da..05a1eda995e3 100644 --- a/compiler/test-resources/type-printer/functions +++ b/compiler/test-resources/type-printer/functions @@ -1,2 +1,10 @@ scala> val toInt: Any => Int = new { def apply(a: Any) = 1; override def toString() = "" } val toInt: Any => Int = +scala> val hoFun: (Int => Int) => Int = new { def apply(a: Int => Int) = 1; override def toString() = "" } +val hoFun: (Int => Int) => Int = +scala> val curriedFun: Int => (Int => Int) = new { def apply(a: Int) = _ => 1; override def toString() = "" } +val curriedFun: Int => Int => Int = +scala> val tupFun: ((Int, Int)) => Int = new { def apply(a: (Int, Int)) = 1; override def toString() = "" } +val tupFun: ((Int, Int)) => Int = +scala> val binFun: (Int, Int) => Int = new { def apply(a: Int, b: Int) = 1; override def toString() = "" } +val binFun: (Int, Int) => Int = From 3257bfb08cb7aa4f1a6519ed0fdf1406d22987b6 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 9 Aug 2018 17:46:40 +0200 Subject: [PATCH 05/13] Refactor: distinguish type- and term- level priority Encapsulate the precedence rules in `parsing.precedence` while allowing for different precedence between term- and type-level operators, as mandated by the spec. This is just a refactoring, the actual priority changes are separate. * Add an `isType` flag to `parsing.precedence` and add constants for `AndTypePrec` and `OrTypePrec` (distinct from `OrPrec` and `AndPrec`, which is now unused and dropped). * In the parser, consider `isType` to decide priority. * In the pretty-printer, use `AndTypePrec` and `OrTypePrec` where appropriate. --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 6 +++--- compiler/src/dotty/tools/dotc/parsing/package.scala | 2 +- compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala | 4 ++-- compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala | 4 ++-- compiler/src/dotty/tools/dotc/printing/package.scala | 5 +++-- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 991255e913fa..50c571a8b5e3 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -471,13 +471,13 @@ object Parsers { syntaxError(MixedLeftAndRightAssociativeOps(op1, op2, op2LeftAssoc), offset) def reduceStack(base: List[OpInfo], top: Tree, prec: Int, leftAssoc: Boolean, op2: Name, isType: Boolean): Tree = { - if (opStack != base && precedence(opStack.head.operator.name) == prec) + if (opStack != base && precedence(opStack.head.operator.name, isType) == prec) checkAssoc(opStack.head.offset, opStack.head.operator.name, op2, leftAssoc) def recur(top: Tree): Tree = { if (opStack == base) top else { val opInfo = opStack.head - val opPrec = precedence(opInfo.operator.name) + val opPrec = precedence(opInfo.operator.name, isType) if (prec < opPrec || leftAssoc && prec == opPrec) { opStack = opStack.tail recur { @@ -514,7 +514,7 @@ object Parsers { var top = first while (isIdent && isOperator) { val op = if (isType) typeIdent() else termIdent() - top = reduceStack(base, top, precedence(op.name), isLeftAssoc(op.name), op.name, isType) + top = reduceStack(base, top, precedence(op.name, isType), isLeftAssoc(op.name), op.name, isType) opStack = OpInfo(top, op, in.offset) :: opStack newLineOptWhenFollowing(canStartOperand) if (maybePostfix && !canStartOperand(in.token)) { diff --git a/compiler/src/dotty/tools/dotc/parsing/package.scala b/compiler/src/dotty/tools/dotc/parsing/package.scala index cdb30d0be7ed..49676e236892 100644 --- a/compiler/src/dotty/tools/dotc/parsing/package.scala +++ b/compiler/src/dotty/tools/dotc/parsing/package.scala @@ -7,7 +7,7 @@ import core.NameOps._ package object parsing { - def precedence(operator: Name): Int = + def precedence(operator: Name, isType: Boolean = false): Int = if (operator eq nme.ERROR) -1 else { val firstCh = operator.firstPart.head diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 85d44a11b5a9..98aa0598c435 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -161,9 +161,9 @@ class PlainPrinter(_ctx: Context) extends Printer { } finally openRecs = openRecs.tail case AndType(tp1, tp2) => - changePrec(AndPrec) { toText(tp1) ~ " & " ~ toText(tp2) } + changePrec(AndTypePrec) { toText(tp1) ~ " & " ~ toText(tp2) } case OrType(tp1, tp2) => - changePrec(OrPrec) { toText(tp1) ~ " | " ~ toText(tp2) } + changePrec(OrTypePrec) { toText(tp1) ~ " | " ~ toText(tp2) } case tp: ErrorType => s"" case tp: WildcardType => diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index a2f316c4c81c..abfa1c3641dd 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -375,9 +375,9 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { case SingletonTypeTree(ref) => toTextLocal(ref) ~ "." ~ keywordStr("type") case AndTypeTree(l, r) => - changePrec(AndPrec) { toText(l) ~ " & " ~ toText(r) } + changePrec(AndTypePrec) { toText(l) ~ " & " ~ toText(r) } case OrTypeTree(l, r) => - changePrec(OrPrec) { toText(l) ~ " | " ~ toText(r) } + changePrec(OrTypePrec) { toText(l) ~ " | " ~ toText(r) } case RefinedTypeTree(tpt, refines) => toTextLocal(tpt) ~ " " ~ blockText(refines) case AppliedTypeTree(tpt, args) => diff --git a/compiler/src/dotty/tools/dotc/printing/package.scala b/compiler/src/dotty/tools/dotc/printing/package.scala index c7d04cd05a3e..cb6d73e23beb 100644 --- a/compiler/src/dotty/tools/dotc/printing/package.scala +++ b/compiler/src/dotty/tools/dotc/printing/package.scala @@ -1,6 +1,6 @@ package dotty.tools.dotc -import core.StdNames.nme +import core.StdNames.{nme,tpnme} import parsing.{precedence, minPrec, maxPrec, minInfixPrec} import util.Property.Key @@ -9,7 +9,8 @@ package object printing { type Precedence = Int val DotPrec = parsing.maxPrec - val AndPrec = parsing.precedence(nme.raw.AMP) + val AndTypePrec = parsing.precedence(tpnme.raw.AMP, true) + val OrTypePrec = parsing.precedence(tpnme.raw.BAR, true) val OrPrec = parsing.precedence(nme.raw.BAR) val InfixPrec = parsing.minInfixPrec val GlobalPrec = parsing.minPrec From a87418b70acdd184745bf0c3000eb30ef9f736e1 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 9 Aug 2018 17:59:46 +0200 Subject: [PATCH 06/13] Treat & and | as *left*-associative While the operators should be associative, they aren't actually guaranteed to be, so correct pretty-printing will help users. * PrinterTests: Add first test for TypeTree printing. * PrinterTests: disable colors, following ErrorMessagesTest.scala. --- .../tools/dotc/printing/PlainPrinter.scala | 4 +-- .../tools/dotc/printing/RefinedPrinter.scala | 4 +-- .../tools/dotc/printing/PrinterTests.scala | 26 ++++++++++++++++++- .../patmat/andtype-opentype-interaction.check | 2 +- .../andtype-refinedtype-interaction.check | 4 +-- 5 files changed, 32 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 98aa0598c435..9de60b26b91c 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -161,9 +161,9 @@ class PlainPrinter(_ctx: Context) extends Printer { } finally openRecs = openRecs.tail case AndType(tp1, tp2) => - changePrec(AndTypePrec) { toText(tp1) ~ " & " ~ toText(tp2) } + changePrec(AndTypePrec) { toText(tp1) ~ " & " ~ atPrec(AndTypePrec + 1) { toText(tp2) } } case OrType(tp1, tp2) => - changePrec(OrTypePrec) { toText(tp1) ~ " | " ~ toText(tp2) } + changePrec(OrTypePrec) { toText(tp1) ~ " | " ~ atPrec(OrTypePrec + 1) { toText(tp2) } } case tp: ErrorType => s"" case tp: WildcardType => diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index abfa1c3641dd..a6978e978bd7 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -375,9 +375,9 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { case SingletonTypeTree(ref) => toTextLocal(ref) ~ "." ~ keywordStr("type") case AndTypeTree(l, r) => - changePrec(AndTypePrec) { toText(l) ~ " & " ~ toText(r) } + changePrec(AndTypePrec) { toText(l) ~ " & " ~ atPrec(AndTypePrec + 1) { toText(r) } } case OrTypeTree(l, r) => - changePrec(OrTypePrec) { toText(l) ~ " | " ~ toText(r) } + changePrec(OrTypePrec) { toText(l) ~ " | " ~ atPrec(OrTypePrec + 1) { toText(r) } } case RefinedTypeTree(tpt, refines) => toTextLocal(tpt) ~ " " ~ blockText(refines) case AppliedTypeTree(tpt, args) => diff --git a/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala b/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala index 629f8529a143..58ade561f756 100644 --- a/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala +++ b/compiler/test/dotty/tools/dotc/printing/PrinterTests.scala @@ -1,13 +1,20 @@ package dotty.tools.dotc.printing import dotty.tools.DottyTest -import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.ast.{Trees,tpd} import dotty.tools.dotc.core.Names._ import dotty.tools.dotc.core.Symbols._ +import dotty.tools.dotc.core.Decorators._ import org.junit.Assert.assertEquals import org.junit.Test class PrinterTests extends DottyTest { + + private def newContext = { + initialCtx.setSetting(ctx.settings.color, "never") + } + ctx = newContext + import tpd._ @Test @@ -24,4 +31,21 @@ class PrinterTests extends DottyTest { assertEquals("package object foo", bar.symbol.owner.show) } } + + @Test + def tpTreeInfixOps: Unit = { + val source = """ + |class &&[T,U] + |object Foo { + | def bar1: Int && (Boolean | String) = ??? + | def bar2: Int & (Boolean | String) = ??? + |} + """.stripMargin + + checkCompile("frontend", source) { (tree, context) => + implicit val ctx = context + val bar @ Trees.DefDef(_, _, _, _, _) = tree.find(tree => tree.symbol.name == termName("bar2")).get + assertEquals("Int & (Boolean | String)", bar.tpt.show) + } + } } diff --git a/tests/patmat/andtype-opentype-interaction.check b/tests/patmat/andtype-opentype-interaction.check index e38fe3ed86b6..ed0bddb70363 100644 --- a/tests/patmat/andtype-opentype-interaction.check +++ b/tests/patmat/andtype-opentype-interaction.check @@ -3,4 +3,4 @@ 31: Pattern Match Exhaustivity: _: SealedTrait & OpenClass, _: Trait & OpenClass 35: Pattern Match Exhaustivity: _: SealedTrait & OpenTrait & OpenClass, _: Trait & OpenTrait & OpenClass 43: Pattern Match Exhaustivity: _: SealedTrait & OpenAbstractClass, _: Trait & OpenAbstractClass -47: Pattern Match Exhaustivity: _: SealedTrait & OpenClass & OpenTrait & OpenClassSubclass, _: Trait & OpenClass & OpenTrait & OpenClassSubclass +47: Pattern Match Exhaustivity: _: SealedTrait & OpenClass & (OpenTrait & OpenClassSubclass), _: Trait & OpenClass & (OpenTrait & OpenClassSubclass) diff --git a/tests/patmat/andtype-refinedtype-interaction.check b/tests/patmat/andtype-refinedtype-interaction.check index 2f8687a868e5..acee175e13b4 100644 --- a/tests/patmat/andtype-refinedtype-interaction.check +++ b/tests/patmat/andtype-refinedtype-interaction.check @@ -1,6 +1,6 @@ 32: Pattern Match Exhaustivity: _: Trait & C1{x: Int} -48: Pattern Match Exhaustivity: _: Clazz & (C1 | C2 | T1){x: Int} & (C3 | C4 | T2){x: Int}, _: Trait & (C1 | C2 | T1){x: Int} & (C3 | C4 | T2){x: Int} -54: Pattern Match Exhaustivity: _: Trait & (C1 | C2 | T1){x: Int} & C3{x: Int} +48: Pattern Match Exhaustivity: _: Clazz & (C1 | (C2 | T1)){x: Int} & (C3 | (C4 | T2)){x: Int}, _: Trait & (C1 | (C2 | T1)){x: Int} & (C3 | (C4 | T2)){x: Int} +54: Pattern Match Exhaustivity: _: Trait & (C1 | (C2 | T1)){x: Int} & C3{x: Int} 65: Pattern Match Exhaustivity: _: Trait & (C1 | C2){x: Int} & (C3 | SubC1){x: Int} 72: Pattern Match Exhaustivity: _: Trait & (T1 & (C1 | SubC2)){x: Int} & (T2 & (C2 | C3 | SubC1)){x: Int} & SubSubC1{x: Int} From 500a9e5ebb6aa0d3b6e2aee0eaf5c0c26605db4c Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 9 Aug 2018 17:41:43 +0200 Subject: [PATCH 07/13] Use precedence levels properly when printing infix types This way, printing an infix type need not inspect its arguments; that ensures we don't need to handle *all* the possible forms of types that need to be wrapped in parentheses. I did this purely as a refactoring, based on how pretty-printers are supposed to be written. But the new test case, combining infix type operators and builting connectives, didn't work with the old code. It also doesn't work in Scala 2: ```scala Welcome to Scala 2.12.6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_172). Type in expressions for evaluation. Or try :help. scala> class &&[T,U] defined class $amp$amp scala> def foo: Int && Boolean = ??? foo: Int && Boolean scala> def foo: Int && Boolean with String = ??? foo: Int && Boolean with String scala> def foo: Int && (Boolean with String) = ??? foo: Int && Boolean with String scala> def foo: (Int && Boolean) with String = ??? foo: Int && Boolean with String ``` --- .../src/dotty/tools/dotc/printing/RefinedPrinter.scala | 6 +++--- compiler/test-resources/type-printer/infix | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index a6978e978bd7..756089960bc9 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -155,15 +155,15 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { case _ => false } - def toTextInfixType(op: Type, args: List[Type]): Text = { + def toTextInfixType(op: Type, args: List[Type]): Text = changePrec(InfixPrec) { /* SLS 3.2.8: all infix types have the same precedence. * In A op B op' C, op and op' need the same associativity. * Therefore, if op is left associative, anything on its right * needs to be parenthesized if it's an infix type, and vice versa. */ val l :: r :: Nil = args val isRightAssoc = op.typeSymbol.name.endsWith(":") - val leftArg = if (isRightAssoc && isInfixType(l)) "(" ~ atPrec(GlobalPrec) { argText(l) } ~ ")" else atPrec(GlobalPrec) { argText(l) } - val rightArg = if (!isRightAssoc && isInfixType(r)) "(" ~ atPrec(GlobalPrec) { argText(r) } ~ ")" else atPrec(GlobalPrec) { argText(r) } + val leftArg = if (isRightAssoc) atPrec(InfixPrec + 1) { argText(l) } else argText(l) + val rightArg = if (!isRightAssoc) atPrec(InfixPrec + 1) { argText(r) } else argText(r) leftArg ~ " " ~ simpleNameString(op.classSymbol) ~ " " ~ rightArg } diff --git a/compiler/test-resources/type-printer/infix b/compiler/test-resources/type-printer/infix index e38fee3352c2..0b7c0e1f4788 100644 --- a/compiler/test-resources/type-printer/infix +++ b/compiler/test-resources/type-printer/infix @@ -25,3 +25,11 @@ scala> @scala.annotation.showAsInfix(false) class ||[T,U] // defined class || scala> def foo: Int || Boolean = ??? def foo: ||[Int, Boolean] +scala> def foo: Int && (Boolean with String) = ??? +def foo: Int && Boolean & String +scala> def foo: (Int && Boolean) with String = ??? +def foo: (Int && Boolean) & String +scala> def foo: (Int && Boolean) & String = ??? +def foo: (Int && Boolean) & String +scala> def foo: Int && (Boolean & String) = ??? +def foo: Int && Boolean & String From 6e8dfb4775a8f4790ca1c85457d9b2ff6e6e7795 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 1 Aug 2018 19:17:51 +0200 Subject: [PATCH 08/13] Handle associativity mismatches for type operator in pretty-printer * RefinedPrinter: Handle associativity mismatches for Type operators. * Add Documentation/tutorial on how to use atPrec and changePrec correctly, since so much code got it wrong and it takes a while to get it. * Extend tests for Type printing. --- .../dotty/tools/dotc/printing/Printer.scala | 39 +++++++++++++++- .../tools/dotc/printing/RefinedPrinter.scala | 44 ++++++++++++++----- compiler/test-resources/type-printer/infix | 36 ++++++++++++--- 3 files changed, 100 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/printing/Printer.scala b/compiler/src/dotty/tools/dotc/printing/Printer.scala index fc011b9f1ab5..dfed73565c38 100644 --- a/compiler/src/dotty/tools/dotc/printing/Printer.scala +++ b/compiler/src/dotty/tools/dotc/printing/Printer.scala @@ -17,10 +17,24 @@ abstract class Printer { private[this] var prec: Precedence = GlobalPrec - /** The current precedence level */ + /** The current precedence level. + * WHen pretty-printing arguments of operator `op`, `currentPrecedence` must equal `op`'s precedence level, + * so that pretty-printing expressions using lower-precedence operators can insert parentheses automatically + * by calling `changePrec`. + */ def currentPrecedence = prec - /** Generate text using `op`, assuming a given precedence level `prec`. */ + /** Generate text using `op`, assuming a given precedence level `prec`. + * + * ### `atPrec` vs `changePrec` + * + * This is to be used when changing precedence inside some sort of parentheses: + * for instance, to print T[A]` use + * `toText(T) ~ '[' ~ atPrec(GlobalPrec) { toText(A) } ~ ']'`. + * + * If the presence of the parentheses depends on precedence, inserting them manually is most certainly a bug. + * Use `changePrec` instead to generate them exactly when needed. + */ def atPrec(prec: Precedence)(op: => Text): Text = { val outerPrec = this.prec this.prec = prec @@ -30,6 +44,27 @@ abstract class Printer { /** Generate text using `op`, assuming a given precedence level `prec`. * If new level `prec` is lower than previous level, put text in parentheses. + * + * ### `atPrec` vs `changePrec` + * + * To pretty-print `A op B`, you need something like + * `changePrec(parsing.precedence(op, isType)) { toText(a) ~ op ~ toText(b) }` // BUGGY + * that will insert parentheses around `A op B` if, for instance, the + * preceding operator has higher precedence. + * + * But that does not handle infix operators with left- or right- associativity. + * + * If op and op' have the same precedence and associativity, + * A op B op' C parses as (A op B) op' C if op and op' are left-associative, and as + * A op (B op' C) if they're right-associative, so we need respectively + * ```scala + * val isType = ??? // is this a term or type operator? + * val prec = parsing.precedence(op, isType) + * // either: + * changePrec(prec) { toText(a) ~ op ~ atPrec(prec + 1) { toText(b) } } // for left-associative op and op' + * // or: + * changePrec(prec) { atPrec(prec + 1) { toText(a) } ~ op ~ toText(b) } // for right-associative op and op' + * ``` */ def changePrec(prec: Precedence)(op: => Text): Text = if (prec < this.prec) atPrec(prec) ("(" ~ op ~ ")") else atPrec(prec)(op) diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 756089960bc9..d5c779eccbbb 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -155,17 +155,24 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { case _ => false } - def toTextInfixType(op: Type, args: List[Type]): Text = changePrec(InfixPrec) { - /* SLS 3.2.8: all infix types have the same precedence. - * In A op B op' C, op and op' need the same associativity. - * Therefore, if op is left associative, anything on its right - * needs to be parenthesized if it's an infix type, and vice versa. */ - val l :: r :: Nil = args - val isRightAssoc = op.typeSymbol.name.endsWith(":") - val leftArg = if (isRightAssoc) atPrec(InfixPrec + 1) { argText(l) } else argText(l) - val rightArg = if (!isRightAssoc) atPrec(InfixPrec + 1) { argText(r) } else argText(r) - - leftArg ~ " " ~ simpleNameString(op.classSymbol) ~ " " ~ rightArg + def tyconName(tp: Type): Name = tp.typeSymbol.name + def checkAssocMismatch(tp: Type, isRightAssoc: Boolean) = tp match { + case AppliedType(tycon, _) => isInfixType(tp) && tyconName(tycon).endsWith(":") != isRightAssoc + case AndType(_, _) => isRightAssoc + case OrType(_, _) => isRightAssoc + case _ => false + } + + def toTextInfixType(opName: Name, l: Type, r: Type)(op: => Text): Text = { + val isRightAssoc = opName.endsWith(":") + val opPrec = parsing.precedence(opName, true) + + changePrec(opPrec) { + val leftPrec = if (isRightAssoc || checkAssocMismatch(l, isRightAssoc)) opPrec + 1 else opPrec + val rightPrec = if (!isRightAssoc || checkAssocMismatch(r, isRightAssoc)) opPrec + 1 else opPrec + + atPrec(leftPrec) { argText(l) } ~ " " ~ op ~ " " ~ atPrec(rightPrec) { argText(r) } + } } homogenize(tp) match { @@ -174,7 +181,20 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { if (tycon.isRepeatedParam) return toTextLocal(args.head) ~ "*" if (defn.isFunctionClass(cls)) return toTextFunction(args, cls.name.isImplicitFunction, cls.name.isErasedFunction) if (defn.isTupleClass(cls)) return toTextTuple(args) - if (isInfixType(tp)) return toTextInfixType(tycon, args) + if (isInfixType(tp)) { + val l :: r :: Nil = args + val opName = tyconName(tycon) + + return toTextInfixType(tyconName(tycon), l, r) { simpleNameString(tycon.classSymbol) } + } + + // Since RefinedPrinter, unlike PlainPrinter, can output right-associative type-operators, we must override handling + // of AndType and OrType to account for associativity + case AndType(tp1, tp2) => + return toTextInfixType(tpnme.raw.AMP, tp1, tp2) { toText(tpnme.raw.AMP) } + case OrType(tp1, tp2) => + return toTextInfixType(tpnme.raw.BAR, tp1, tp2) { toText(tpnme.raw.BAR) } + case EtaExpansion(tycon) => return toText(tycon) case tp: RefinedType if defn.isFunctionType(tp) => diff --git a/compiler/test-resources/type-printer/infix b/compiler/test-resources/type-printer/infix index 0b7c0e1f4788..caac75e62fe2 100644 --- a/compiler/test-resources/type-printer/infix +++ b/compiler/test-resources/type-printer/infix @@ -16,20 +16,46 @@ scala> def foo: (Int && String) &: Boolean = ??? def foo: (Int && String) &: Boolean scala> def foo: Int && (Boolean &: String) = ??? def foo: Int && (Boolean &: String) +scala> def foo: (Int &: String) && Boolean = ??? +def foo: (Int &: String) && Boolean +scala> def foo: Int &: (Boolean && String) = ??? +def foo: Int &: (Boolean && String) +scala> def foo: (Int & String) &: Boolean = ??? +def foo: (Int & String) &: Boolean +scala> def foo: Int & (Boolean &: String) = ??? +def foo: Int & (Boolean &: String) +scala> def foo: (Int &: String) & Boolean = ??? +def foo: (Int &: String) & Boolean +scala> def foo: Int &: (Boolean & String) = ??? +def foo: Int &: (Boolean & String) scala> import scala.annotation.showAsInfix scala> @scala.annotation.showAsInfix class Mappy[T,U] // defined class Mappy +scala> def foo: (Int Mappy Boolean) && String = ??? +def foo: (Int Mappy Boolean) && String +scala> def foo: Int Mappy Boolean && String = ??? +def foo: Int Mappy Boolean && String scala> def foo: Int Mappy (Boolean && String) = ??? -def foo: Int Mappy (Boolean && String) +def foo: Int Mappy Boolean && String scala> @scala.annotation.showAsInfix(false) class ||[T,U] // defined class || scala> def foo: Int || Boolean = ??? def foo: ||[Int, Boolean] -scala> def foo: Int && (Boolean with String) = ??? +scala> def foo: Int && Boolean & String = ??? def foo: Int && Boolean & String -scala> def foo: (Int && Boolean) with String = ??? -def foo: (Int && Boolean) & String scala> def foo: (Int && Boolean) & String = ??? -def foo: (Int && Boolean) & String +def foo: Int && Boolean & String scala> def foo: Int && (Boolean & String) = ??? +def foo: Int && (Boolean & String) +scala> def foo: Int && (Boolean with String) = ??? +def foo: Int && (Boolean & String) +scala> def foo: (Int && Boolean) with String = ??? def foo: Int && Boolean & String +scala> def foo: Int && Boolean with String = ??? +def foo: Int && (Boolean & String) +scala> def foo: Int && Boolean | String = ??? +def foo: Int && Boolean | String +scala> def foo: Int && (Boolean | String) = ??? +def foo: Int && (Boolean | String) +scala> def foo: (Int && Boolean) | String = ??? +def foo: Int && Boolean | String From 5e01496d910caaa57a0d950863816d40d8443981 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 9 Aug 2018 16:15:55 +0200 Subject: [PATCH 09/13] Use typeSymbol not classSymbol Refines 8b941d611b7ba09b91328e3632c89e2c4d5b7864: classSymbol is unnecessarily aggressive here. --- compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index d5c779eccbbb..17901b431cfa 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -185,7 +185,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { val l :: r :: Nil = args val opName = tyconName(tycon) - return toTextInfixType(tyconName(tycon), l, r) { simpleNameString(tycon.classSymbol) } + return toTextInfixType(tyconName(tycon), l, r) { simpleNameString(tycon.typeSymbol) } } // Since RefinedPrinter, unlike PlainPrinter, can output right-associative type-operators, we must override handling From 0333dd61db28dd015f2501f091d0b391c2c1ebbb Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 20 Aug 2018 17:59:57 +0200 Subject: [PATCH 10/13] Fix typo --- compiler/src/dotty/tools/dotc/printing/Printer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/printing/Printer.scala b/compiler/src/dotty/tools/dotc/printing/Printer.scala index dfed73565c38..ae0ead4896b3 100644 --- a/compiler/src/dotty/tools/dotc/printing/Printer.scala +++ b/compiler/src/dotty/tools/dotc/printing/Printer.scala @@ -18,7 +18,7 @@ abstract class Printer { private[this] var prec: Precedence = GlobalPrec /** The current precedence level. - * WHen pretty-printing arguments of operator `op`, `currentPrecedence` must equal `op`'s precedence level, + * When pretty-printing arguments of operator `op`, `currentPrecedence` must equal `op`'s precedence level, * so that pretty-printing expressions using lower-precedence operators can insert parentheses automatically * by calling `changePrec`. */ From dfdf9a1ed77184a93099d11b99e2fda01aab2767 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 20 Aug 2018 18:01:09 +0200 Subject: [PATCH 11/13] Type-printer tests: simplify per review comment --- .../test-resources/type-printer/functions | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/test-resources/type-printer/functions b/compiler/test-resources/type-printer/functions index 05a1eda995e3..6ed45e2ee8cd 100644 --- a/compiler/test-resources/type-printer/functions +++ b/compiler/test-resources/type-printer/functions @@ -1,10 +1,10 @@ -scala> val toInt: Any => Int = new { def apply(a: Any) = 1; override def toString() = "" } -val toInt: Any => Int = -scala> val hoFun: (Int => Int) => Int = new { def apply(a: Int => Int) = 1; override def toString() = "" } -val hoFun: (Int => Int) => Int = -scala> val curriedFun: Int => (Int => Int) = new { def apply(a: Int) = _ => 1; override def toString() = "" } -val curriedFun: Int => Int => Int = -scala> val tupFun: ((Int, Int)) => Int = new { def apply(a: (Int, Int)) = 1; override def toString() = "" } -val tupFun: ((Int, Int)) => Int = -scala> val binFun: (Int, Int) => Int = new { def apply(a: Int, b: Int) = 1; override def toString() = "" } -val binFun: (Int, Int) => Int = +scala> def toInt: Any => Int = ??? +def toInt: Any => Int +scala> def hoFun: (Int => Int) => Int = ??? +def hoFun: (Int => Int) => Int +scala> def curriedFun: Int => (Int => Int) = ??? +def curriedFun: Int => Int => Int +scala> def tupFun: ((Int, Int)) => Int = ??? +def tupFun: ((Int, Int)) => Int +scala> def binFun: (Int, Int) => Int = ??? +def binFun: (Int, Int) => Int From de8eb4dfa3887e2e7217f79eee2c8c6f4b8429e7 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 20 Aug 2018 18:17:34 +0200 Subject: [PATCH 12/13] Hardcode SIP-33: term and type operators have the same priority --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 6 +++--- compiler/src/dotty/tools/dotc/parsing/package.scala | 2 +- compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala | 2 +- compiler/src/dotty/tools/dotc/printing/package.scala | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 50c571a8b5e3..991255e913fa 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -471,13 +471,13 @@ object Parsers { syntaxError(MixedLeftAndRightAssociativeOps(op1, op2, op2LeftAssoc), offset) def reduceStack(base: List[OpInfo], top: Tree, prec: Int, leftAssoc: Boolean, op2: Name, isType: Boolean): Tree = { - if (opStack != base && precedence(opStack.head.operator.name, isType) == prec) + if (opStack != base && precedence(opStack.head.operator.name) == prec) checkAssoc(opStack.head.offset, opStack.head.operator.name, op2, leftAssoc) def recur(top: Tree): Tree = { if (opStack == base) top else { val opInfo = opStack.head - val opPrec = precedence(opInfo.operator.name, isType) + val opPrec = precedence(opInfo.operator.name) if (prec < opPrec || leftAssoc && prec == opPrec) { opStack = opStack.tail recur { @@ -514,7 +514,7 @@ object Parsers { var top = first while (isIdent && isOperator) { val op = if (isType) typeIdent() else termIdent() - top = reduceStack(base, top, precedence(op.name, isType), isLeftAssoc(op.name), op.name, isType) + top = reduceStack(base, top, precedence(op.name), isLeftAssoc(op.name), op.name, isType) opStack = OpInfo(top, op, in.offset) :: opStack newLineOptWhenFollowing(canStartOperand) if (maybePostfix && !canStartOperand(in.token)) { diff --git a/compiler/src/dotty/tools/dotc/parsing/package.scala b/compiler/src/dotty/tools/dotc/parsing/package.scala index 49676e236892..cdb30d0be7ed 100644 --- a/compiler/src/dotty/tools/dotc/parsing/package.scala +++ b/compiler/src/dotty/tools/dotc/parsing/package.scala @@ -7,7 +7,7 @@ import core.NameOps._ package object parsing { - def precedence(operator: Name, isType: Boolean = false): Int = + def precedence(operator: Name): Int = if (operator eq nme.ERROR) -1 else { val firstCh = operator.firstPart.head diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 17901b431cfa..7ae4192c7bd6 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -165,7 +165,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { def toTextInfixType(opName: Name, l: Type, r: Type)(op: => Text): Text = { val isRightAssoc = opName.endsWith(":") - val opPrec = parsing.precedence(opName, true) + val opPrec = parsing.precedence(opName) changePrec(opPrec) { val leftPrec = if (isRightAssoc || checkAssocMismatch(l, isRightAssoc)) opPrec + 1 else opPrec diff --git a/compiler/src/dotty/tools/dotc/printing/package.scala b/compiler/src/dotty/tools/dotc/printing/package.scala index cb6d73e23beb..80dfb5244d97 100644 --- a/compiler/src/dotty/tools/dotc/printing/package.scala +++ b/compiler/src/dotty/tools/dotc/printing/package.scala @@ -9,8 +9,8 @@ package object printing { type Precedence = Int val DotPrec = parsing.maxPrec - val AndTypePrec = parsing.precedence(tpnme.raw.AMP, true) - val OrTypePrec = parsing.precedence(tpnme.raw.BAR, true) + val AndTypePrec = parsing.precedence(tpnme.raw.AMP) + val OrTypePrec = parsing.precedence(tpnme.raw.BAR) val OrPrec = parsing.precedence(nme.raw.BAR) val InfixPrec = parsing.minInfixPrec val GlobalPrec = parsing.minPrec From f9990c7724f6c2ce242cae5916673e8ad9832786 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 20 Aug 2018 18:18:00 +0200 Subject: [PATCH 13/13] Give normative references for operator precedence I verified the links use correct Dottydoc syntax. --- compiler/src/dotty/tools/dotc/parsing/package.scala | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/parsing/package.scala b/compiler/src/dotty/tools/dotc/parsing/package.scala index cdb30d0be7ed..5ba02fd1d3bb 100644 --- a/compiler/src/dotty/tools/dotc/parsing/package.scala +++ b/compiler/src/dotty/tools/dotc/parsing/package.scala @@ -7,6 +7,13 @@ import core.NameOps._ package object parsing { + /** + * Compute the precedence of infix operator `operator` according to the SLS [ยง 6.12.3][SLS]. + * We implement [SIP-33][SIP-33] and give type operators the same precedence as term operators. + * + * [SLS]: https://www.scala-lang.org/files/archive/spec/2.13/06-expressions.html#infix-operations + * [SIP-33]: https://docs.scala-lang.org/sips/priority-based-infix-type-precedence.html + */ def precedence(operator: Name): Int = if (operator eq nme.ERROR) -1 else {