Since this fixes #4415, can you add a test for it:

scala> lazy val x = ???
val x: Nothing = <lazy>

I'm not convinced having a few hardcoded cases is better than a Show typeclass. I agree that the Show typeclass isn't ideal if it's only used by the REPL, but if we agree that having a standard Show typeclass in the standard library of Scala is useful, then it makes a lot of sense to use it for the REPL.

/cc @felixmulder Maybe you want to defend Show too :).

Obviously the repl should only be able to show things that have a Show instance 😜

But I'm not emotionally attached to it even though I agree with Guillaume's comment :)

A proper Show type class would clearly be better, but what we have right now is simply a fancy way to hardcoded a few cases with a fallback to .toString. Using implicit resolution makes things more fragile and somewhat less consistent (would you have guessed the output of my code snippet?). Given that we've already had a couple of issues related to Show, the proposal is to burn it with fire :)

So to fix Show, you'd have to both fix implicit resolution and remove the default value (thus no Java show - how would we solve that anyway with a principled type class 😭)?

If that's correct then we'd still have to fix implicit resolution to accommodate contravariant typeclasses at some point?

we'd still have to fix implicit resolution to accommodate contravariant typeclasses at some point?

This is already "fixed" in Dotty compared to scalac, unless you have something else in mind than what Dotty does now? See #4329

@smarter - that's what I thought as well. So I'm sort of curious what part is broken about our Show when it comes to "fickle implicits". 🙂

Maybe it's the base case for Any that needs to disappear?

There isn't anything to fix about implicits, they are just the wrong tool for the job. I think people are used to have the REPL produce Strings for everything:

scala> class Foo
defined class Foo

scala> val a = new Foo
a: Foo = Foo@377008df

Unless we plan to change the above (and update the entire Scala ecosystem to provide instances of Show), we shouldn't use a typeclass.

For reference, here is what you get in ghci if you forget the deriving Show:

Prelude> data Shape = Circle Float Float Float | Square Float -- deriving Show
Prelude> Circle 10 20 5 

<interactive>:2:1: error:
    • No instance for (Show Shape) arising from a use of ‘print’
    • In a stmt of an interactive GHCi command: print it

If you just want feature parity with Scala 2 and to close some minor bugs - then yes, you're absolutely right.

I think we can do better with Dotty, but I leave the how up to you 😀

I think the best implementation of Show that we have is http://www.lihaoyi.com/PPrint/, unsurprisingly, but it relies on current macros.

Re variance, it seems Show should be made contravariant, and that would fix the issue with Show[Some[T]]. Testing just the implicit search in Dotty:

trait Show[-T] { def show(x: T): String }
implicit def showOption[T]: Show[Option[T]] = new Show { def show(x: Option[T]) = "showOption" } // XXX
implicitly[Show[Some[String]]].show(Some(""))

Of course, that wouldn't fix the issue with showing values upcast to Any.

Unless we plan to change the above (and update the entire Scala ecosystem to provide instances of Show), we shouldn't use a typeclass.

Yes. The de-facto policy seems that we're trying to keep the same library as Scala2.

@Blaisorblade Show used to be contravariant but was changed in cee2b50#diff-4c776b5d5db3fc261f0abe59b7a1a803

We got ambiguous implicit errors when Nothing was inferred:

val x = List() // error ambiguous implicit. found charShow and floatShow...

We got ambiguous implicit errors when Nothing was inferred:

I see why, but adding a more specific Show[Nothing] instance should fix this. Turns out it doesn't, but I'd wonder if that's either a specification or implementation bug — wouldn't it affect any contravariant typeclass? I'd certainly like to define class Ord[-T], Ord[List[T]] and Ord[Set[T]] (either in some extra library, or in the future in the standard library).

(EDIT: in the example below instName is just a cheap way to test the found instance).

trait Show[-T] { def show(x: T): String; def instName: String }
implicit def showOption[T]: Show[Option[T]] = new Show { def show(x: Option[T]) = "showOption"; def instName = "showOption" } // XXX
scala> implicitly[Show[Nothing]].instName
val res5: String = "showOption"
scala> implicit def showNothing: Show[Nothing] = new Show[Nothing] { def show(x: Nothing) = ""; def instName = "showNothing" }
implicit val showNothing: Show[Nothing]
scala> implicitly[Show[Nothing]].instName
val res6: String = "showNothing"

scala> implicit def showList[T]: Show[List[T]] = new Show { def show(x: List[T]) = "showList"; def instName = "showList" } // XXX
implicit val showList[T] => Show[List[T]]
scala> implicitly[Show[Nothing]].instName
1 |implicitly[Show[Nothing]].instName
  |                         ^
  |ambiguous implicit arguments: both method showList and method showNothing match type Show[Nothing] of parameter ev of method implicitly in object DottyPredef

wouldn't it affect any contravariant typeclass?

The pattern is called implicit spaghetti, people try to avoid it ;)

Seriously, if you need contravariance + an instance for Nothing, it's really not a typeclass...

Why not? There is nothing wrong with instances for Nothing.

new Show[Nothing] {
  def show(n: Nothing): String = n
}

is a perfectly valid Show.

I would not add Show[Any] though, that is evil 😉

The main reason people don't put variance on typeclasses in Scala2 is because implicit resolution w/ variance simply doesn't work there in most cases.

Right, I mixed the two, the non sense is a Nothing instance for a covariant typeclass and an Any instance for a contravarant one 😄

@alexknvl Opened a separate issue on typeclass/implicit resolution.

Note that Ammonite made the same decision and moved it's pretty-printing (https://github.com/lihaoyi/PPrint) to pprintln(x: Any) and based on asInstanceOf rather than an typeclass. You can configure it by manually registering Class[_] => pprint.Tree handlers, rather than relying on implicit resolution.

While it would be nice to "fix" implicit resolution to make Show always work, I highly doubt you'll be able to get implicit resolution stable enough to work 100% of the time in the presence of all the various type-system tricks that people and libraries use. Getting it working 100% of the time is necessary for something as simple as showing the result of a value at the REPL; people do not like it when things do not compile for a feature they never asked for!

In practice, manually recursing over Any using asInstanceOf checks works well enough, is 100% stable (falling back to .toString for un-handled cases), and have the advantage of printing Seq(1, 2, 3) and Seq(1, 2, 3): Any the same way

@Blaisorblade your information is out of date, PPrint has been macro-free for almost exactly a year now. In fact it could probably be rewritten in Java if necessary; it doesn't require fancy Scala stuff at all. I would be highly surprised if you couldn't copy-paste the 10 files of the pprint codebase into dotc and have it compile out of the box

@lihaoyi Ah thanks for the correction, I didn't find that in http://www.lihaoyi.com/PPrint/#TPrint, and I might have been misled by https://github.com/lihaoyi/PPrint/blob/005a51a509bae088dc69c4427a27280249ce8cf6/pprint/shared/src/main/scala-2.10%2B/TPrintImpl.scala. Is that dead code?

have the advantage of printing Seq(1, 2, 3) and Seq(1, 2, 3): Any the same way

👍 yes, I think that's crucial for a REPL, the typeclass issues are probably more relevant for other typeclasses.

@Blaisorblade oh TPrint is used just for printing the type signature, not the actual value. For some reason that hasn't caused any stability issues like the typeclass-based value pretty-printer has

If you want to pretty-print values in dotty, just delete the TPrint source files when you copy-paste

Having something similar to PPrint for the Dotty REPL would be nice. This can be implemented in a separate PR.

I have a dead PR against scala/scala to "Introduce ReplPrinter as a hook for alternatives to replStringOf" that might be useful inspiration: scala/scala#5222.