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

scala.Symbol's CleanUp code is broken and never used #7974

Closed
scabug opened this issue Nov 17, 2013 · 3 comments
Closed

scala.Symbol's CleanUp code is broken and never used #7974

scabug opened this issue Nov 17, 2013 · 3 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Nov 17, 2013

The comment in CleanUp says:

This transformation should identify Scala symbol
invocations in the tree and replace them with
references to a static member. Also, whenever
a class has at least a single symbol invocation
somewhere in its methods, a new static member
should be created and initialized for that symbol.
[...]
The reasoning behind this transformation is the
following. Symbols get interned - they are stored
in a global map which is protected with a lock.
The reason for this is making equality checks
quicker. But calling Symbol.apply, although it
does return a unique symbol, accesses a locked
object, making symbol access slow. To solve this,
the unique symbol from the global symbol map
in Symbol is accessed only once during class
loading, and after that, the unique symbol is in
the static member. Hence, it is cheap to both
reach the unique symbol and do equality
checks on it.

It seems like this transformation was never executed because the pattern did never match, and if it had, it would have broken horribly due bugs in the implementation.

@scabug
Copy link
Author

@scabug scabug commented Nov 17, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7974?orig=1
Reporter: @soc
Affected Versions: 2.9.3, 2.10.2, 2.11.0-M6

@scabug
Copy link
Author

@scabug scabug commented Nov 17, 2013

@scabug scabug closed this Jan 9, 2014
@scabug
Copy link
Author

@scabug scabug commented Nov 6, 2014

@retronym said:
Followup: scala/scala#4095

@scabug scabug added this to the 2.11.0-M8 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants