Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REPL: values that fail to initialise should not be accessible in later runs #4416

Closed
allanrenucci opened this issue Apr 29, 2018 · 4 comments · Fixed by #14702
Closed

REPL: values that fail to initialise should not be accessible in later runs #4416

allanrenucci opened this issue Apr 29, 2018 · 4 comments · Fixed by #14702
Assignees
Milestone

Comments

@allanrenucci
Copy link
Contributor

scala> val x = 1 / 0 
java.lang.ArithmeticException: / by zero
	at rs$line$5$.<init>(rs$line$5:1)
	at rs$line$5$.<clinit>(rs$line$5)
	at rs$line$5.xShow(rs$line$5)
        ...

scala> def foo = x // should not compile
def foo: Int

scala> x // should not compile
java.lang.NoClassDefFoundError: Could not initialize class rs$line$5$
	at rs$line$8$.<init>(rs$line$8:1)
	at rs$line$8$.<clinit>(rs$line$8)
	at rs$line$8.res3Show(rs$line$8)
@ankitson
Copy link
Contributor

I want to try this.

It looks like the problem is that a successful compile adds definitions to the REPL state, but they are evaluated only after the compile by the renderer when it tries to show them. I am thinking of patching the compile method to evaluate all values added to definitions (like in Rendering.renderVal) before returning success. Does that make sense?

@allanrenucci
Copy link
Contributor Author

Good analysis! I suggest you patch displayDefinitions
https://github.com/lampepfl/dotty/blob/17c4497455de59f56c4704053608599fa4f225b4/compiler/src/dotty/tools/repl/ReplDriver.scala#L249

It should probably return the old state if a failure occurs during evaluation. I think renderVal should return a Try[String]
https://github.com/lampepfl/dotty/blob/17c4497455de59f56c4704053608599fa4f225b4/compiler/src/dotty/tools/repl/Rendering.scala#L73

@allanrenucci
Copy link
Contributor Author

Ideally, I think we should decouple rendering and evaluation. But it's probably best implemented as another PR

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Dec 10, 2018

Just run today into a likely variant:

scala> val x = ???; val y = x
scala.NotImplementedError: an implementation is missing
	at scala.Predef$.$qmark$qmark$qmark(Predef.scala:284)
	at rs$line$1$.<init>(rs$line$1:1)
	at rs$line$1$.<clinit>(rs$line$1)
	at rs$line$1.x(rs$line$1)
	...

java.lang.NoClassDefFoundError: Could not initialize class rs$line$1$
	at rs$line$1.y(rs$line$1)
	...

Edited(@allanrenucci) for simplicity: REPL shouldn't try to initialise y if it failed to initialise x

@griggt griggt self-assigned this Mar 4, 2022
griggt added a commit to griggt/dotty that referenced this issue Mar 16, 2022
The right hand side of value definitions in the REPL are computed in the static
initializer for the wrapper object created for that input line (e.g. rs$line$1).
If any of these definitions throws an exception, the wrapper class will fail to
initialize, and further attempts to use the class will throw NoClassDefFoundError.

In this commit, we avoid all reflective access on a wrapper class once we notice
that it failed to initialize, and mark that wrapper object as invalid in the REPL
state. We discard all input from the failed wrapper (which may have been multi-line
containing many statements and definitions); any types, terms, aliases, or imports
defined there will not override any existing with the same name, and will not be
accessible in subsequent runs.

Fixes scala#4416
Fixes scala#14473
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants