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 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

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug scabug commented Sep 2, 2012

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

Amended PR:
scala/scala#1230

@scabug

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug 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

This comment has been minimized.

Copy link
Author

@scabug scabug commented Sep 9, 2012

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

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Sep 9, 2012

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

@scabug

This comment has been minimized.

Copy link
Author

@scabug 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
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.