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

Case classes exhibit bizarre, order-dependent behavior when members are backquoted #8831

Closed
scabug opened this issue Aug 30, 2014 · 16 comments · Fixed by scala/scala#9008
Closed
Assignees
Labels
backend compiler crash fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) help wanted minimized
Milestone

Comments

@scabug
Copy link

scabug commented Aug 30, 2014

Scala version used: 10.4.2

I have encountered what I am fairly sure is broken behavior in case classes when members are declared using backquotes.

I have the following minimized test case:

object HelloScalaBug {

  case class wrong(`a b`: Int, a: Int)
  case class right(a: Int, `a b`: Int)

  def main(args: Array[String]) {
    val w = wrong(1, 2) // Should produce a 'wrong' with `a b`=1 and a=2.  Actually produces a 'wrong' w/ `a b`=1 and a=1
    val r = right(2, 1) // Produces a 'right' with `a b`=1 and a=2


    // You would think the above would produce objects whose respective fields were equal ...

    if (r.a == w.a) {
      println ("Test passes")
    } else {
      println ("Test fails")
    }
  }
}
@scabug
Copy link
Author

scabug commented Aug 30, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8831?orig=1
Reporter: Dayton Williams (LrdDimwit)
Attachments:

@scabug
Copy link
Author

scabug commented Aug 30, 2014

Dayton Williams (LrdDimwit) said:
What actually happens when you construct a 'wrong' is that the value of wrong.a is set to the value of the parameter corresponding to wrong.a b.

If you run the preceding in a debugger, you will see that both fields of w have the value 1.

However, if you change the order of declaration of the fields in this case class - the 'right' class - then this behavior disappears, and the values are set as you would expect.

I admit I'm a bit of a scala newbie, so there may be some caveat I don't know about saying "don't do that". This minimized test case actually comes from using json4s to parse JSON, the keys of which contain spaces. This condition came up in some of the JSON I was trying to parse, and I finally tracked down the causes of my failing tests to be this issue.

@scabug
Copy link
Author

scabug commented Aug 30, 2014

@som-snytt said:
Freaky.

scala> case class wrong(`a b`: Int, a: Int)
defined class wrong

scala> wrong(1, 2)
res6: wrong = wrong(1,1)

scala> .`a b`
res7: Int = 1

scala> res6.a
res8: Int = 1

scala> .`a beans

Plus, code completion tab after the b yields what you see here.

But there's an issue for backticks in REPL.

@scabug
Copy link
Author

scabug commented Sep 3, 2014

Dayton Williams (LrdDimwit) said:
Further update: If the types of a b and "a" are not the same, the behavior is even worse:

Using the same scala interpreter I used before:

scala> case class verywrong(a b: Int, a: String)

[...]
uncaught exception during compilation: scala.reflect.internal.Types$TypeError
scala.reflect.internal.Types$TypeError: type mismatch;
found : Int
required: String
[...]
That entry seems to have slain the compiler. Shall I replay
your session? I can re-run each line except the last one.

I have attached the full log of the session.

@scabug
Copy link
Author

scabug commented Mar 2, 2015

@refried said:

Welcome to Scala version 2.11.5 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_25).
Type in expressions to have them evaluated.
Type :help for more information.

scala> case class right3a(a: Int, `a b`: Int, b: Int)
defined class right3a

scala> right3a(1,2,3)
res0: right3a = right3a(1,2,3)

scala> case class right3b(b: Int, `a b`: Int, a: Int)
defined class right3b

scala> right3b(1,2,3)
res1: right3b = right3b(1,2,2)

scala> case class right4(b: Int, `a b`: Int, a: Int, `a `: Int)
defined class right4

scala> right4(1,2,3,4)
res2: right4 = right4(1,2,2,4)

scala> case class right5(b: Int, `a b`: Int, a: Int, `a `: Int, `a b c`: Int)
defined class right5

scala> right5(1,2,3,4,5)
res3: right5 = right5(1,2,5,2,4) // lolwut

scala> 

@scabug
Copy link
Author

scabug commented Mar 4, 2015

Jens Moeller (jensmoeller) said:
I looked into the issue a bit, and I think I found the main cause and a potential fix (though I am uncertain about the fix).

The source of the issue seems to be: https://github.com/scala/scala/blob/2.11.x/src/compiler/scala/tools/nsc/transform/Constructors.scala#L501, where the wrong parameter can be matched when the names involved include dollar signs (even when the names used in the source program do not include dollar signs). The current behaviour matches names that are exactly the same (which is correct) as well as names that start with the same substring, followed by a dollar sign, followed by anything. That behaviour is somewhat weird, but would explain the different test cases, since the space in a b is represented using an initial dollar sign followed by some representation of space.

I looked into it, and it seems the current behaviour was changed in this commit: scala/scala@6b6d214#diff-086eb87150c79730cf3cfe9cb8b12219L70, where the previous behaviour was to match names that are exactly the same as well as names that start with the same substring and with a dollar sign at the end. Reverting the behaviour to that in the commit solves most cases discussed here (as well as the case "case class plus(a_+ : Int, a_ : String)"): Only Arya Irani's case with "case class right5(b: Int, a b: Int, a: Int, a : Int, a b c: Int)" does not print correctly, and accessing its fields works correctly. Cases where dollar signs are used directly at the end of names, such as "case class dollarsign(a$: Int, a: String)" and "case class dollarsign2(a$: Int, a: String)", still fail.

I did not figure out what the purpose of the check is. The original behaviour seems to be to check against an "original name": scala/scala@cb7711d#diff-086eb87150c79730cf3cfe9cb8b12219L58. In either case, I think a refactoring of the handling of Names from working on the string values to encapsulating the string value and instead provide more descriptive methods would be useful in making it much easier to verify and maintain correctness in relation to the handling of names, though I can imagine that would be quite a lot of work. In line with this, a better fix than presented below would implement a descriptive method (describing what the matching means) on Name (or similar) that handles the matching and call that instead of directly matching on parts of the string value of the name.

The potential fix (tested using "ant test-opt"):

def matchesName(param: Symbol) =
  param.name == name ||
  (param.name.startsWith(name) && param.name.endsWith(nme.NAME_JOIN_STRING))

@scabug
Copy link
Author

scabug commented Mar 5, 2015

@retronym said:
Your analysis is spot on, Jens. I'd actually started a patch for this last week: https://github.com/scala/scala/compare/2.11.x...retronym:ticket/8831?expand=1

In the commit comment, I describe the rationale for startsWith; the case accessor method for a non-public case class parameter a is mangled to a name like a$1. This itself is problematic, as it is leads to fragile binary compatibility. The 2.12.x branch uses a more structured name for these mangled a$accessor$1.

I'll make sure we fix this before 2.11.7. Until then, the workaround is to avoid using case class param names that start with someOtherParamName$.

@scabug
Copy link
Author

scabug commented Mar 19, 2015

@retronym said:
Another manifestation of this bug, reported on the mailing list:

we noticed some strange behaviour of productIterator in Scala 2.11.5:

case class Data(a: String, b: String, `a-some`: String)
val m = Data("1", "2", "3")
assert(m.productIterator.mkString(", ") == "1, 2, 3")

Result: "1, [3, 2]" did not equal "1, [2, 3]"

"a-some" can be any mangled symbol with an "a" prefix.

Is the order of productIterator on case classes guaranteed to be in parameter order? If so, this seems to be a bug. Can anyone reproduce it?

@scabug
Copy link
Author

scabug commented Apr 10, 2015

@melezov said:
Q: Should there be a sanity-check after the filter step to see if there were multiple parameters matched?

@scabug
Copy link
Author

scabug commented Mar 20, 2017

@adriaanm said:
I started working on this for 2.11, but did not complete in time for this to ship with 2.11.9: scala/scala@2.11.x...adriaanm:ticket/8831.

@hrhino
Copy link
Member

hrhino commented Oct 18, 2018

found again in #11212

@Guillaume-iov42
Copy link

Guillaume-iov42 commented Apr 8, 2019

Found again a flavor of it in 2.12.6:

case class MyClass(`a.b.c`: String, `a.b`: String)

object AdHocApp {

  def main(args: Array[String]): Unit = {
    val myInstance = MyClass(`a.b.c` = "abc", `a.b` = "ab")

    println(myInstance.`a.b`)
    println(myInstance.`a.b.c`)
  }
}

This should output "ab" then "abc", but outputs "abc" and "abc"...

@SethTisue
Copy link
Member

another slight variant at #11704

@fmeeuw
Copy link

fmeeuw commented Sep 11, 2019

Experienced a manifestation of this issue when using variable names that contains hyphens within backticks inside case classes.

case class WrongSameTypes(val `foo-bar`: String, `foo`: String)
case class WrongDifferentTypes(val `foo-bar`: Long, `foo`: String)
println(WrongSameTypes("foo-bar", "foo")) 
println(WrongDifferentTypes(3L, "foo"))

@SethTisue
Copy link
Member

With this ticket closed, we'll just have to devise some new ways to make Scala programmers question their own sanity!

👏 @martijnhoekstra

@daytonwilliams-okta
Copy link

daytonwilliams-okta commented Nov 5, 2020

Don't tempt me :trollface:

Glad to see it finally got closed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend compiler crash fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) help wanted minimized
Projects
None yet
Development

Successfully merging a pull request may close this issue.