Skip to content

Commit 0725449

Browse files
committed
Fix type operator precedence levels in parser & pretty-printer
The correct precedence levels are up for debate, but this patch follows the spec, while encapsulating the precedence rules in `parsing.precedence`. * According to the spec, term- and type-level operators precedences are different, so 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. And since the operators are left-associative, increase precedence for right operands, for both `Type`s and `TypeTree`s. * 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. * Fix printing for AndTypeTree and OrTypeTree along AndType and OrType. * Extend tests for Type printing. * PrinterTests: Add first test for TypeTree printing. * PrinterTests: disable colors, following ErrorMessagesTest.scala.
1 parent 43941f4 commit 0725449

File tree

8 files changed

+138
-29
lines changed

8 files changed

+138
-29
lines changed

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -454,13 +454,13 @@ object Parsers {
454454
syntaxError(MixedLeftAndRightAssociativeOps(op1, op2, op2LeftAssoc), offset)
455455

456456
def reduceStack(base: List[OpInfo], top: Tree, prec: Int, leftAssoc: Boolean, op2: Name, isType: Boolean): Tree = {
457-
if (opStack != base && precedence(opStack.head.operator.name) == prec)
457+
if (opStack != base && precedence(opStack.head.operator.name, isType) == prec)
458458
checkAssoc(opStack.head.offset, opStack.head.operator.name, op2, leftAssoc)
459459
def recur(top: Tree): Tree = {
460460
if (opStack == base) top
461461
else {
462462
val opInfo = opStack.head
463-
val opPrec = precedence(opInfo.operator.name)
463+
val opPrec = precedence(opInfo.operator.name, isType)
464464
if (prec < opPrec || leftAssoc && prec == opPrec) {
465465
opStack = opStack.tail
466466
recur {
@@ -497,7 +497,7 @@ object Parsers {
497497
var top = first
498498
while (isIdent && isOperator) {
499499
val op = if (isType) typeIdent() else termIdent()
500-
top = reduceStack(base, top, precedence(op.name), isLeftAssoc(op.name), op.name, isType)
500+
top = reduceStack(base, top, precedence(op.name, isType), isLeftAssoc(op.name), op.name, isType)
501501
opStack = OpInfo(top, op, in.offset) :: opStack
502502
newLineOptWhenFollowing(canStartOperand)
503503
if (maybePostfix && !canStartOperand(in.token)) {

compiler/src/dotty/tools/dotc/parsing/package.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import core.NameOps._
77

88
package object parsing {
99

10-
def precedence(operator: Name): Int =
10+
def precedence(operator: Name, isType: Boolean = false): Int =
1111
if (operator eq nme.ERROR) -1
12+
/* SLS 3.2.10 (https://www.scala-lang.org/files/archive/spec/2.13/03-types.html#infix-types):
13+
* all infix types have the same precedence */
14+
else if (isType) minInfixPrec
1215
else {
1316
val firstCh = operator.firstPart.head
1417
if (isScalaLetter(firstCh)) 1

compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,9 @@ class PlainPrinter(_ctx: Context) extends Printer {
161161
}
162162
finally openRecs = openRecs.tail
163163
case AndType(tp1, tp2) =>
164-
changePrec(AndPrec) { toText(tp1) ~ " & " ~ toText(tp2) }
164+
changePrec(AndTypePrec) { toText(tp1) ~ " & " ~ atPrec(AndTypePrec + 1) { toText(tp2) } }
165165
case OrType(tp1, tp2) =>
166-
changePrec(OrPrec) { toText(tp1) ~ " | " ~ toText(tp2) }
166+
changePrec(OrTypePrec) { toText(tp1) ~ " | " ~ atPrec(OrTypePrec + 1) { toText(tp2) } }
167167
case tp: ErrorType =>
168168
s"<error ${tp.msg.msg}>"
169169
case tp: WildcardType =>

compiler/src/dotty/tools/dotc/printing/Printer.scala

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,24 @@ abstract class Printer {
1717

1818
private[this] var prec: Precedence = GlobalPrec
1919

20-
/** The current precedence level */
20+
/** The current precedence level.
21+
* WHen pretty-printing arguments of operator `op`, `currentPrecedence` must equal `op`'s precedence level,
22+
* so that pretty-printing expressions using lower-precedence operators can insert parentheses automatically
23+
* by calling `changePrec`.
24+
*/
2125
def currentPrecedence = prec
2226

23-
/** Generate text using `op`, assuming a given precedence level `prec`. */
27+
/** Generate text using `op`, assuming a given precedence level `prec`.
28+
*
29+
* ### `atPrec` vs `changePrec`
30+
*
31+
* This is to be used when changing precedence inside some sort of parentheses:
32+
* for instance, to print T[A]` use
33+
* `toText(T) ~ '[' ~ atPrec(GlobalPrec) { toText(A) } ~ ']'`.
34+
*
35+
* If the presence of the parentheses depends on precedence, inserting them manually is most certainly a bug.
36+
* Use `changePrec` instead to generate them exactly when needed.
37+
*/
2438
def atPrec(prec: Precedence)(op: => Text): Text = {
2539
val outerPrec = this.prec
2640
this.prec = prec
@@ -30,6 +44,27 @@ abstract class Printer {
3044

3145
/** Generate text using `op`, assuming a given precedence level `prec`.
3246
* If new level `prec` is lower than previous level, put text in parentheses.
47+
*
48+
* ### `atPrec` vs `changePrec`
49+
*
50+
* To pretty-print `A op B`, you need something like
51+
* `changePrec(parsing.precedence(op, isType)) { toText(a) ~ op ~ toText(b) }` // BUGGY
52+
* that will insert parentheses around `A op B` if, for instance, the
53+
* preceding operator has higher precedence.
54+
*
55+
* But that does not handle infix operators with left- or right- associativity.
56+
*
57+
* If op and op' have the same precedence and associativity,
58+
* A op B op' C parses as (A op B) op' C if op and op' are left-associative, and as
59+
* A op (B op' C) if they're right-associative, so we need respectively
60+
* ```scala
61+
* val isType = ??? // is this a term or type operator?
62+
* val prec = parsing.precedence(op, isType)
63+
* // either:
64+
* changePrec(prec) { toText(a) ~ op ~ atPrec(prec + 1) { toText(b) } } // for left-associative op and op'
65+
* // or:
66+
* changePrec(prec) { atPrec(prec + 1) { toText(a) } ~ op ~ toText(b) } // for right-associative op and op'
67+
* ```
3368
*/
3469
def changePrec(prec: Precedence)(op: => Text): Text =
3570
if (prec < this.prec) atPrec(prec) ("(" ~ op ~ ")") else atPrec(prec)(op)

compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -155,17 +155,24 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
155155
case _ => false
156156
}
157157

158-
def toTextInfixType(op: Type, args: List[Type]): Text = changePrec(InfixPrec) {
159-
/* SLS 3.2.8: all infix types have the same precedence.
160-
* In A op B op' C, op and op' need the same associativity.
161-
* Therefore, if op is left associative, anything on its right
162-
* needs to be parenthesized if it's an infix type, and vice versa. */
163-
val l :: r :: Nil = args
164-
val isRightAssoc = op.typeSymbol.name.endsWith(":")
165-
val leftArg = if (isRightAssoc) atPrec(OrPrec) { argText(l) } else argText(l)
166-
val rightArg = if (!isRightAssoc) atPrec(OrPrec) { argText(r) } else argText(r)
167-
168-
leftArg ~ " " ~ toTextLocal(op) ~ " " ~ rightArg
158+
def tyconName(tp: Type): Name = tp.typeSymbol.name
159+
def checkAssocMismatch(tp: Type, isRightAssoc: Boolean) = tp match {
160+
case AppliedType(tycon, _) => isInfixType(tp) && tyconName(tycon).endsWith(":") != isRightAssoc
161+
case AndType(_, _) => isRightAssoc
162+
case OrType(_, _) => isRightAssoc
163+
case _ => false
164+
}
165+
166+
def toTextInfixType(opName: Name, l: Type, r: Type)(op: => Text): Text = {
167+
val isRightAssoc = opName.endsWith(":")
168+
val opPrec = parsing.precedence(opName, true)
169+
170+
changePrec(opPrec) {
171+
val leftPrec = if (isRightAssoc || checkAssocMismatch(l, isRightAssoc)) opPrec + 1 else opPrec
172+
val rightPrec = if (!isRightAssoc || checkAssocMismatch(r, isRightAssoc)) opPrec + 1 else opPrec
173+
174+
atPrec(leftPrec) { argText(l) } ~ " " ~ op ~ " " ~ atPrec(rightPrec) { argText(r) }
175+
}
169176
}
170177

171178
homogenize(tp) match {
@@ -174,7 +181,20 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
174181
if (tycon.isRepeatedParam) return toTextLocal(args.head) ~ "*"
175182
if (defn.isFunctionClass(cls)) return toTextFunction(args, cls.name.isImplicitFunction, cls.name.isErasedFunction)
176183
if (defn.isTupleClass(cls)) return toTextTuple(args)
177-
if (isInfixType(tp)) return toTextInfixType(tycon, args)
184+
if (isInfixType(tp)) {
185+
val l :: r :: Nil = args
186+
val opName = tyconName(tycon)
187+
188+
return toTextInfixType(tyconName(tycon), l, r) { toTextLocal(tycon) }
189+
}
190+
191+
// Since RefinedPrinter, unlike PlainPrinter, can output right-associative type-operators, we must override handling
192+
// of AndType and OrType to account for associativity
193+
case AndType(tp1, tp2) =>
194+
return toTextInfixType(tpnme.raw.AMP, tp1, tp2) { toText(tpnme.raw.AMP) }
195+
case OrType(tp1, tp2) =>
196+
return toTextInfixType(tpnme.raw.BAR, tp1, tp2) { toText(tpnme.raw.BAR) }
197+
178198
case EtaExpansion(tycon) =>
179199
return toText(tycon)
180200
case tp: RefinedType if defn.isFunctionType(tp) =>
@@ -375,9 +395,9 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
375395
case SingletonTypeTree(ref) =>
376396
toTextLocal(ref) ~ "." ~ keywordStr("type")
377397
case AndTypeTree(l, r) =>
378-
changePrec(AndPrec) { toText(l) ~ " & " ~ toText(r) }
398+
changePrec(AndTypePrec) { toText(l) ~ " & " ~ atPrec(AndTypePrec + 1) { toText(r) } }
379399
case OrTypeTree(l, r) =>
380-
changePrec(OrPrec) { toText(l) ~ " | " ~ toText(r) }
400+
changePrec(OrTypePrec) { toText(l) ~ " | " ~ atPrec(OrTypePrec + 1) { toText(r) } }
381401
case RefinedTypeTree(tpt, refines) =>
382402
toTextLocal(tpt) ~ " " ~ blockText(refines)
383403
case AppliedTypeTree(tpt, args) =>

compiler/src/dotty/tools/dotc/printing/package.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package dotty.tools.dotc
22

3-
import core.StdNames.nme
3+
import core.StdNames.{nme,tpnme}
44
import parsing.{precedence, minPrec, maxPrec, minInfixPrec}
55
import util.Property.Key
66

@@ -9,7 +9,8 @@ package object printing {
99
type Precedence = Int
1010

1111
val DotPrec = parsing.maxPrec
12-
val AndPrec = parsing.precedence(nme.raw.AMP)
12+
val AndTypePrec = parsing.precedence(tpnme.raw.AMP, true)
13+
val OrTypePrec = parsing.precedence(tpnme.raw.BAR, true)
1314
val OrPrec = parsing.precedence(nme.raw.BAR)
1415
val InfixPrec = parsing.minInfixPrec
1516
val GlobalPrec = parsing.minPrec

compiler/test-resources/type-printer/infix

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,46 @@ scala> def foo: (Int && String) &: Boolean = ???
1616
def foo: (Int && String) &: Boolean
1717
scala> def foo: Int && (Boolean &: String) = ???
1818
def foo: Int && (Boolean &: String)
19+
scala> def foo: (Int &: String) && Boolean = ???
20+
def foo: (Int &: String) && Boolean
21+
scala> def foo: Int &: (Boolean && String) = ???
22+
def foo: Int &: (Boolean && String)
23+
scala> def foo: (Int & String) &: Boolean = ???
24+
def foo: (Int & String) &: Boolean
25+
scala> def foo: Int & (Boolean &: String) = ???
26+
def foo: Int & (Boolean &: String)
27+
scala> def foo: (Int &: String) & Boolean = ???
28+
def foo: (Int &: String) & Boolean
29+
scala> def foo: Int &: (Boolean & String) = ???
30+
def foo: Int &: (Boolean & String)
1931
scala> import scala.annotation.showAsInfix
2032
scala> @scala.annotation.showAsInfix class Mappy[T,U]
2133
// defined class Mappy
34+
scala> def foo: (Int Mappy Boolean) && String = ???
35+
def foo: Int Mappy Boolean && String
36+
scala> def foo: Int Mappy Boolean && String = ???
37+
def foo: Int Mappy Boolean && String
2238
scala> def foo: Int Mappy (Boolean && String) = ???
2339
def foo: Int Mappy (Boolean && String)
2440
scala> @scala.annotation.showAsInfix(false) class ||[T,U]
2541
// defined class ||
2642
scala> def foo: Int || Boolean = ???
2743
def foo: ||[Int, Boolean]
28-
scala> def foo: Int && (Boolean with String) = ???
44+
scala> def foo: Int && Boolean & String = ???
2945
def foo: Int && Boolean & String
30-
scala> def foo: (Int && Boolean) with String = ???
31-
def foo: (Int && Boolean) & String
3246
scala> def foo: (Int && Boolean) & String = ???
33-
def foo: (Int && Boolean) & String
47+
def foo: Int && Boolean & String
3448
scala> def foo: Int && (Boolean & String) = ???
49+
def foo: Int && (Boolean & String)
50+
scala> def foo: Int && (Boolean with String) = ???
51+
def foo: Int && (Boolean & String)
52+
scala> def foo: (Int && Boolean) with String = ???
3553
def foo: Int && Boolean & String
54+
scala> def foo: Int && Boolean with String = ???
55+
def foo: Int && (Boolean & String)
56+
scala> def foo: Int && Boolean | String = ???
57+
def foo: Int && Boolean | String
58+
scala> def foo: Int && (Boolean | String) = ???
59+
def foo: Int && (Boolean | String)
60+
scala> def foo: (Int && Boolean) | String = ???
61+
def foo: Int && Boolean | String

compiler/test/dotty/tools/dotc/printing/PrinterTests.scala

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
package dotty.tools.dotc.printing
22

33
import dotty.tools.DottyTest
4-
import dotty.tools.dotc.ast.tpd
4+
import dotty.tools.dotc.ast.{Trees,tpd}
55
import dotty.tools.dotc.core.Names._
66
import dotty.tools.dotc.core.Symbols._
7+
import dotty.tools.dotc.core.Decorators._
78
import org.junit.Assert.assertEquals
89
import org.junit.Test
910

1011
class PrinterTests extends DottyTest {
12+
13+
private def newContext = {
14+
initialCtx.setSetting(ctx.settings.color, "never")
15+
}
16+
ctx = newContext
17+
1118
import tpd._
1219

1320
@Test
@@ -24,4 +31,21 @@ class PrinterTests extends DottyTest {
2431
assertEquals("package object foo", bar.symbol.owner.show)
2532
}
2633
}
34+
35+
@Test
36+
def tpTreeInfixOps: Unit = {
37+
val source = """
38+
|class &&[T,U]
39+
|object Foo {
40+
| def bar1: Int && (Boolean | String) = ???
41+
| def bar2: Int & (Boolean | String) = ???
42+
|}
43+
""".stripMargin
44+
45+
checkCompile("frontend", source) { (tree, context) =>
46+
implicit val ctx = context
47+
val bar @ Trees.DefDef(_, _, _, _, _) = tree.find(tree => tree.symbol.name == termName("bar2")).get
48+
assertEquals("Int & (Boolean | String)", bar.tpt.show)
49+
}
50+
}
2751
}

0 commit comments

Comments
 (0)