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

Jump to bottom

Class-based REPL does not support defining value classes #9910

Closed
scabug opened this issue Sep 1, 2016 · 12 comments
Closed

Class-based REPL does not support defining value classes #9910

scabug opened this issue Sep 1, 2016 · 12 comments
Assignees
@dwijnand
Labels
repl
Milestone
2.13.2

Comments

@scabug
Copy link

scabug commented Sep 1, 2016

Classes extending AnyVal cannot be defined in a REPL started with the -Yrepl-class-based option.

E.g.

scala> case class Foo(x: Int) extends AnyVal
error: value class may not be a member of another class
       case class Foo(x: Int) extends AnyVal
                  ^

without the -Yrepl-class-based option, everything works as expected.
@scabug
Copy link
Author

scabug commented Sep 1, 2016

Imported From: https://issues.scala-lang.org/browse/SI-9910?orig=1
Reporter: @jodersky
Affected Versions: 2.11.0, 2.12.0-M5
See #9799

@scabug scabug added the repl label Apr 7, 2017
@scabug scabug added this to the Backlog milestone Apr 7, 2017
@Jasper-M
Copy link
Member

This has now turned into this. (default REPL in 2.13.2-bin-8ee21e7)

scala> case class Foo(x: Int) extends AnyVal
java.lang.NumberFormatException: For input string: "1$extension"
	at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
	at java.lang.Integer.parseInt(Integer.java:580)
	at java.lang.Integer.parseInt(Integer.java:615)
	at scala.reflect.internal.StdNames$TermNames.splitDefaultGetterName(StdNames.scala:520)
	at scala.tools.nsc.typechecker.TypeDiagnostics$UnusedPrivates.rescindPrivateConstructorDefault$1(TypeDiagnostics.scala:595)
	at scala.tools.nsc.typechecker.TypeDiagnostics$UnusedPrivates.isSyntheticWarnable(TypeDiagnostics.scala:598)
	at scala.tools.nsc.typechecker.TypeDiagnostics$UnusedPrivates.isUnusedTerm(TypeDiagnostics.scala:603)
	at scala.tools.nsc.typechecker.TypeDiagnostics$UnusedPrivates.$anonfun$unusedTerms$1(TypeDiagnostics.scala:628)
	at scala.tools.nsc.typechecker.TypeDiagnostics$UnusedPrivates.unusedTerms(TypeDiagnostics.scala:628)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:76)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:63)
	at scala.reflect.api.Trees$Transformer.transformTemplate(Trees.scala:2587)
	at scala.reflect.internal.Trees$ModuleDef.$anonfun$transform$3(Trees.scala:370)
	at scala.reflect.api.Trees$Transformer.atOwner(Trees.scala:2625)
	at scala.reflect.internal.Trees$ModuleDef.transform(Trees.scala:369)
	at scala.reflect.internal.Trees$InternalTransformer.transform(Trees.scala:1461)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:72)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:63)
	at scala.reflect.api.Trees$Transformer.$anonfun$transformStats$1(Trees.scala:2614)
	at scala.reflect.api.Trees$Transformer.transformStats(Trees.scala:2612)
	at scala.reflect.internal.Trees$Template.transform(Trees.scala:518)
	at scala.reflect.internal.Trees$InternalTransformer.transform(Trees.scala:1461)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:72)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:63)
	at scala.reflect.api.Trees$Transformer.transformTemplate(Trees.scala:2587)
	at scala.reflect.internal.Trees$ModuleDef.$anonfun$transform$3(Trees.scala:370)
	at scala.reflect.api.Trees$Transformer.atOwner(Trees.scala:2625)
	at scala.reflect.internal.Trees$ModuleDef.transform(Trees.scala:369)
	at scala.reflect.internal.Trees$InternalTransformer.transform(Trees.scala:1461)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:72)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:63)
	at scala.reflect.api.Trees$Transformer.$anonfun$transformStats$1(Trees.scala:2614)
	at scala.reflect.api.Trees$Transformer.transformStats(Trees.scala:2612)
	at scala.reflect.internal.Trees$PackageDef.$anonfun$transform$1(Trees.scala:316)
	at scala.reflect.api.Trees$Transformer.atOwner(Trees.scala:2625)
	at scala.reflect.internal.Trees$PackageDef.transform(Trees.scala:316)
	at scala.reflect.internal.Trees$InternalTransformer.transform(Trees.scala:1461)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transform(ReplGlobal.scala:72)
	at scala.tools.nsc.ast.Trees$Transformer.transformUnit(Trees.scala:162)
	at scala.tools.nsc.interpreter.ReplGlobal$wrapperCleanup$WrapperCleanupTransformer.transformUnit(ReplGlobal.scala:65)
	at scala.tools.nsc.transform.Transform$Phase.apply(Transform.scala:36)
	at scala.tools.nsc.Global$GlobalPhase.applyPhase(Global.scala:451)
	at scala.tools.nsc.Global$GlobalPhase.run(Global.scala:398)
	at scala.tools.nsc.Global$Run.compileUnitsInternal(Global.scala:1506)
	at scala.tools.nsc.Global$Run.compileUnits(Global.scala:1490)
	at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.compile(IMain.scala:716)
	at scala.tools.nsc.interpreter.IMain$Request.compile(IMain.scala:928)
	at scala.tools.nsc.interpreter.IMain.compile(IMain.scala:508)
	at scala.tools.nsc.interpreter.IMain.doInterpret(IMain.scala:494)
	at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:478)
	at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:476)
	at scala.tools.nsc.interpreter.shell.ILoop.interpretStartingWith(ILoop.scala:897)
	at scala.tools.nsc.interpreter.shell.ILoop.command(ILoop.scala:754)
	at scala.tools.nsc.interpreter.shell.ILoop.processLine(ILoop.scala:428)
	at scala.tools.nsc.interpreter.shell.ILoop.loop(ILoop.scala:451)
	at scala.tools.nsc.interpreter.shell.ILoop.run(ILoop.scala:984)
	at xsbt.ConsoleInterface.run(ConsoleInterface.scala:78)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at sbt.internal.inc.AnalyzingCompiler.call(AnalyzingCompiler.scala:248)
	at sbt.internal.inc.AnalyzingCompiler.console(AnalyzingCompiler.scala:210)
	at sbt.Console.console0$1(Console.scala:48)
	at sbt.Console.$anonfun$apply$2(Console.scala:51)
	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
	at sbt.util.InterfaceUtil$$anon$1.get(InterfaceUtil.scala:10)
	at sbt.TrapExit$App.run(TrapExit.scala:257)
	at java.lang.Thread.run(Thread.java:748)

That entry seems to have slain the compiler.  Shall I replay
your session? I can re-run each line except the last one.
[y/n]

@Jasper-M Jasper-M mentioned this issue Mar 30, 2020
Closed
@dwijnand dwijnand mentioned this issue Mar 30, 2020
Merged
@lrytz lrytz modified the milestones: Backlog, 2.13.2 Mar 30, 2020
@lrytz lrytz closed this as completed Mar 30, 2020
@dwijnand dwijnand modified the milestones: 2.13.2, Backlog Mar 30, 2020
@dwijnand dwijnand reopened this Mar 30, 2020
@dwijnand dwijnand removed their assignment Mar 30, 2020
@dwijnand
Copy link
Member

Value classes "work" in the class-based REPL by interpreting snippets like case class Foo(x: Int) extends AnyVal with the object-based wrapper implementation: scala/scala@fd6ea5b. In my mind, this issue is resolved when that's removed and handled "properly".

@Jasper-M
Copy link
Member

I'm not sure there's a better way though. Is there a realistic scenario where an object wrapper for a value class can still cause a deadlock?

@smarter
Copy link
Member

smarter commented Mar 30, 2020

Not sure if this is the best place to mention this, but it's possible to have object wrappers without deadlocks, dotty has been doing that for several years: scala/scala-dev#161 (comment)

@lrytz
Copy link
Member

lrytz commented Mar 31, 2020

IIUC this comes at the cost of capturing the object in the lambda, which can cause problems when serializing the lambda.

@dwijnand
Copy link
Member

Speaking with Lukas about this, this isn't "resolved", but it's very unlikely we'll actually make this work better in the class-based REPL, so keeping it open is pointless. And I'll take the brownie points too, 🏁.

@dwijnand dwijnand modified the milestones: Backlog, 2.13.2 Mar 31, 2020
@dwijnand dwijnand self-assigned this Mar 31, 2020
@smarter
Copy link
Member

smarter commented Mar 31, 2020

IIUC this comes at the cost of capturing the object in the lambda, which can cause problems when serializing the lambda.

Correct, the fix in Dotty was to make all objects extend Serializable. This has ~zero cost since like in Scala 2, Dotty objects use writeReplace to not actually serialize anything.

@lrytz
Copy link
Member

lrytz commented Apr 1, 2020

So is it a special case only for the REPL? Capturing the repl wrapper is not a problem of course, but capturing arbitrary objects is.

@smarter
Copy link
Member

smarter commented Apr 1, 2020

No it's done for every object (since the same deadlock problem can happen for regular objects too), what kind of problem do you have in mind ?

@lrytz
Copy link
Member

lrytz commented Apr 2, 2020 •
edited

OK, so the difference to Scala 2 is that every object is made extends Serializable in dotty. So when a lambda captures the enclosing object it can still be serialized. This even works if the object has itself some non-serializable fields, because of the writeReplace -> ModuleSerializationProxy. We use the same strategy in 2.13 for serializing objects, but we don't make every object extend Serializable by default.

cc @retronym

@smarter
Copy link
Member

smarter commented Apr 2, 2020

OK, so the difference to Scala 2 is that every object is made extends Serializable in dotty.

Yep! One additional trick we considered but haven't implemented so far is to add the Serializable superclass in a phase after pickler, so that it does not influence typechecking / type inference seeing as it's "just" an implementation detail.

@vreuter vreuter mentioned this issue Mar 12, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dwijnand dwijnand

Labels
repl
Projects
None yet
Milestone
2.13.2
Development

No branches or pull requests
5 participants
@smarter @lrytz @dwijnand @Jasper-M @scabug