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

Implicit resolution doesn't work with -Yrepl-class-based in REPL #9799

Open
scabug opened this Issue Jun 1, 2016 · 13 comments

Comments

Projects
None yet
1 participant
@scabug
Copy link

scabug commented Jun 1, 2016

The following codes failed when enabling -Yrepl-class-based.

$ build/quick/bin/scala -Yrepl-class-based
Welcome to Scala 2.12.0-20160601-124948-128ac65991 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_45).
Type in expressions for evaluation. Or try :help.

scala> :paste
// Entering paste mode (ctrl-D to finish)

object X {
  implicit def bar(a: Array[String]) = new Foo(a)
  class Foo(a: Array[String]) {
    def toX: Seq[String] = a.toSeq
  }
}

// Exiting paste mode, now interpreting.

warning: there was one feature warning; re-run with -feature for details
defined object X

scala> import X._
import X._

scala> :paste
// Entering paste mode (ctrl-D to finish)

class Bar(s: String) {
  val x = Array(s).toX
}

// Exiting paste mode, now interpreting.

<console>:12: error: value toX is not a member of Array[String]
         val x = Array(s).toX
                          ^

scala> 

The above codes work well when disabling -Yrepl-class-based

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jun 1, 2016

Imported From: https://issues.scala-lang.org/browse/SI-9799?orig=1
Reporter: Shixiong Zhu (zsxwing)
Affected Versions: 2.11.8
See #7722, #9880, #9740, #9881, #7747

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jun 1, 2016

Shixiong Zhu (zsxwing) said (edited on Jun 1, 2016 10:03:29 PM UTC):
The issue is this assumption is wrong: While defining classes in class based mode - implicits are not needed
https://github.com/scala/scala/blob/cf38f078ad3bda0127e741a18f66d2c02d77b05a/src/repl/scala/tools/nsc/interpreter/Imports.scala#L118

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jun 2, 2016

@som-snytt said:
Commentary in the commit message

scala/scala@a3bb887

and earlier PR

scala/scala#4493

"This stuff is really subtle." -- scala/scala#4522

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jun 3, 2016

Shixiong Zhu (zsxwing) said:
scala/scala#5210

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jun 6, 2016

Prashant Sharma (scrapcodes) said (edited on Jun 6, 2016 10:19:15 AM UTC):
So the solution proposed in PR 5210 will lead to re-initializing of a class declarations and all things even unrelated to the class declaration on remote machines.

@som-snytt Do we have a way to know what all implicit imports are required by a particular expression ? The same way as referencedNames in MemberHandler class.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jun 6, 2016

@som-snytt said (edited on Jun 6, 2016 8:06:44 PM UTC):
Extended discussion should happen on gitter or mailing list.

Unused imports don't affect binaries; I don't know if the problem you're asking about relates to remote execution or remote compilation.

To respond to the question about limiting imports, I could speculate that -Ywarn-unused-import could be used to strip unused imports from the generated source. That is, compile with eager imports, check the warnings, strip unused imports and then recompile. (Linking the issue that would benefit from that mode.)

The issue linked from the PR: https://issues.apache.org/jira/browse/SPARK-1199

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 17, 2016

Shixiong Zhu (zsxwing) said (edited on Aug 17, 2016 6:02:03 PM UTC):
Another problem caused by this import optimization reported in https://issues.apache.org/jira/browse/SPARK-17103 :

scala -Yrepl-class-based

scala> import java.io.File
import java.io.File

scala> class Test {val f=new File(".")}
<console>:11: error: not found: type File
       class Test {val f=new File(".")}

If we cannot get the correct wanted imports, I would suggest just removing this wrong optimization.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 17, 2016

@som-snytt said:
The related issue is the other example mentioned here.

For implicits, one idea is to take the candidate imports and add them until it compiles. That would change the possibly rare use case of imports introducing ambiguous implicits. Better would be improved control of import history, which was proposed once.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 17, 2016

@som-snytt said:
So with respect to the two spark tickets, those correspond to these two tickets, which are different. The other issue is just about inspecting what members are referenced by an import, whereas this is about when to include an import. Trying to compute whether an import introduces a useful implicit may be too error prone; an alternative to adding imports until it compiles, is to pre-compile with -Ywarn-unused-imports and strip out what is unused. There is a ticket about that warning getting triggered by superfluous imports in REPL.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 22, 2016

Shixiong Zhu (zsxwing) said:
Hm..., this issue seems worse than I thought. It will ignore all wildcard imports. E.g.,

bin/scala -Yrepl-class-based
Welcome to Scala 2.11.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_45).
Type in expressions for evaluation. Or try :help.

scala> import java.text._
import java.text._

scala> class Foo() { val f = new SimpleDateFormat("hh") }
<console>:11: error: not found: type SimpleDateFormat
       class Foo() { val f = new SimpleDateFormat("hh") }
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 22, 2016

@som-snytt said:
I agree that it's a crisis for spark-shell. I don't know how it is usable. Transitively, it is a scala crisis.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 24, 2016

Denis Bolshakov (dbolshak) said:
@zsxwing Looks like scala's jira has the similar issue already resolved in scala 2.11.9.
Please checkout #9734

Could you please check that https://issues.apache.org/jira/browse/SPARK-17103 is reproducible with scala 2.11.9?

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 24, 2016

Shixiong Zhu (zsxwing) said:
[~dbolshak] That's a different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment