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

Invisible forward reference when using implicit class #6278

Closed
scabug opened this issue Aug 23, 2012 · 13 comments
Closed

Invisible forward reference when using implicit class #6278

scabug opened this issue Aug 23, 2012 · 13 comments

Comments

@scabug
Copy link

scabug commented Aug 23, 2012

Implicit def synthesis from an implicit class suffers from the dread forward reference bug.

object tiny {

  def main(args: Array[String]) {
    ok(); nope()
  }
  def ok() {
    class Foo(val i: Int) {
      def foo[A](body: =>A): A = body
    }
    implicit def toFoo(i: Int): Foo = new Foo(i)

    val k = 1
    k foo println("k?")
    val j = 2
  }
  def nope() {
    implicit class Foo(val i: Int) {
      def foo[A](body: =>A): A = body
    }

    val k = 1
    k foo println("k?")
    //lazy
    val j = 2
  }
}
@scabug
Copy link
Author

scabug commented Aug 23, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6278?orig=1
Reporter: @som-snytt
Affected Versions: 2.10.0
Other Milestones: 2.10.0, 2.11.0-M1
See #6227, #2489

@scabug
Copy link
Author

scabug commented Aug 23, 2012

@som-snytt said:
I've added a case to the addSynthetics code which is (ironically) sorting the implicit def to the end of the class body.

"Pretty ugly", that useful oxymoron; but for now, limited to the three cases.

As nice issue tracker feature would be: when I create a ticket, let me post my test code which gets put immediately into the repo under tests/pending with the useful file name; then PR acceptance means that test passes and the test is moved to non-pending and the ticket is closed.

@scabug
Copy link
Author

scabug commented Aug 23, 2012

@som-snytt said (edited on Sep 2, 2012 1:04:06 AM UTC):
scala/scala#1186 (strike out, in both senses)
67281a1e7b14a6b6bc4bcc6b1454fcbb6d47e105

@scabug
Copy link
Author

scabug commented Aug 23, 2012

@som-snytt said (edited on Aug 23, 2012 6:59:57 PM UTC):
Linked to #6227 because this fix depends on naming convention.

@scabug
Copy link
Author

scabug commented Sep 1, 2012

@jsuereth said:
Note: Errors with:

scala> object tiny {
     | 
     |   def main(args: Array[String]) {
     |     ok(); nope()
     |   }
     |   def ok() {
     |     class Foo(val i: Int) {
     |       def foo[A](body: =>A): A = body
     |     }
     |     implicit def toFoo(i: Int): Foo = new Foo(i)
     | 
     |     val k = 1
     |     k foo println("k?")
     |     val j = 2
     |   }
     |   def nope() {
     |     implicit class Foo(val i: Int) {
     |       def foo[A](body: =>A): A = body
     |     }
     | 
     |     val k = 1
     |     k foo println("k?")
     |     //lazy
     |     val j = 2
     |   }
     | }
<console>:28: error: forward reference extends over definition of value j
           k foo println("k?")

@scabug
Copy link
Author

scabug commented Sep 2, 2012

@som-snytt said:
Thanks, Josh, for adding the error message.

Amended PR:
scala/scala#1230

@scabug
Copy link
Author

scabug commented Sep 9, 2012

@retronym said:
This PR was merged to master. If this bug is marked critical, it should also go to 2.10.x.

@scabug
Copy link
Author

scabug commented Sep 9, 2012

@som-snytt said:
Is a PR against 2.10 required, or does an oompa-loompa do the merge? Thx.

@scabug
Copy link
Author

scabug commented Sep 9, 2012

@retronym said:
You'd better submit a PR, just in case they are too busy staving off the hordes of snozzwangers and whangdoodles.

@scabug
Copy link
Author

scabug commented Sep 9, 2012

@som-snytt said:
Back-applied patch to 2.10.x.

scala/scala#1277

Learned about git format-patch and git am. Was that the most appropriate workflow?

A vermicious knid ate my patch file.

@scabug
Copy link
Author

scabug commented Sep 9, 2012

@retronym said:
I'd have gone for git co 2.10.x; git cherry-pick <hash>.

@scabug
Copy link
Author

scabug commented Sep 9, 2012

@som-snytt said:
Thanks. I thought when you guys talked about cherry picking commits, you were speaking idiomatically.

@scabug
Copy link
Author

scabug commented Sep 11, 2012

@som-snytt said:
This form of the fix was merged on trunk and 2.10.x.

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

No branches or pull requests

2 participants