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

unapply permutes case class fields #7035

Closed
scabug opened this issue Jan 28, 2013 · 4 comments
Closed

unapply permutes case class fields #7035

scabug opened this issue Jan 28, 2013 · 4 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Jan 28, 2013

ticket/5082 ~/code/scala2 scala29
Welcome to Scala version 2.9.1.final (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_27).
Type in expressions to have them evaluated.
Type :help for more information.

scala> case class Y(final var x: Int, final private var y: String, final val z1: Boolean, final private val z2: Any)
defined class Y

scala> def x = Y.unapply(null)
x: Option[(Int, String, Boolean, Any)]

scala> ticket/5082 ~/code/scala2 scala-hash v2.10.0
[info] v2.10.0 => /Users/jason/usr/scala-v2.10.0-0-g18481ce
Welcome to Scala version 2.10.0-20121205-112020-18481cef9b (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_27).
Type in expressions to have them evaluated.
Type :help for more information.

scala> case class Y(final var x: Int, final private var y: String, final val z1: Boolean, final private val z2: Any)
defined class Y

scala> def x = Y.unapply(null)
x: Option[(Int, Boolean, String, Any)]

scala> val Y(a, b, c, d) = Y(0, "", true, new {})
a: Int = 0
b: String = ""
c: Boolean = true
d: Any = $anon$1@56801919

scala> Y.unapply(Y(0, "", true, new {}))
res0: Option[(Int, Boolean, String, Any)] = Some((0,true,,$anon$1@2eac0b4))

See also #5082. My proposed patch for it happens to reverse this regression.

Pattern matching, which bypasses the synthesized unapply method seems, superficially, to be immune. I seem to remember some code in there that corrected for out-of-order case accessors, but I can't find it. It does use caseAccessors when producing counter-examples, which may be subject to this bug, too.

@scabug
Copy link
Author

@scabug scabug commented Jan 28, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7035?orig=1
Reporter: @retronym
Affected Versions: 2.10.0

Loading

@scabug
Copy link
Author

@scabug scabug commented Jan 28, 2013

@retronym said:

      override protected def tupleSel(binder: Symbol)(i: Int): Tree = { import CODE._
        // caseFieldAccessors is messed up after typers (reversed, names mangled for non-public fields)
        // TODO: figure out why...
        val accessors = binder.caseFieldAccessors
        // luckily, the constrParamAccessors are still sorted properly, so sort the field-accessors using them
        // (need to undo name-mangling, including the sneaky trailing whitespace)
        val constrParamAccessors = binder.constrParamAccessors

Loading

@scabug
Copy link
Author

@scabug scabug commented Jan 28, 2013

@retronym said:

    val originalAccessors = clazz.caseFieldAccessors
    // private ones will have been renamed -- make sure they are entered
    // in the original order.
    def accessors = clazz.caseFieldAccessors sortBy { acc =>
      originalAccessors indexWhere { orig =>
        (acc.name == orig.name) || (acc.name startsWith (orig.name append "$"))
      }
    }

Loading

@scabug
Copy link
Author

@scabug scabug commented Jan 28, 2013

Loading

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

Successfully merging a pull request may close this issue.

None yet
2 participants