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

Naming of enumeration fields relies on unspecified behaviour of the JVM #5462

Open
scabug opened this issue Feb 14, 2012 · 3 comments
Open

Comments

@scabug
Copy link

scabug commented Feb 14, 2012

On Oracle JDK:

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

scala> object State extends Enumeration {
     |   val IDLE, STARTED = Value
     |   var x = IDLE
     | }
defined module State

scala> State.x
res0: State.Value = IDLE

scala> State.IDLE
res1: State.Value = IDLE 

On OpenJDK, it prints "x" instead of "IDLE" because the name of the var is accidentally picked up instead of the intended field name. This is implementation-dependent: "The elements in the array returned are not sorted and are not in any particular order" ([http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/Class.html#getMethods()]).

There's probably not much we can do about this unless we can use Scala reflection instead of Java reflection to get the fields in the correct order.

@scabug
Copy link
Author

scabug commented Feb 14, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5462?orig=1
Reporter: @szeiger
Affected Versions: 2.9.1
See #5211

@som-snytt
Copy link

Result also changes from run to run, although on this jvm 2.11 usually produces one order for the accessors and 2.12 produces the reverse order.

$ scala
Welcome to Scala 2.12.4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_144).
Type in expressions for evaluation. Or try :help.

scala> :pa
// Entering paste mode (ctrl-D to finish)

object Xab extends Enumeration {
  type Xab = Value
  val Aab, Bab = Value
  val Cab = Aab
}

// Exiting paste mode, now interpreting.

defined object Xab

scala> Xab.values
res0: Xab.ValueSet = Xab.ValueSet(Aab, Bab)

scala> :quit
$ scala
Welcome to Scala 2.12.4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_144).
Type in expressions for evaluation. Or try :help.

scala> :pa
// Entering paste mode (ctrl-D to finish)

object Xab extends Enumeration {
  type Xab = Value
  val Aab, Bab = Value
  val Cab = Aab
}

// Exiting paste mode, now interpreting.

defined object Xab

scala> Xab.values
res0: Xab.ValueSet = Xab.ValueSet(Cab, Bab)

@sjrd
Copy link
Member

sjrd commented Dec 20, 2017

This could be completely fixed by a macro that looks at the left-hand-side to which it is assigned, similarly to settingKey in sbt.

This would also remove all uses of reflection in Enumeration, which would make it fit for Scala.js out of the box. Currently we have to special-case it in our compiler plugin (and we basically do the job that a macro should be doing).

hrhino added a commit to hrhino/scala that referenced this issue Dec 21, 2017
Java reflection's field order is undefined and potentially
variable between runs; defining a `val` that refers to one
of the enumeration's `Value`s can cause wrong names to get
put in the names map at runtime. We can do the computation
at compile time with a fast-tracked macro instead.

This fixes scala/bug#5462.

Also, it should help to simplify scala-js, which evidently
special-cases this class pretty hard to compensate for the
lack of Java reflection in Javascript.

Because `Enumeration` is used in the library and compiler,
the removal of the non-macro `Value` methods isn't able to
happen until 2.13.x is reSTARRed. Therefore, this includes
a few temporary changes:

- `Value`'s old implementation moves to `_Value`, and gets
  the `protected[scala]` designation. All users of `Value`
  in the compiler and library temporarily call this method
  instead, until the intrinsic is in STARR.
- A few tests that depend on `scala-xml` get suspended for
  now, until there's a version that uses the new intrinsic
  method. I've somewhat whimsically replaced the test code
  with a `println` reproducing the check file, in the hope
  that, when the test is re-enabled, the git history won't
  vanish into the aether.
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

3 participants