Skip to content

Commit e40c5df

Browse files
dwijnanddwijnand
committed
Switch our string interpolators to use Show/Shown
As it was our string interpolators were taking Any values and then trying to pattern match back their classes to try to show them nicely. This inevitably fails for things like the opaque type FlagSet, which interpolates as an indecipherable long number. Now, instead, they take "Shown" arguments, for which there is an implicit conversion in scope, given there is a Show instance for value. I captured some desired results in some new unit test cases. In the process a small handful of bugs were discovered, the only particularly bad one was consuming a Iterator when the "transforms" printer was enabled (accessorDefs), followed by an unintentional eta-expansion of a method with a defaulted argument (showSummary). I also lifted out the Showable and exception fallback function as an extension method, so I could use it more broadly. The use of WrappedResult and its `result` in `showing` was also impacted, because the new expected Shown type was driving `result`'s context lookup. Fortunately I was able to continue to use WrappedResult and `result` as defined by handling this API change inside `showing` alone. I wasn't, however, able to find a solution to the impact the new Shown expected type was having on the `stripModuleClassSuffix` extension method, sadly. JSExportsGen is interpolating a private type, so rather than open its access or giving it a Show instance I changed the string interpolation. SyntaxFormatter was no longer used, since the "hl" interpolator was dropped.
1 parent e54a934 commit e40c5df

File tree

12 files changed

+160
-73
lines changed

12 files changed

+160
-73
lines changed

‎compiler/src/dotty/tools/backend/sjs/JSExportsGen.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
119119
if (kind != overallKind) {
120120
bad = true
121121
report.error(
122-
em"export overload conflicts with export of $firstSym: they are of different types ($kind / $overallKind)",
122+
em"export overload conflicts with export of $firstSym: they are of different types (${kind.show} / ${overallKind.show})",
123123
info.pos)
124124
}
125125
}

‎compiler/src/dotty/tools/dotc/core/Decorators.scala

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ package dotty.tools
22
package dotc
33
package core
44

5-
import annotation.tailrec
6-
import Symbols._
7-
import Contexts._, Names._, Phases._, printing.Texts._
8-
import collection.mutable.ListBuffer
9-
import dotty.tools.dotc.transform.MegaPhase
10-
import printing.Formatting._
5+
import scala.annotation.tailrec
6+
import scala.collection.mutable.ListBuffer
7+
import scala.util.control.NonFatal
8+
9+
import Contexts._, Names._, Phases._, Symbols._
10+
import printing.{ Printer, Showable }, printing.Formatting._, printing.Texts._
11+
import transform.MegaPhase
1112

1213
/** This object provides useful implicit decorators for types defined elsewhere */
1314
object Decorators {
@@ -246,13 +247,30 @@ object Decorators {
246247
}
247248

248249
extension [T](x: T)
249-
def showing(
250-
op: WrappedResult[T] ?=> String,
251-
printer: config.Printers.Printer = config.Printers.default): T = {
252-
printer.println(op(using WrappedResult(x)))
250+
def showing[U](
251+
op: WrappedResult[U] ?=> String,
252+
printer: config.Printers.Printer = config.Printers.default)(using c: Conversion[T, U] = null): T = {
253+
// either the use of `$result` was driven by the expected type of `Shown`
254+
// which lead to the summoning of `Conversion[T, Shown]` (which we'll invoke)
255+
// or no such conversion was found so we'll consume the result as it is instead
256+
val obj = if c == null then x.asInstanceOf[U] else c(x)
257+
printer.println(op(using WrappedResult(obj)))
253258
x
254259
}
255260

261+
/** Instead of `toString` call `show` on `Showable` values, falling back to `toString` if an exception is raised. */
262+
def show(using Context)(using z: Show[T] = null): String = x match
263+
case _ if z != null => z.show(x).toString
264+
case x: Showable =>
265+
try x.show
266+
catch
267+
case ex: CyclicReference => "... (caught cyclic reference) ..."
268+
case NonFatal(ex)
269+
if !ctx.mode.is(Mode.PrintShowExceptions) && !ctx.settings.YshowPrintErrors.value =>
270+
val msg = ex match { case te: TypeError => te.toMessage case _ => ex.getMessage }
271+
s"[cannot display due to $msg, raw string = $x]"
272+
case _ => String.valueOf(x)
273+
256274
extension [T](x: T)
257275
def assertingErrorsReported(using Context): T = {
258276
assert(ctx.reporter.errorsReported)
@@ -269,19 +287,19 @@ object Decorators {
269287

270288
extension (sc: StringContext)
271289
/** General purpose string formatting */
272-
def i(args: Any*)(using Context): String =
290+
def i(args: Shown*)(using Context): String =
273291
new StringFormatter(sc).assemble(args)
274292

275293
/** Formatting for error messages: Like `i` but suppress follow-on
276294
* error messages after the first one if some of their arguments are "non-sensical".
277295
*/
278-
def em(args: Any*)(using Context): String =
296+
def em(args: Shown*)(using Context): String =
279297
new ErrorMessageFormatter(sc).assemble(args)
280298

281299
/** Formatting with added explanations: Like `em`, but add explanations to
282300
* give more info about type variables and to disambiguate where needed.
283301
*/
284-
def ex(args: Any*)(using Context): String =
302+
def ex(args: Shown*)(using Context): String =
285303
explained(em(args: _*))
286304

287305
extension [T <: AnyRef](arr: Array[T])

‎compiler/src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2711,15 +2711,16 @@ object TypeComparer {
27112711
*/
27122712
val Fresh: Repr = 4
27132713

2714-
extension (approx: Repr)
2715-
def low: Boolean = (approx & LoApprox) != 0
2716-
def high: Boolean = (approx & HiApprox) != 0
2717-
def addLow: Repr = approx | LoApprox
2718-
def addHigh: Repr = approx | HiApprox
2719-
def show: String =
2720-
val lo = if low then " (left is approximated)" else ""
2721-
val hi = if high then " (right is approximated)" else ""
2722-
lo ++ hi
2714+
object Repr:
2715+
extension (approx: Repr)
2716+
def low: Boolean = (approx & LoApprox) != 0
2717+
def high: Boolean = (approx & HiApprox) != 0
2718+
def addLow: Repr = approx | LoApprox
2719+
def addHigh: Repr = approx | HiApprox
2720+
def show: String =
2721+
val lo = if low then " (left is approximated)" else ""
2722+
val hi = if high then " (right is approximated)" else ""
2723+
lo ++ hi
27232724
end ApproxState
27242725
type ApproxState = ApproxState.Repr
27252726

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ object Parsers {
596596
if startIndentWidth <= nextIndentWidth then
597597
i"""Line is indented too far to the right, or a `{` is missing before:
598598
|
599-
|$t"""
599+
|${t.show}"""
600600
else
601601
in.spaceTabMismatchMsg(startIndentWidth, nextIndentWidth),
602602
in.next.offset

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

Lines changed: 80 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,103 @@
1-
package dotty.tools.dotc
1+
package dotty.tools
2+
package dotc
23
package printing
34

45
import scala.language.unsafeNulls
56

7+
import scala.collection.mutable
8+
69
import core._
710
import Texts._, Types._, Flags._, Symbols._, Contexts._
8-
import collection.mutable
911
import Decorators._
10-
import scala.util.control.NonFatal
1112
import reporting.Message
1213
import util.DiffUtil
1314
import Highlighting._
1415

1516
object Formatting {
1617

18+
object ShownDef:
19+
/** Represents a value that has been "shown" and can be consumed by StringFormatter.
20+
* Not just a string because it may be a Seq that StringFormatter will intersperse with the trailing separator.
21+
* Also, it's not a `String | Seq[String]` because then we'd need to a Context to call `Showable#show`. We could
22+
* make Context a requirement for a Show instance but then we'd have lots of instances instead of just one ShowAny
23+
* instance. We could also try to make `Show#show` require the Context, but then that breaks the Conversion. */
24+
opaque type Shown = Any
25+
object Shown:
26+
given [A: Show]: Conversion[A, Shown] = Show[A].show(_)
27+
28+
sealed abstract class Show[-T]:
29+
/** Show a value T by returning a "shown" result. */
30+
def show(x: T): Shown
31+
32+
/** The base implementation, passing the argument to StringFormatter which will try to `.show` it. */
33+
object ShowAny extends Show[Any]:
34+
def show(x: Any): Shown = x
35+
36+
class ShowImplicits1:
37+
given Show[ImplicitRef] = ShowAny
38+
given Show[Names.Designator] = ShowAny
39+
given Show[util.SrcPos] = ShowAny
40+
41+
object Show extends ShowImplicits1:
42+
inline def apply[A](using inline z: Show[A]): Show[A] = z
43+
44+
given [X: Show]: Show[Seq[X]] with
45+
def show(x: Seq[X]) = x.map(Show[X].show)
46+
47+
given [A: Show, B: Show]: Show[(A, B)] with
48+
def show(x: (A, B)) = (Show[A].show(x._1), Show[B].show(x._2))
49+
50+
given Show[FlagSet] with
51+
def show(x: FlagSet) = x.flagsString
52+
53+
given Show[TypeComparer.ApproxState] with
54+
def show(x: TypeComparer.ApproxState) = TypeComparer.ApproxState.Repr.show(x)
55+
56+
given Show[Showable] = ShowAny
57+
given Show[Shown] = ShowAny
58+
given Show[Int] = ShowAny
59+
given Show[Char] = ShowAny
60+
given Show[Boolean] = ShowAny
61+
given Show[String] = ShowAny
62+
given Show[Class[?]] = ShowAny
63+
given Show[Exception] = ShowAny
64+
given Show[StringBuffer] = ShowAny
65+
given Show[Atoms] = ShowAny
66+
given Show[Highlight] = ShowAny
67+
given Show[HighlightBuffer] = ShowAny
68+
given Show[CompilationUnit] = ShowAny
69+
given Show[Mode] = ShowAny
70+
given Show[Phases.Phase] = ShowAny
71+
given Show[Signature] = ShowAny
72+
given Show[TyperState] = ShowAny
73+
given Show[config.ScalaVersion] = ShowAny
74+
given Show[io.AbstractFile] = ShowAny
75+
given Show[parsing.Scanners.IndentWidth] = ShowAny
76+
given Show[parsing.Scanners.Scanner] = ShowAny
77+
given Show[util.SourceFile] = ShowAny
78+
given Show[util.Spans.Span] = ShowAny
79+
given Show[dotty.tools.tasty.TastyBuffer.Addr] = ShowAny
80+
given Show[tasty.TreeUnpickler#OwnerTree] = ShowAny
81+
given Show[interactive.Completion] = ShowAny
82+
given Show[transform.sjs.JSSymUtils.JSName] = ShowAny
83+
given Show[org.scalajs.ir.Position] = ShowAny
84+
end Show
85+
end ShownDef
86+
export ShownDef.{ Show, Shown }
87+
1788
/** General purpose string formatter, with the following features:
1889
*
19-
* 1) On all Showables, `show` is called instead of `toString`
20-
* 2) Exceptions raised by a `show` are handled by falling back to `toString`.
21-
* 3) Sequences can be formatted using the desired separator between two `%` signs,
90+
* 1. Invokes the `show` extension method on the interpolated arguments.
91+
* 2. Sequences can be formatted using the desired separator between two `%` signs,
2292
* eg `i"myList = (${myList}%, %)"`
23-
* 4) Safe handling of multi-line margins. Left margins are skipped om the parts
93+
* 3. Safe handling of multi-line margins. Left margins are stripped on the parts
2494
* of the string context *before* inserting the arguments. That way, we guard
2595
* against accidentally treating an interpolated value as a margin.
2696
*/
2797
class StringFormatter(protected val sc: StringContext) {
28-
protected def showArg(arg: Any)(using Context): String = arg match {
29-
case arg: Showable =>
30-
try arg.show
31-
catch {
32-
case ex: CyclicReference => "... (caught cyclic reference) ..."
33-
case NonFatal(ex)
34-
if !ctx.mode.is(Mode.PrintShowExceptions) &&
35-
!ctx.settings.YshowPrintErrors.value =>
36-
val msg = ex match
37-
case te: TypeError => te.toMessage
38-
case _ => ex.getMessage
39-
s"[cannot display due to $msg, raw string = ${arg.toString}]"
40-
}
41-
case _ => String.valueOf(arg)
42-
}
98+
protected def showArg(arg: Any)(using Context): String = arg.show
4399

44-
private def treatArg(arg: Any, suffix: String)(using Context): (Any, String) = arg match {
100+
private def treatArg(arg: Shown, suffix: String)(using Context): (Any, String) = arg match {
45101
case arg: Seq[?] if suffix.nonEmpty && suffix.head == '%' =>
46102
val (rawsep, rest) = suffix.tail.span(_ != '%')
47103
val sep = StringContext.processEscapes(rawsep)
@@ -51,7 +107,7 @@ object Formatting {
51107
(showArg(arg), suffix)
52108
}
53109

54-
def assemble(args: Seq[Any])(using Context): String = {
110+
def assemble(args: Seq[Shown])(using Context): String = {
55111
def isLineBreak(c: Char) = c == '\n' || c == '\f' // compatible with StringLike#isLineBreak
56112
def stripTrailingPart(s: String) = {
57113
val (pre, post) = s.span(c => !isLineBreak(c))
@@ -77,18 +133,6 @@ object Formatting {
77133
override protected def showArg(arg: Any)(using Context): String =
78134
wrapNonSensical(arg, super.showArg(arg)(using errorMessageCtx))
79135

80-
class SyntaxFormatter(sc: StringContext) extends StringFormatter(sc) {
81-
override protected def showArg(arg: Any)(using Context): String =
82-
arg match {
83-
case hl: Highlight =>
84-
hl.show
85-
case hb: HighlightBuffer =>
86-
hb.toString
87-
case _ =>
88-
SyntaxHighlighting.highlight(super.showArg(arg))
89-
}
90-
}
91-
92136
private def wrapNonSensical(arg: Any, str: String)(using Context): String = {
93137
import Message._
94138
def isSensical(arg: Any): Boolean = arg match {

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1751,7 +1751,7 @@ import transform.SymUtils._
17511751

17521752
class ClassAndCompanionNameClash(cls: Symbol, other: Symbol)(using Context)
17531753
extends NamingMsg(ClassAndCompanionNameClashID) {
1754-
def msg = em"Name clash: both ${cls.owner} and its companion object defines ${cls.name.stripModuleClassSuffix}"
1754+
def msg = em"Name clash: both ${cls.owner} and its companion object defines ${cls.name.stripModuleClassSuffix.show}"
17551755
def explain =
17561756
em"""|A ${cls.kindString} and its companion object cannot both define a ${hl("class")}, ${hl("trait")} or ${hl("object")} with the same name:
17571757
| - ${cls.owner} defines ${cls}

‎compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
776776
case n: Name =>
777777
h = nameHash(n, h)
778778
case elem =>
779-
cannotHash(what = i"`$elem` of unknown class ${elem.getClass}", elem, tree)
779+
cannotHash(what = i"`${elem.show}` of unknown class ${elem.getClass}", elem, tree)
780780
h
781781
end iteratorHash
782782

‎compiler/src/dotty/tools/dotc/transform/AccessProxies.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ abstract class AccessProxies {
5858

5959
/** Add all needed accessors to the `body` of class `cls` */
6060
def addAccessorDefs(cls: Symbol, body: List[Tree])(using Context): List[Tree] = {
61-
val accDefs = accessorDefs(cls)
61+
val accDefs = accessorDefs(cls).toList
6262
transforms.println(i"add accessors for $cls: $accDefs%, %")
6363
if (accDefs.isEmpty) body else body ++ accDefs
6464
}

‎compiler/src/dotty/tools/dotc/transform/DropOuterAccessors.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class DropOuterAccessors extends MiniPhase with IdentityDenotTransformer:
8080
cpy.Block(rhs)(inits.filterNot(dropOuterInit), expr)
8181
})
8282
assert(droppedParamAccessors.isEmpty,
83-
i"""Failed to eliminate: $droppedParamAccessors
83+
i"""Failed to eliminate: ${droppedParamAccessors.toList}
8484
when dropping outer accessors for ${ctx.owner} with
8585
$impl""")
8686
cpy.Template(impl)(constr = constr1, body = body1)

‎compiler/src/dotty/tools/dotc/transform/Erasure.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ object Erasure {
266266
def constant(tree: Tree, const: Tree)(using Context): Tree =
267267
(if (isPureExpr(tree)) const else Block(tree :: Nil, const)).withSpan(tree.span)
268268

269-
final def box(tree: Tree, target: => String = "")(using Context): Tree = trace(i"boxing ${tree.showSummary}: ${tree.tpe} into $target") {
269+
final def box(tree: Tree, target: => String = "")(using Context): Tree = trace(i"boxing ${tree.showSummary()}: ${tree.tpe} into $target") {
270270
tree.tpe.widen match {
271271
case ErasedValueType(tycon, _) =>
272272
New(tycon, cast(tree, underlyingOfValueClass(tycon.symbol.asClass)) :: Nil) // todo: use adaptToType?
@@ -286,7 +286,7 @@ object Erasure {
286286
}
287287
}
288288

289-
def unbox(tree: Tree, pt: Type)(using Context): Tree = trace(i"unboxing ${tree.showSummary}: ${tree.tpe} as a $pt") {
289+
def unbox(tree: Tree, pt: Type)(using Context): Tree = trace(i"unboxing ${tree.showSummary()}: ${tree.tpe} as a $pt") {
290290
pt match {
291291
case ErasedValueType(tycon, underlying) =>
292292
def unboxedTree(t: Tree) =
@@ -1031,7 +1031,7 @@ object Erasure {
10311031
}
10321032

10331033
override def adapt(tree: Tree, pt: Type, locked: TypeVars, tryGadtHealing: Boolean)(using Context): Tree =
1034-
trace(i"adapting ${tree.showSummary}: ${tree.tpe} to $pt", show = true) {
1034+
trace(i"adapting ${tree.showSummary()}: ${tree.tpe} to $pt", show = true) {
10351035
if ctx.phase != erasurePhase && ctx.phase != erasurePhase.next then
10361036
// this can happen when reading annotations loaded during erasure,
10371037
// since these are loaded at phase typer.

‎compiler/src/dotty/tools/dotc/typer/Checking.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1197,7 +1197,7 @@ trait Checking {
11971197
case _: TypeTree =>
11981198
case _ =>
11991199
if tree.tpe.typeParams.nonEmpty then
1200-
val what = if tree.symbol.exists then tree.symbol else i"type $tree"
1200+
val what = if tree.symbol.exists then tree.symbol.show else i"type $tree"
12011201
report.error(em"$what takes type parameters", tree.srcPos)
12021202

12031203
/** Check that we are in an inline context (inside an inline method or in inline code) */

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

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
package dotty.tools.dotc.printing
1+
package dotty.tools
2+
package dotc
3+
package printing
24

3-
import dotty.tools.DottyTest
4-
import dotty.tools.dotc.ast.{Trees,tpd}
5-
import dotty.tools.dotc.core.Names._
6-
import dotty.tools.dotc.core.Symbols._
5+
import ast.{ Trees, tpd }
6+
import core.Names._
7+
import core.Symbols._
8+
import core.Decorators._
79
import dotty.tools.dotc.core.Contexts.Context
810

911
import org.junit.Assert.assertEquals
@@ -49,4 +51,26 @@ class PrinterTests extends DottyTest {
4951
assertEquals("Int & (Boolean | String)", bar.tpt.show)
5052
}
5153
}
54+
55+
@Test def string: Unit = assertEquals("foo", i"${"foo"}")
56+
57+
import core.Flags._
58+
@Test def flagsSingle: Unit = assertEquals("final", i"$Final")
59+
@Test def flagsSeq: Unit = assertEquals("<static>, final", i"${Seq(JavaStatic, Final)}%, %")
60+
@Test def flagsTuple: Unit = assertEquals("(<static>,final)", i"${(JavaStatic, Final)}")
61+
@Test def flagsSeqOfTuple: Unit = assertEquals("(final,given), (private,lazy)", i"${Seq((Final, Given), (Private, Lazy))}%, %")
62+
63+
class StorePrinter extends config.Printers.Printer:
64+
var string: String = "<never set>"
65+
override def println(msg: => String) = string = msg
66+
67+
@Test def testShowing: Unit =
68+
val store = StorePrinter()
69+
(JavaStatic | Final).showing(i"flags=$result", store)
70+
assertEquals("flags=final <static>", store.string)
71+
72+
@Test def TestShowingWithOriginalType: Unit =
73+
val store = StorePrinter()
74+
(JavaStatic | Final).showing(i"flags=${if result.is(Private) then result &~ Private else result | Private}", store)
75+
assertEquals("flags=private final <static>", store.string)
5276
}

0 commit comments

Comments
 (0)