Skip to content

Commit

Permalink
reflection no longer uses atPhase and friends
Browse files Browse the repository at this point in the history
Mentioned methods mutate the global `atPhaseStack` variable, which can
easily lead to imbalances and, ultimately, to the empty stack error.

Luckily for us, there's only one dummy phase, SomePhase, which is used
by runtime reflection, so there is absolutely zero need to invoke atPhase
in non-compiler reflexive universes.

The cleanest solution would be to override `atPhase` for runtime reflection,
but it's @inline final, so I didn't want to pay performance penalties for
something that's used three times in runtime reflection (during unpickling, in
reflection-specific completers and in `Symbol.typeParams/unsafeTypeParams`).

Therefore I added overrideable analogues of `atPhase` and `atPhaseNotLaterThan`
which are called from the aforementioned code shared between the compiler and
runtime reflection. I also had to duplicate the code of `Symbol.XXXtypeParams`,
again due to them being very performance-sensitive.
  • Loading branch information
xeno-by committed Feb 11, 2013
1 parent a9dca51 commit b2c2493
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 28 deletions.
5 changes: 5 additions & 0 deletions src/reflect/scala/reflect/internal/SymbolTable.scala
Expand Up @@ -209,6 +209,8 @@ abstract class SymbolTable extends macros.Universe
finally popPhase(saved)
}

def slowButSafeAtPhase[T](ph: Phase)(op: => T): T =
if (isCompilerUniverse) atPhase(ph)(op) else op

/** Since when it is to be "at" a phase is inherently ambiguous,
* a couple unambiguously named methods.
Expand All @@ -221,6 +223,9 @@ abstract class SymbolTable extends macros.Universe
@inline final def atPhaseNotLaterThan[T](target: Phase)(op: => T): T =
if (isAtPhaseAfter(target)) atPhase(target)(op) else op

def slowButSafeAtPhaseNotLaterThan[T](target: Phase)(op: => T): T =
if (isCompilerUniverse) atPhaseNotLaterThan(target)(op) else op

final def isValid(period: Period): Boolean =
period != 0 && runId(period) == currentRunId && {
val pid = phaseId(period)
Expand Down
4 changes: 2 additions & 2 deletions src/reflect/scala/reflect/internal/pickling/UnPickler.scala
Expand Up @@ -853,7 +853,7 @@ abstract class UnPickler {
private val p = phase
override def complete(sym: Symbol) : Unit = try {
val tp = at(i, () => readType(sym.isTerm)) // after NMT_TRANSITION, revert `() => readType(sym.isTerm)` to `readType`
atPhase(p) (sym setInfo tp)
slowButSafeAtPhase(p)(sym setInfo tp)
if (currentRunId != definedAtRunId)
sym.setInfo(adaptToNewRunMap(tp))
}
Expand All @@ -871,7 +871,7 @@ abstract class UnPickler {
super.complete(sym)
var alias = at(j, readSymbol)
if (alias.isOverloaded)
alias = atPhase(picklerPhase)((alias suchThat (alt => sym.tpe =:= sym.owner.thisType.memberType(alt))))
alias = slowButSafeAtPhase(picklerPhase)((alias suchThat (alt => sym.tpe =:= sym.owner.thisType.memberType(alt))))

sym.asInstanceOf[TermSymbol].setAlias(alias)
}
Expand Down
26 changes: 1 addition & 25 deletions src/reflect/scala/reflect/runtime/SymbolLoaders.scala
Expand Up @@ -15,37 +15,13 @@ private[reflect] trait SymbolLoaders { self: SymbolTable =>
* is found, a package is created instead.
*/
class TopClassCompleter(clazz: Symbol, module: Symbol) extends SymLoader with FlagAssigningCompleter {
// def makePackage() {
// println("wrong guess; making package "+clazz)
// val ptpe = newPackageType(module.moduleClass)
// for (sym <- List(clazz, module, module.moduleClass)) {
// sym setFlag Flags.PACKAGE
// sym setInfo ptpe
// }
// }

override def complete(sym: Symbol) = {
debugInfo("completing "+sym+"/"+clazz.fullName)
assert(sym == clazz || sym == module || sym == module.moduleClass)
// try {
atPhaseNotLaterThan(picklerPhase) {
slowButSafeAtPhaseNotLaterThan(picklerPhase) {
val loadingMirror = mirrorThatLoaded(sym)
val javaClass = loadingMirror.javaClass(clazz.javaClassName)
loadingMirror.unpickleClass(clazz, module, javaClass)
// } catch {
// case ex: ClassNotFoundException => makePackage()
// case ex: NoClassDefFoundError => makePackage()
// Note: We catch NoClassDefFoundError because there are situations
// where a package and a class have the same name except for capitalization.
// It seems in this case the class is loaded even if capitalization differs
// but then a NoClassDefFound error is issued with a ("wrong name: ...")
// reason. (I guess this is a concession to Windows).
// The present behavior is a bit too forgiving, in that it masks
// all class load errors, not just wrong name errors. We should try
// to be more discriminating. To get on the right track simply delete
// the clause above and load a collection class such as collection.Iterable.
// You'll see an error that class `parallel` has the wrong name.
// }
}
}
override def load(sym: Symbol) = complete(sym)
Expand Down
24 changes: 23 additions & 1 deletion src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala
Expand Up @@ -44,7 +44,29 @@ private[reflect] trait SynchronizedSymbols extends internal.Symbols { self: Symb
override def updateInfo(info: Type): Symbol = synchronized { super.updateInfo(info) }
override def rawInfo: Type = synchronized { super.rawInfo }

override def typeParams: List[Symbol] = synchronized { super.typeParams }
override def typeParams: List[Symbol] = synchronized {
if (isCompilerUniverse) super.typeParams
else {
if (isMonomorphicType) Nil
else {
// analogously to the "info" getter, here we allow for two completions:
// one: sourceCompleter to LazyType, two: LazyType to completed type
if (validTo == NoPeriod)
rawInfo load this
if (validTo == NoPeriod)
rawInfo load this

rawInfo.typeParams
}
}
}
override def unsafeTypeParams: List[Symbol] = synchronized {
if (isCompilerUniverse) super.unsafeTypeParams
else {
if (isMonomorphicType) Nil
else rawInfo.typeParams
}
}

override def reset(completer: Type): this.type = synchronized { super.reset(completer) }

Expand Down

1 comment on commit b2c2493

@scala-jenkins
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Job pr-rangepos-per-commit failed for b2c2493 (results):


Took 23 s.
sad kitty
to rebuild, comment "PLS REBUILD/pr-rangepos-per-commit@b2c2493b22e2b6b8feb8124a2503fbb794c0bde7"on PR #2083

Please sign in to comment.