Skip to content

Commit da2a245

Browse files
BlaisorbladeBlaisorblade
authored
Merge pull request #4072 from dotty-staging/fix-#4068
Pretty-printing: handle type-operator precedence correctly
2 parents 3d3ed97 + f9990c7 commit da2a245

File tree

11 files changed

+169
-33
lines changed

11 files changed

+169
-33
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ import core.NameOps._
77

88
package object parsing {
99

10+
/**
11+
* Compute the precedence of infix operator `operator` according to the SLS [§ 6.12.3][SLS].
12+
* We implement [SIP-33][SIP-33] and give type operators the same precedence as term operators.
13+
*
14+
* [SLS]: https://www.scala-lang.org/files/archive/spec/2.13/06-expressions.html#infix-operations
15+
* [SIP-33]: https://docs.scala-lang.org/sips/priority-based-infix-type-precedence.html
16+
*/
1017
def precedence(operator: Name): Int =
1118
if (operator eq nme.ERROR) -1
1219
else {

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,15 @@ class PlainPrinter(_ctx: Context) extends Printer {
102102
(refinementNameString(rt) ~ toTextRHS(rt.refinedInfo)).close
103103

104104
protected def argText(arg: Type): Text = homogenizeArg(arg) match {
105-
case arg: TypeBounds => "_" ~ toTextGlobal(arg)
106-
case arg => toTextGlobal(arg)
105+
case arg: TypeBounds => "_" ~ toText(arg)
106+
case arg => toText(arg)
107107
}
108108

109+
/** Pretty-print comma-separated type arguments for a constructor to be inserted among parentheses or brackets
110+
* (hence with `GlobalPrec` precedence).
111+
*/
112+
protected def argsText(args: List[Type]): Text = atPrec(GlobalPrec) { Text(args.map(arg => argText(arg) ), ", ") }
113+
109114
/** The longest sequence of refinement types, starting at given type
110115
* and following parents.
111116
*/
@@ -144,7 +149,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
144149
case tp: SingletonType =>
145150
toTextLocal(tp.underlying) ~ "(" ~ toTextRef(tp) ~ ")"
146151
case AppliedType(tycon, args) =>
147-
(toTextLocal(tycon) ~ "[" ~ Text(args map argText, ", ") ~ "]").close
152+
(toTextLocal(tycon) ~ "[" ~ argsText(args) ~ "]").close
148153
case tp: RefinedType =>
149154
val parent :: (refined: List[RefinedType @unchecked]) =
150155
refinementChain(tp).reverse
@@ -156,9 +161,9 @@ class PlainPrinter(_ctx: Context) extends Printer {
156161
}
157162
finally openRecs = openRecs.tail
158163
case AndType(tp1, tp2) =>
159-
changePrec(AndPrec) { toText(tp1) ~ " & " ~ toText(tp2) }
164+
changePrec(AndTypePrec) { toText(tp1) ~ " & " ~ atPrec(AndTypePrec + 1) { toText(tp2) } }
160165
case OrType(tp1, tp2) =>
161-
changePrec(OrPrec) { toText(tp1) ~ " | " ~ toText(tp2) }
166+
changePrec(OrTypePrec) { toText(tp1) ~ " | " ~ atPrec(OrTypePrec + 1) { toText(tp2) } }
162167
case tp: ErrorType =>
163168
s"<error ${tp.msg.msg}>"
164169
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: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
130130

131131
override def toText(tp: Type): Text = controlled {
132132
def toTextTuple(args: List[Type]): Text =
133-
"(" ~ Text(args.map(argText), ", ") ~ ")"
133+
"(" ~ argsText(args) ~ ")"
134134

135135
def toTextFunction(args: List[Type], isImplicit: Boolean, isErased: Boolean): Text =
136136
changePrec(GlobalPrec) {
@@ -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 = {
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 && isInfixType(l)) "(" ~ argText(l) ~ ")" else argText(l)
166-
val rightArg = if (!isRightAssoc && isInfixType(r)) "(" ~ argText(r) ~ ")" else argText(r)
167-
168-
leftArg ~ " " ~ simpleNameString(op.classSymbol) ~ " " ~ 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)
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) { simpleNameString(tycon.typeSymbol) }
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) =>
@@ -201,7 +221,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
201221
// (they don't need to because we keep the original type tree with
202222
// the original annotation anyway. Therefore, there will always be
203223
// one version of the annotation tree that has the correct positions).
204-
withoutPos(super.toText(tp))
224+
return withoutPos(super.toText(tp))
205225
case tp: SelectionProto =>
206226
return "?{ " ~ toText(tp.name) ~
207227
(Str(" ") provided !tp.name.toSimpleName.last.isLetterOrDigit) ~
@@ -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/Texts.scala‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import language.implicitConversions
44

55
object Texts {
66

7-
abstract class Text {
7+
sealed abstract class Text {
88

99
protected def indentMargin = 2
1010

@@ -46,9 +46,11 @@ object Texts {
4646
case Str(s1, lines2) => Str(s1 + s2, lines1 union lines2)
4747
case Fluid(Str(s1, lines2) :: prev) => Fluid(Str(s1 + s2, lines1 union lines2) :: prev)
4848
case Fluid(relems) => Fluid(that :: relems)
49+
case Vertical(_) => throw new IllegalArgumentException("Unexpected Vertical.appendToLastLine")
4950
}
5051
case Fluid(relems) =>
5152
(this /: relems.reverse)(_ appendToLastLine _)
53+
case Vertical(_) => throw new IllegalArgumentException("Unexpected Text.appendToLastLine(Vertical(...))")
5254
}
5355

5456
private def appendIndented(that: Text)(width: Int): Text =

‎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)
13+
val OrTypePrec = parsing.precedence(tpnme.raw.BAR)
1314
val OrPrec = parsing.precedence(nme.raw.BAR)
1415
val InfixPrec = parsing.minInfixPrec
1516
val GlobalPrec = parsing.minPrec

‎compiler/test-resources/type-printer/functions‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,10 @@
1-
scala> val toInt: Any => Int = new { def apply(a: Any) = 1; override def toString() = "<func1>" }
2-
val toInt: Any => Int = <func1>
1+
scala> def toInt: Any => Int = ???
2+
def toInt: Any => Int
3+
scala> def hoFun: (Int => Int) => Int = ???
4+
def hoFun: (Int => Int) => Int
5+
scala> def curriedFun: Int => (Int => Int) = ???
6+
def curriedFun: Int => Int => Int
7+
scala> def tupFun: ((Int, Int)) => Int = ???
8+
def tupFun: ((Int, Int)) => Int
9+
scala> def binFun: (Int, Int) => Int = ???
10+
def binFun: (Int, Int) => Int

‎compiler/test-resources/type-printer/infix‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,12 +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) = ???
23-
def foo: Int Mappy (Boolean && String)
39+
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]
44+
scala> def foo: Int && Boolean & String = ???
45+
def foo: Int && Boolean & String
46+
scala> def foo: (Int && Boolean) & String = ???
47+
def foo: Int && Boolean & String
48+
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 = ???
53+
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
}

‎tests/patmat/andtype-opentype-interaction.check‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
31: Pattern Match Exhaustivity: _: SealedTrait & OpenClass, _: Trait & OpenClass
44
35: Pattern Match Exhaustivity: _: SealedTrait & OpenTrait & OpenClass, _: Trait & OpenTrait & OpenClass
55
43: Pattern Match Exhaustivity: _: SealedTrait & OpenAbstractClass, _: Trait & OpenAbstractClass
6-
47: Pattern Match Exhaustivity: _: SealedTrait & OpenClass & OpenTrait & OpenClassSubclass, _: Trait & OpenClass & OpenTrait & OpenClassSubclass
6+
47: Pattern Match Exhaustivity: _: SealedTrait & OpenClass & (OpenTrait & OpenClassSubclass), _: Trait & OpenClass & (OpenTrait & OpenClassSubclass)

0 commit comments

Comments
 (0)