Skip to content

Commit b9527bc

Browse files
lrytzlrytz
committed
Context handling
The builders and their components have an implicit `ctx` field. The compiler phase, and the `GenBCode` object and its parents on the other hand cannot have a `ctx` field, that would be a memory leak. Instead, the functions outside the builder need to take a context as parameter.
1 parent 74037f3 commit b9527bc

8 files changed

+57
-48
lines changed

‎src/dotty/tools/dotc/backend/jvm/BCodeBodyBuilder.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
3434
/*
3535
* Functionality to build the body of ASM MethodNode, except for `synchronized` and `try` expressions.
3636
*/
37-
abstract class PlainBodyBuilder(cunit: CompilationUnit,
38-
implicit val ctx: Context) extends PlainSkelBuilder(cunit) {
37+
abstract class PlainBodyBuilder(cunit: CompilationUnit)
38+
extends PlainSkelBuilder(cunit) {
3939

4040
import icodes.TestOp
4141
import icodes.opcodes.InvokeStyle

‎src/dotty/tools/dotc/backend/jvm/BCodeHelpers.scala

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,17 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
295295

296296
} // end of method addInnerClassesASM()
297297

298+
/**
299+
* All components (e.g. BCPickles, BCInnerClassGen) of the builder classes
300+
* extend this trait to have access to the context.
301+
*
302+
* The context is provided by the three leaf classes (PlainClassBuilder,
303+
* JMirrorBuilder and JBeanInfoBuilder) as class parameter.
304+
*/
305+
trait HasContext {
306+
implicit protected val ctx: Context
307+
}
308+
298309
/*
299310
* Custom attribute (JVMS 4.7.1) "ScalaSig" used as marker only
300311
* i.e., the pickle is contained in a custom annotation, see:
@@ -307,7 +318,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
307318
* while the "Signature" attribute can be associated to classes, methods, and fields.)
308319
*
309320
*/
310-
trait BCPickles {
321+
trait BCPickles extends HasContext {
311322

312323
import scala.reflect.internal.pickling.{ PickleFormat, PickleBuffer }
313324

@@ -382,7 +393,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
382393

383394
} // end of trait BCPickles
384395

385-
trait BCInnerClassGen {
396+
trait BCInnerClassGen extends HasContext {
386397

387398
def debugLevel = settings.debuginfo.indexOfChoice
388399

@@ -404,14 +415,14 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
404415
*
405416
* must-single-thread
406417
*/
407-
final def internalName(sym: Symbol)(implicit ctx: Context): String = asmClassType(sym).getInternalName
418+
final def internalName(sym: Symbol): String = asmClassType(sym).getInternalName
408419

409420
/*
410421
* Tracks (if needed) the inner class given by `sym`.
411422
*
412423
* must-single-thread
413424
*/
414-
final def asmClassType(sym: Symbol)(implicit ctx: Context): BType = {
425+
final def asmClassType(sym: Symbol): BType = {
415426
assert(
416427
hasInternalName(sym),
417428
{
@@ -437,10 +448,8 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
437448
* Tracks (if needed) the inner class given by `t`.
438449
*
439450
* must-single-thread
440-
*
441-
* TODO(lry): check if `ctx` should be a paramter of the class instead.
442451
*/
443-
final def toTypeKind(t: Type)(implicit ctx: Context): BType = {
452+
final def toTypeKind(t: Type): BType = {
444453

445454
/* Interfaces have to be handled delicately to avoid introducing spurious errors,
446455
* but if we treat them all as AnyRef we lose too much information.
@@ -534,7 +543,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
534543
/*
535544
* must-single-thread
536545
*/
537-
def asmMethodType(msym: Symbol)(implicit ctx: Context): BType = {
546+
def asmMethodType(msym: Symbol): BType = {
538547
assert(msym is Flags.Method, s"not a method-symbol: $msym")
539548
val resT: BType =
540549
if (msym.isClassConstructor || msym.isConstructor) BType.VOID_TYPE
@@ -549,7 +558,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
549558
*
550559
* must-single-thread
551560
*/
552-
final def trackMemberClasses(csym: Symbol, lateClosuresBTs: List[BType])(implicit ctx: Context): List[BType] = {
561+
final def trackMemberClasses(csym: Symbol, lateClosuresBTs: List[BType]): List[BType] = {
553562
val lateInnerClasses = exitingErasure {
554563
for (sym <- List(csym, csym.linkedClassOfClass); memberc <- sym.info.decls.map(innerClassSymbolFor) if memberc.isClass)
555564
yield memberc
@@ -573,14 +582,14 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
573582
*
574583
* must-single-thread
575584
*/
576-
final def descriptor(t: Type)(implicit ctx: Context): String = (toTypeKind(t).getDescriptor)
585+
final def descriptor(t: Type): String = (toTypeKind(t).getDescriptor)
577586

578587
/*
579588
* Tracks (if needed) the inner class given by `sym`.
580589
*
581590
* must-single-thread
582591
*/
583-
final def descriptor(sym: Symbol)(implicit ctx: Context): String = (asmClassType(sym).getDescriptor)
592+
final def descriptor(sym: Symbol): String = (asmClassType(sym).getDescriptor)
584593

585594
} // end of trait BCInnerClassGen
586595

@@ -773,7 +782,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
773782

774783
} // end of trait BCAnnotGen
775784

776-
trait BCJGenSigGen {
785+
trait BCJGenSigGen extends HasContext {
777786

778787
// @M don't generate java generics sigs for (members of) implementation
779788
// classes, as they are monomorphic (TODO: ok?)
@@ -995,7 +1004,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
9951004

9961005
} // end of trait BCForwardersGen
9971006

998-
trait BCClassGen extends BCInnerClassGen {
1007+
trait BCClassGen extends BCInnerClassGen with HasContext {
9991008

10001009
// Used as threshold above which a tableswitch bytecode instruction is preferred over a lookupswitch.
10011010
// There's a space tradeoff between these multi-branch instructions (details in the JVM spec).
@@ -1080,15 +1089,18 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
10801089

10811090
} // end of class JBuilder
10821091

1083-
/* functionality for building plain and mirror classes */
1092+
/* functionality for building plain and mirror classes
1093+
* TODO(lrytz): it seems only `JMirrorBuilder` extends `JCommonBuilder`.
1094+
* So this class could be removed.
1095+
*/
10841096
abstract class JCommonBuilder
10851097
extends JBuilder
10861098
with BCAnnotGen
10871099
with BCForwardersGen
10881100
with BCPickles { }
10891101

10901102
/* builder of mirror classes */
1091-
class JMirrorBuilder extends JCommonBuilder {
1103+
class JMirrorBuilder(implicit protected val ctx: Context) extends JCommonBuilder {
10921104

10931105
private var cunit: CompilationUnit = _
10941106
def getCurrentCUnit(): CompilationUnit = cunit;
@@ -1143,7 +1155,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
11431155
} // end of class JMirrorBuilder
11441156

11451157
/* builder of bean info classes */
1146-
class JBeanInfoBuilder extends JBuilder {
1158+
class JBeanInfoBuilder(implicit protected val ctx: Context) extends JBuilder {
11471159

11481160
/*
11491161
* Generate a bean info class that describes the given class.

‎src/dotty/tools/dotc/backend/jvm/BCodeSkelBuilder.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import scala.annotation.switch
1212
import dotty.tools.asm
1313

1414
import ast.Trees._
15+
import core.Contexts.Context
1516
import core.Types.Type
1617
import core.Symbols.{Symbol, NoSymbol}
1718

‎src/dotty/tools/dotc/backend/jvm/BCodeSyncAndTry.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ abstract class BCodeSyncAndTry extends BCodeBodyBuilder {
2727
/*
2828
* Functionality to lower `synchronized` and `try` expressions.
2929
*/
30-
abstract class SyncAndTryBuilder(cunit: CompilationUnit,
31-
ctx: Context) extends PlainBodyBuilder(cunit, ctx) {
30+
abstract class SyncAndTryBuilder(cunit: CompilationUnit)
31+
extends PlainBodyBuilder(cunit) {
3232

3333
import ast.tpd._
3434

‎src/dotty/tools/dotc/backend/jvm/BCodeTypes.scala

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,6 @@ abstract class BCodeTypes extends BCodeIdiomatic {
383383
* On the other hand, this method does record the inner-class status of the argument, via `buildExemplar()`.
384384
*
385385
* must-single-thread
386-
*
387-
* TODO(lry) check if ctx should be a class parameter
388386
*/
389387
final def exemplar(csym0: Symbol)(implicit ctx: Context): Tracked = {
390388
assert(csym0 != NoSymbol, "NoSymbol can't be tracked")

‎src/dotty/tools/dotc/backend/jvm/GenBCode.scala

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import scala.annotation.switch
1111

1212
import dotty.tools.asm
1313

14-
import ast.Trees
14+
import ast.Trees._
1515
import core.Contexts.Context
16+
import core.Phases.Phase
1617
import core.Types.Type
1718
import core.Symbols.{Symbol, NoSymbol}
1819

@@ -47,22 +48,26 @@ import core.Symbols.{Symbol, NoSymbol}
4748
*/
4849
object GenBCode extends BCodeSyncAndTry {
4950

50-
final class PlainClassBuilder(cunit: CompilationUnit,
51-
ctx: Context) extends SyncAndTryBuilder(cunit, ctx)
51+
import ast.tpd._
5252

53-
class BCodePhase extends dotc.core.Phases.Phase {
53+
final class PlainClassBuilder(cunit: CompilationUnit)(implicit protected val ctx: Context)
54+
extends SyncAndTryBuilder(cunit)
55+
56+
class BCodePhase extends Phase {
5457

5558
def name = "jvm"
5659
override def description = "Generate bytecode from ASTs using the ASM library"
57-
// override def erasedTypes = true // TODO(lry) remove, probably not necessary in dotty
60+
// override def erasedTypes = true // TODO(lrytz) remove, probably not necessary in dotty
5861

5962
private var bytecodeWriter : BytecodeWriter = null
63+
// TODO(lrytz): pass builders around instead of storing them in fields. Builders
64+
// have a context, potential for memory leaks.
6065
private var mirrorCodeGen : JMirrorBuilder = null
6166
private var beanInfoCodeGen : JBeanInfoBuilder = null
6267

6368
/* ---------------- q1 ---------------- */
6469

65-
case class Item1(arrivalPos: Int, cd: ast.tpd.TypeDef, cunit: CompilationUnit) {
70+
case class Item1(arrivalPos: Int, cd: TypeDef, cunit: CompilationUnit) {
6671
def isPoison = { arrivalPos == Int.MaxValue }
6772
}
6873
private val poison1 = Item1(Int.MaxValue, null, null)
@@ -74,7 +79,7 @@ object GenBCode extends BCodeSyncAndTry {
7479
mirror: asm.tree.ClassNode,
7580
plain: asm.tree.ClassNode,
7681
bean: asm.tree.ClassNode,
77-
outFolder: scala.tools.nsc.io.AbstractFile) {
82+
outFolder: dotty.tools.io.AbstractFile) {
7883
def isPoison = { arrivalPos == Int.MaxValue }
7984
}
8085

@@ -99,7 +104,7 @@ object GenBCode extends BCodeSyncAndTry {
99104
mirror: SubItem3,
100105
plain: SubItem3,
101106
bean: SubItem3,
102-
outFolder: scala.tools.nsc.io.AbstractFile) {
107+
outFolder: dotty.tools.io.AbstractFile) {
103108

104109
def isPoison = { arrivalPos == Int.MaxValue }
105110
}
@@ -316,14 +321,14 @@ object GenBCode extends BCodeSyncAndTry {
316321

317322
/* Feed pipeline-1: place all ClassDefs on q1, recording their arrival position. */
318323
private def feedPipeline1(units: List[CompilationUnit]): Unit = {
319-
units forech apply
324+
units foreach addToQ1
320325
q1 add poison1
321326
}
322327

323328
/* Pipeline that writes classfile representations to disk. */
324329
private def drainQ3(): Unit = {
325330

326-
def sendToDisk(cfr: SubItem3, outFolder: scala.tools.nsc.io.AbstractFile): Unit = {
331+
def sendToDisk(cfr: SubItem3, outFolder: dotty.tools.io.AbstractFile): Unit = {
327332
if (cfr != null){
328333
val SubItem3(jclassName, jclassBytes) = cfr
329334
try {
@@ -360,26 +365,19 @@ object GenBCode extends BCodeSyncAndTry {
360365
assert(q1.isEmpty, s"Some ClassDefs remained in the first queue: $q1")
361366
assert(q2.isEmpty, s"Some classfiles remained in the second queue: $q2")
362367
assert(q3.isEmpty, s"Some classfiles weren't written to disk: $q3")
363-
364368
}
365369

366-
override def apply(cunit: CompilationUnit): Unit = {
367-
368-
def gen(tree: Trees.Tree): Unit = {
369-
370-
import Trees.{PackageDef, TypeDef}
371-
370+
def addToQ1(cunit: CompilationUnit): Unit = {
371+
def gen(tree: Tree): Unit = {
372372
tree match {
373-
case ast.untpd.EmptyTree => ()
374-
case ast.tpd.EmptyTree => ()
373+
case EmptyTree => ()
375374
case PackageDef(_, stats) => stats foreach gen
376375
case cd: TypeDef =>
377376
q1 add Item1(arrivalPos, cd, cunit)
378377
arrivalPos += 1
379378
}
380379
}
381-
382-
gen(cunit.body)
380+
gen(cunit.tpdTree)
383381
}
384382

385383
} // end of class BCodePhase

‎src/dotty/tools/dotc/core/Flags.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ object Flags {
2626
}
2727

2828
/** The intersection of this flag set and the given flag set
29-
* TODO(lry): check if resulting flag set has a non-empty kind?
29+
* TODO(lrytz): check if resulting flag set has a non-empty kind?
3030
*/
3131
def & (that: FlagSet) = FlagSet(bits & that.bits)
3232

@@ -115,7 +115,7 @@ object Flags {
115115
* conjunctively. I.e. for a flag conjunction `fc`,
116116
* `x is fc` tests whether `x` contains all flags in `fc`.
117117
*
118-
* TODO(lry) cannot be a value class because its erause is the same as `FlagSet`,
118+
* TODO(lrytz) cannot be a value class because its erause is the same as `FlagSet`,
119119
* the overloaded `is` would not work. Maybe rename `is` to `isAny` and `isAll`,
120120
* get rid of `FlagConjunction`? Code would also be more explicit.
121121
*/
@@ -256,7 +256,7 @@ object Flags {
256256
final val PackageClass = Package.toTypeFlags
257257

258258
/** A case class or its companion object
259-
* TODO(lry): Is CaseVal set for the companion of a case class? Or for a `case object`?
259+
* TODO(lrytz): Is CaseVal set for the companion of a case class? Or for a `case object`?
260260
* Or both? Is CaseClass set for the module class of a `case object`?
261261
*/
262262
final val Case = commonFlag(17, "case")
@@ -524,4 +524,4 @@ object Flags {
524524

525525
implicit def conjToFlagSet(conj: FlagConjunction): FlagSet =
526526
FlagSet(conj.bits)
527-
}
527+
}

‎src/dotty/tools/package.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package dotty
22

33
package object tools {
4-
// TODO(lry) there also exists the class `dotty.tools.dotc.FatalError`. The aliases here
4+
// TODO(lrytz) there also exists the class `dotty.tools.dotc.FatalError`. The aliases here
55
// seem to be used in the `dotty.tools.io` package. Needs clean up. If `io` needs a different
66
// exception, it should have a different name.
77
type FatalError = scala.reflect.internal.FatalError

0 commit comments

Comments
 (0)