No per-run caches in GenASM?

[scabug]

The compiler shouldn't have any global maps, and this one can trigger crashes in the presentation compiler. The smug message in the assertion is not very helpful for tracking down this issue, but I'd start by fixing the map.

I see this in GenASM.scala:155

  // Don't put this in per run caches. Contains entries for classes as well as members.
  val javaNameCache = new mutable.WeakHashMap[Symbol, Name]() ++= List(
    NothingClass        -> binarynme.RuntimeNothing,
    RuntimeNothingClass -> binarynme.RuntimeNothing,
    NullClass           -> binarynme.RuntimeNull,
    RuntimeNullClass    -> binarynme.RuntimeNull
  )

and crash (probably caused by two symbols for the same class, coming from different compiler runs):

ava.lang.AssertionError: assertion failed: how can getCommonSuperclass() do its job if different class symbols get the same bytecode-level internal name: scala/reflect/internal/Trees/Tree
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaName(GenASM.scala:547)
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaType(GenASM.scala:581)
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaType(GenASM.scala:587)
	at scala.tools.eclipse.JVMUtils$class.javaType(JVMUtils.scala:22)
	at scala.tools.eclipse.ScalaPresentationCompiler.javaType(ScalaPresentationCompiler.scala:36)
	at scala.tools.eclipse.javaelements.ScalaJavaMapper$class.mapParamTypeSignature(ScalaJavaMapper.scala:242)
	at scala.tools.eclipse.ScalaPresentationCompiler.mapParamTypeSignature(ScalaPresentationCompiler.scala:36)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$$anonfun$13.apply(ScalaStructureBuilder.scala:659)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$$anonfun$13.apply(ScalaStructureBuilder.scala:659)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.immutable.List.foreach(List.scala:318)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
	at scala.collection.AbstractTraversable.map(Traversable.scala:105)

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-7422?orig=1
Reporter: @dragos
Assignee: @magarciaEPFL
Affected Versions: 2.10.0

[scabug]

@magarciaEPFL said:

probably caused by two symbols for the same class, coming from different compiler runs

GenASM clears it maps at the end of GenASM.AsmPhase.run(), once all classes have been processed (that's also where bytecodeWriter.close() takes place).

Looks like GenASM.javaNameCache should also be cleared there. That should take care of clearing it properly. Regarding populating it properly, that can be achieved upon:

  /** Create a new phase */
  override def newPhase(p: Phase): Phase = new AsmPhase(p)

by just moving the following two maps from their current location `` into GenASM.AsmpPhase:

  val javaNameCache = new mutable.WeakHashMap[Symbol, Name]() ++= List(
    NothingClass        -> binarynme.RuntimeNothing,
    RuntimeNothingClass -> binarynme.RuntimeNothing,
    NullClass           -> binarynme.RuntimeNull,
    RuntimeNullClass    -> binarynme.RuntimeNull
  )

  // unlike javaNameCache, reverseJavaName contains entries only for class symbols and their internal names.
  val reverseJavaName = mutable.Map.empty[String, Symbol] ++= List(
    binarynme.RuntimeNothing.toString() -> RuntimeNothingClass, // RuntimeNothingClass is the bytecode-level return type of Scala methods with Nothing return-type.
    binarynme.RuntimeNull.toString()    -> RuntimeNullClass
  )

The above can't fail, even if holding for too long onto instances of GenASM, GenASM.AsmPhase, etc.

@dragos, can you confirm whether it works for you?

[scabug]

@retronym said:
AFAIK, all caches should now be created with perRunCaches. These will be automatically cleared at the end of each Run.

[scabug]

@magarciaEPFL said:
The alternative to perRunCaches that I mentioned should serve the IDE well (a phase starts and ends within a Run, thus the caches in question will get populated and cleared within a Run, too). The reason I mentioned this "alternative" is because I (faintly remember) entries disappearing (like, you know, magic) before the Run was over (actually, before the AsmPhase was over). Thus my suggestion to Iulian to stay clear from that magic's reach. Oh well, now it sounds as if I believe there's some magic to perRunCaches.

[scabug]

@magarciaEPFL said:
Proposed fix:
magarciaEPFL/scala@master...backendish6

It would be great if @dragos could test it in the Eclipse setting.

[scabug]

@magarciaEPFL said:
Regarding the proposed fix above, I've run a successful nightly on it.

Once @dragos OKs the fix above for the interplay with the presentation compiler, I can submit a PR.

[scabug]

@magarciaEPFL said:
scala/scala#2453

[scabug]

@dragos said:
I'll have a look.

[scabug]

@magarciaEPFL said:
scala/scala@a50f5db

[scabug]

@dragos said:

java.lang.AssertionError: assertion failed: Different class symbols have the same bytecode-level internal name:
     name: scala/tools/nsc/transform/patmat/MatchApproximation/MatchApproximator/Test
   oldsym: scala.tools.nsc.transform.patmat.MatchApproximation.MatchApproximator.Test
  tracked: scala.tools.nsc.transform.patmat.MatchApproximation.MatchApproximator.Test
              
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaName(GenASM.scala:553)
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaType(GenASM.scala:595)
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaType(GenASM.scala:601)
	at scala.tools.eclipse.JVMUtils$class.javaType(JVMUtils.scala:22)
	at scala.tools.eclipse.ScalaPresentationCompiler.javaType(ScalaPresentationCompiler.scala:36)
	at scala.tools.eclipse.javaelements.ScalaJavaMapper$class.mapParamTypeSignature(ScalaJavaMapper.scala:242)
	at scala.tools.eclipse.ScalaPresentationCompiler.mapParamTypeSignature(ScalaPresentationCompiler.scala:36)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$$anonfun$13.apply(ScalaStructureBuilder.scala:659)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$$anonfun$13.apply(ScalaStructureBuilder.scala:659)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
	at scala.collection.AbstractTraversable.map(Traversable.scala:103)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$class.addDef(ScalaStructureBuilder.scala:659)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$Builder.addDef(ScalaStructureBuilder.scala:857)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$class.addDef(ScalaStructureBuilder.scala:644)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$Builder.addDef(ScalaStructureBuilder.scala:857)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:929)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser.traverse(ScalaStructureBuilder.scala:889)
	at scala.tools.eclipse.javaelements.ScalaCompilationUnit$$anonfun$buildStructure$2$$anonfun$apply$5$$anonfun$apply$1.apply$mcV$sp(ScalaCompilationUnit.scala:89)
	at scala.tools.eclipse.javaelements.ScalaCompilationUnit$$anonfun$buildStructure$2$$anonfun$apply$5$$anonfun$apply$1.apply(ScalaCompilationUnit.scala:88)
	at scala.tools.eclipse.javaelements.ScalaCompilationUnit$$anonfun$buildStructure$2$$anonfun$apply$5$$anonfun$apply$1.apply(ScalaCompilationUnit.scala:88)
	at scala.tools.nsc.util.InterruptReq.execute(InterruptReq.scala:26)
	at scala.tools.nsc.interactive.Global.pollForWork(Global.scala:423)
	at scala.tools.nsc.interactive.PresentationCompilerThread.run(PresentationCompilerThread.scala:22)

(this was before the latest nightly, I can't yet test it because the signature change in GenASM makes it hard to have the same source in 2.10 and 2.11)

[scabug]

@magarciaEPFL said:

Some context on the check in question: duplicates are detected to avoid the situation where, given a bytecode-level classname, the super classes tracked for it don't match those of "the other" symbol that also maps to the same internal name. Knowing a class' superclasses is needed to compute Java 6 StackMapTables.

Coming back to debugging, what we don't know is the relative ordering of:

(1) clearing and initial population of javaNameCache, reverseJavaName, ie

    javaNameCache.clear()
    javaNameCache   ++= List( ...

    reverseJavaName.clear()
    reverseJavaName ++= List( ...

(2) clearing of the above performed on Run end by perRunCaches

(3) adding symbols to the maps above, this is expected to happen for a single Run bracketed between (1) and (2).

From what I can see, two scenarios under which the assertion fails are:

(a) symbols from two (consecutive, in general different) Runs get in the same bracket (1) -- (2)

(b) within a single Run, two different class symbols map to the same bytecode-level internal name.

(c) both of the above? (not so likely)

Depending on which scenario is at play, is the party to blame.