Skip to content

Commit

Permalink
Fix Formatting.Show for types X | Null.
Browse files Browse the repository at this point in the history
Once we saw what got changed we got priority errors like this:
```scala
[warn] -- Warning: /Users/odersky/workspace/dotty/compiler/src/dotty/tools/dotc/reporting/messages.scala:2145:15
[warn] 2145 |        |    ${Magenta("case a @ C()")} => 2
[warn]      |               ^
[warn]      |Change in given search preference for dotty.tools.dotc.printing.Formatting.ShownDef.Show[Product | Null] between alternatives (dotty.tools.dotc.printing.Formatting.ShownDef.Show.given_Show_Product :
[warn]      |  dotty.tools.dotc.printing.Formatting.ShownDef.Show[Product]) and (dotty.tools.dotc.printing.Formatting.ShownDef.Show.oldShowNull :
[warn]      |  [X]
[warn]      |    (implicit evidence$1: dotty.tools.dotc.printing.Formatting.ShownDef.Show[X])
[warn]      |      : dotty.tools.dotc.printing.Formatting.ShownDef.Show.oldShowNull[X]
[warn]      |)
[warn]      |Previous choice          : the first alternative
[warn]      |New choice from Scala 3.6: the second alternative
```
I believe there is some funky thing going on with nulls. Probably unsafe nulls somewhere. Anyway what seems to happen is that before Product's given was selected for an argument of `Product | Null`, which seems to indicate that the two types were seen as equivalent. Anyway, if we compare
`Show[Product]` with `Show[X | Null]` it seems that Show[Product] won the type score and lost the owner score, so it was a draw. But then the second criterion kicked in which rules that alternatives without any implicit arguments take priority over alternatives with explicit
arguments. The given for Product was unconditional but the given for X | Null has a context bound. So Show[Product] won.

But once we switch to more general, we have that Show[Product] loses both type and owner score, so Show[X | Null] won. And that seems to have
caused the classcast exceptions. I am not sure why, the logic for Shown was too hard for me to follow. The fix is to move Show[X | Null] to have
lowest priority.
  • Loading branch information
odersky committed Jul 29, 2024
1 parent 1ea5485 commit 8f85da5
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 11 deletions.
20 changes: 9 additions & 11 deletions compiler/src/dotty/tools/dotc/printing/Formatting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package dotty.tools
package dotc
package printing

import scala.language.unsafeNulls

import scala.collection.mutable

import core.*
Expand Down Expand Up @@ -50,9 +48,14 @@ object Formatting {

/** The base implementation, passing the argument to StringFormatter which will try to `.show` it. */
object ShowAny extends Show[Any]:
def show(x: Any): Shown = x
def show(x: Any): Shown =
if x == null then "null" else x // defensive action in case `showNull` does not trigger because of unsafeNulls

class ShowImplicit4:
given showNull[X: Show]: Show[X | Null] with
def show(x: X | Null) = if x == null then "null" else CtxShow(toStr(x.nn))

class ShowImplicits3:
class ShowImplicits3 extends ShowImplicit4:
given Show[Product] = ShowAny

class ShowImplicits2 extends ShowImplicits3:
Expand All @@ -77,15 +80,10 @@ object Formatting {
given [K: Show, V: Show]: Show[Map[K, V]] with
def show(x: Map[K, V]) =
CtxShow(x.map((k, v) => s"${toStr(k)} => ${toStr(v)}"))
end given

given [H: Show, T <: Tuple: Show]: Show[H *: T] with
def show(x: H *: T) =
CtxShow(toStr(x.head) *: toShown(x.tail).asInstanceOf[Tuple])
end given

given [X: Show]: Show[X | Null] with
def show(x: X | Null) = if x == null then "null" else CtxShow(toStr(x.nn))

given Show[FlagSet] with
def show(x: FlagSet) = x.flagsString
Expand Down Expand Up @@ -148,8 +146,8 @@ object Formatting {
private def treatArg(arg: Shown, suffix: String)(using Context): (String, String) = arg.runCtxShow match {
case arg: Seq[?] if suffix.indexOf('%') == 0 && suffix.indexOf('%', 1) != -1 =>
val end = suffix.indexOf('%', 1)
val sep = StringContext.processEscapes(suffix.substring(1, end))
(arg.mkString(sep), suffix.substring(end + 1))
val sep = StringContext.processEscapes(suffix.substring(1, end).nn)
(arg.mkString(sep), suffix.substring(end + 1).nn)
case arg: Seq[?] =>
(arg.map(showArg).mkString("[", ", ", "]"), suffix)
case arg =>
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotty/tools/dotc/StringFormatterTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class StringFormatterTest extends AbstractStringFormatterTest:
@Test def flagsTup = check("(<static>,final)", i"${(JavaStatic, Final)}")
@Test def seqOfTup2 = check("(final,given), (private,lazy)", i"${Seq((Final, Given), (Private, Lazy))}%, %")
@Test def seqOfTup3 = check("(Foo,given, (right is approximated))", i"${Seq((Foo, Given, TypeComparer.ApproxState.None.addHigh))}%, %")
@Test def tupleNull = check("(1,null)", i"${(1, null: String | Null)}")

class StorePrinter extends Printer:
var string: String = "<never set>"
Expand Down

0 comments on commit 8f85da5

Please sign in to comment.