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

post-match mutation must not affect pattern variable #5158

Closed
scabug opened this issue Nov 7, 2011 · 10 comments
Closed

post-match mutation must not affect pattern variable #5158

scabug opened this issue Nov 7, 2011 · 10 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Nov 7, 2011

case class B(var x: Int) {
  def succ() {
    x = x + 1
  }
}

object TestFoo {
  def main(args: Array[String]) {
    val b = B(0)
    b match {
      case B(x) =>
        //println(x)
        b.succ()
        println(x)
    }
  }
}

This code prints "1" instead of "0". However if you uncomment first println you will get what expected "0\n0".

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 7, 2011

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 15, 2012

@retronym said:
@adriaanm rather than tinkering around this to extract the non-val case class parameters eagerly, I think we should just revert to using B.unapply.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 19, 2012

@adriaanm said:
unfortunately that's expensive, or do you propose we only do it for case classes with mutable variables?

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 19, 2012

@retronym said:
Yep, definitely keep the optimized version for case classes without vars. You could also do the same if it has a var parameters, but that isn't bound in the pattern.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 12, 2012

@dragos said:
I don't see what's being optimized. The number of local variables? I really doubt that has any benefit at runtime, and code size is definitely increased when there's more than one reference to a pattern variable. But all these considerations are offset by making debugging way more difficult that it was before.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 12, 2012

@adriaanm said:
agreed -- this is what I'm implementing now (one local var per pattern var)

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 12, 2012

@retronym said:
By optimized, I just meant avoiding the call to unapply for a case class.

+1 for local variables.

Adriaan: do you plan to add locals for bound names that are used in the body, or for all bound names? The latter would be convenient while debugging.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 12, 2012

@adriaanm said:
I'd just add all of them and let the optimizer do the rest.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 12, 2012

@retronym said:
One more convenience for us dirty debuggers:

scrut match {
  case (Foo(x, y), _) =>
    // generate a var for constructor patterns
    val foo$1: Foo

    // in addition to the user provided binders
    val x: Int
    val y: String
}
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 18, 2012

@adriaanm said (edited on Jul 20, 2012 8:51:03 AM UTC):
scala/scala#937

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.