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.

Sign up for GitHub

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

Jump to bottom

DefinitionsApi.{IntClass, ...} ought to be vals, not defs, to facilitate use as a stable identifier pattern #8483

Open
scabug opened this issue Apr 7, 2014 · 7 comments
Open

DefinitionsApi.{IntClass, ...} ought to be vals, not defs, to facilitate use as a stable identifier pattern #8483

scabug opened this issue Apr 7, 2014 · 7 comments
Labels
enhancement
Milestone
Backlog

Comments

@scabug
Copy link

scabug commented Apr 7, 2014

See #6815 and https://groups.google.com/d/topic/scala-internals/COqh09lG0P0/discussion for previous discussion/work. But is still broken, at least in terms of the usability of reflection.

The fix version of #6815 says 2.10.2-RC1, but in 2.10.4 there's no progress.

scala> u.emptyValDef match { case u.emptyValDef => }
<console>:9: error: stable identifier required, but u.emptyValDef found.
 Note that value emptyValDef is not stable because its type, => u.ValDef, is volatile.
              u.emptyValDef match { case u.emptyValDef => }
                                           ^

In 2.11.0-RC3 that example works, but more relevant ones don't.

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

scala> val u = scala.reflect.runtime.universe
u: scala.reflect.api.JavaUniverse = scala.reflect.runtime.JavaUniverse@4ce1748a

scala> import u._
import u._

scala> (null: Any) match { case TypeRef(_, definitions.IntClass, _) => }
<console>:12: error: stable identifier required, but u.definitions.IntClass found.
 Note that method IntClass is not stable because its type, => u.ClassSymbol, is volatile.
              (null: Any) match { case TypeRef(_, definitions.IntClass, _) => }
                                                              ^

It seems the behavior is a consequence of IntClass and friends appearing as defs via the reflection API, whereas in reality they are lazy vals. If I cast the universe to scala.reflect.internal.SymbolTable then I can pattern match on IntClass.

Given that pattern matching is essentially the only way to get at the structure of a typeref, if pattern matching on the core types doesn't work at all without reassigning to dummy vals it seems like a problem. It isn't so bad if one knows and can apply the secret incantation to expose the internals - maybe it is being taken as a given that everyone does that anyway.
@scabug
Copy link
Author

scabug commented Apr 7, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8483?orig=1
Reporter: @paulp
Affected Versions: 2.11.1
See #6815

@scabug
Copy link
Author

scabug commented Apr 7, 2014

@paulp said:
I withdraw the suggestion that there is a workaround. One simply trades in one set of bugs for another.

scala> val u = scala.reflect.runtime.universe
u: scala.reflect.api.JavaUniverse = scala.reflect.runtime.JavaUniverse@7e73b262

scala> import u._
import u._

scala> def f[T: WeakTypeTag] = weakTypeOf[T]
f: [T](implicit evidence$1: u.WeakTypeTag[T])u.Type

scala> f[List[_]]
res0: u.Type = scala.List[_]

// result of f[List[_]] if u is asInstanceOf[scala.reflect.internal.SymbolTable]
scala> f[List[_]]
scala.ScalaReflectionException: object $iw not found.
  at scala.reflect.internal.Mirrors$RootsBase.staticModule(Mirrors.scala:160)
  at scala.reflect.internal.Mirrors$RootsBase.staticModule(Mirrors.scala:22)
  at $typecreator1$1.apply(<console>:13)

@scabug
Copy link
Author

scabug commented Apr 7, 2014

@xeno-by said:
With the recent pull request that exposes c.internal, casting is actually discouraged, even for those who know their way around internals. This thing is a bug that I overlooked. Too bad that it didn't end up being on the aforementioned pull request, because now we'll have to wait for 2.12 :(

@scabug
Copy link
Author

scabug commented Apr 8, 2014

@retronym said:
Changing the defs to vals might end up being a two-way binary compatible change.

@scabug
Copy link
Author

scabug commented Apr 8, 2014

@retronym said:
Retitling this to make it sound less like a bug in the pattern matcher.

@scabug
Copy link
Author

scabug commented Apr 9, 2014

@paulp said:
Those with long memories may recall they first appeared as vals and I argued to change them into defs. Those reasons still stand: the problem is that there are no stable defs, so you are always left to choose between subsets of desirable qualities. See comments in #4605.

@scabug
Copy link
Author

scabug commented Apr 5, 2016

@szeiger said:
Scheduling for 2.12.0-M5. This would be the right time to get this into 2.12 before we try to freeze binary compatibility.

@scabug scabug added this to the Backlog milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
2 participants
@xeno-by @scabug