Reject failed inits

[ankitson]

This is a fix for #4416 . Its still WIP, some cleanup & style fixes needed I think, but the basic idea is here so I thought I would open it up to suggestions.

@allanrenucci It mostly does what you suggested, except we also have to patch compile to account for failures in displayDefinitions.

It doesn't seem very clean, but it is explicit about the dependency between display and the REPL state. I think it will remain a little messy until we decouple eval and rendering.

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

[ankitson]

Also found another weird behavior:

Without this patch:

scala> lazy val x = 1/0
val x: Int = <lazy>
scala> val y = x
java.lang.ArithmeticException: / by zero
	...
scala> y
java.lang.NoClassDefFoundError: Could not initialize class rs$line$2$
...
scala> x
val res1: Int = 0

With the patch:

scala> lazy val x = 1/0
val x: Int = <lazy>
scala> val y = x
java.lang.ArithmeticException: / by zero
	...
scala> y
1 |y
  |^
  |not found: y
scala> x
<rendering error>

Which seems better, maybe not ideal.

[allanrenucci]

We should stop rendering vals after the first failure, like scalac REPL:

scala> val x = 1 / 0; println("1")
java.lang.ArithmeticException: / by zero
  ... 28 elided

In the example above, println("1") is not evaluated

[allanrenucci]

And not render anything (e.g. aliases, methods) in case of failure

[allanrenucci]

We should propagate any exception that is not an InvocationTargetException

[ankitson]

@allanrenucci I managed to get this working. For some reason to do with classloaders I don't fully understand, the Rendering class gets into a bad state if we throw an exception while evaluating, and this happens:

scala> val x = 1/0
java.lang.ArithmeticException: / by zero
...
scala> val y = 1/0

scala> def f = y
def f: Int

My PR now creates a new Rendering upon failure.

[odersky]

@allanrenucci Since this was not re-assigned to you it probably fell through the cracks. Can you take a look and decide whether this can go in now? Thanks

[OlivierBlanvillain]

@ankitson Sorry for the long silence. Do still plan to work on this PR? The latest commits seems like a hack rather than a clean solution; a var with a scala.util.control.Breaks.{break, breakable}, we are not going to merge a diff like that...

[OlivierBlanvillain]

Closing for now, feel free to reopen if you plan to further work on it!