An object with a main method can now extend its...

An object with a main method can now extend its companion class without
losing the ability to run. Also gave genjvm a wax and polish while in
the neigborhood. Closes #3487.

I had to get a little tricky with this one. No review per se, but if
anyone is frightened by the prospect of the forwarders not being marked
final in this case (because as of now, they aren't) then speak now or
ACC_FINAL hold your peace.

1 files changed, 131 insertions, 153 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
index 49d3251448..7be9475f23 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
@@ -9,15 +9,16 @@ package backend.jvm
 
 import java.nio.ByteBuffer
 
-import scala.collection.immutable.{Set, ListSet}
-import scala.collection.mutable.{Map, HashMap, HashSet}
+import scala.collection.{ mutable, immutable }
 import scala.tools.nsc.io.AbstractFile
 import scala.tools.nsc.symtab._
 import scala.tools.nsc.symtab.classfile.ClassfileConstants._
 
 import ch.epfl.lamp.fjbg._
-import java.io.{ByteArrayOutputStream, DataOutputStream, File, OutputStream}
-import reflect.generic.{PickleFormat, PickleBuffer}
+import JAccessFlags._
+import JObjectType.{ JAVA_LANG_STRING, JAVA_LANG_OBJECT }
+import java.io.{ DataOutputStream }
+import reflect.generic.{ PickleFormat, PickleBuffer }
 
 /** This class ...
  *
@@ -32,7 +33,9 @@ abstract class GenJVM extends SubComponent {
   import definitions.{
     NullClass, RuntimeNullClass, NothingClass, RuntimeNothingClass,
     AnyClass, ObjectClass, ThrowsClass, ThrowableClass, ClassfileAnnotationClass,
-    DeprecatedAttr,
+    SerializableClass, StringClass, ClassClass, FunctionClass,
+    DeprecatedAttr, SerializableAttr, SerialVersionUIDAttr, VolatileAttr,
+    TransientAttr, CloneableAttr, RemoteAttr,
     getPrimitiveCompanion
   }
 
@@ -77,8 +80,6 @@ abstract class GenJVM extends SubComponent {
    *
    */
   class BytecodeGenerator {
-    import JAccessFlags._
-
     def debugLevel = settings.debuginfo.indexOfChoice
 
     val MIN_SWITCH_DENSITY = 0.7
@@ -92,14 +93,13 @@ abstract class GenJVM extends SubComponent {
     val BoxesRunTime = "scala.runtime.BoxesRunTime"
 
     val StringBuilderType = new JObjectType(StringBuilderClass)
-    val toStringType      = new JMethodType(JObjectType.JAVA_LANG_STRING, JType.EMPTY_ARRAY)
-    val arrayCloneType    = new JMethodType(JObjectType.JAVA_LANG_OBJECT, JType.EMPTY_ARRAY)
+    val toStringType      = new JMethodType(JAVA_LANG_STRING, JType.EMPTY_ARRAY)
+    val arrayCloneType    = new JMethodType(JAVA_LANG_OBJECT, JType.EMPTY_ARRAY)
     val MethodTypeType    = new JObjectType("java.dyn.MethodType")
     val JavaLangClassType = new JObjectType("java.lang.Class")
     val MethodHandleType  = new JObjectType("java.dyn.MethodHandle")
 
     // Scala attributes
-    import definitions.{ SerializableAttr, SerialVersionUIDAttr, VolatileAttr, TransientAttr, CloneableAttr, RemoteAttr }
     val BeanInfoAttr     = definitions.getClass("scala.reflect.BeanInfo")
     val BeanInfoSkipAttr = definitions.getClass("scala.reflect.BeanInfoSkip")
     val BeanDisplayNameAttr = definitions.getClass("scala.reflect.BeanDisplayName")
@@ -130,7 +130,7 @@ abstract class GenJVM extends SubComponent {
     var jmethod: JMethod = _
 //    var jcode: JExtendedCode = _
 
-    var innerClasses: Set[Symbol] = ListSet.empty // referenced inner classes
+    var innerClasses: immutable.Set[Symbol] = immutable.ListSet.empty // referenced inner classes
 
     val fjbgContext = new FJBGContext(49, 0)
 
@@ -200,7 +200,7 @@ abstract class GenJVM extends SubComponent {
 
     def genClass(c: IClass) {
       clasz = c
-      innerClasses = ListSet.empty
+      innerClasses = immutable.ListSet.empty
 
       var parents = c.symbol.info.parents
       var ifaces  = JClass.NO_INTERFACES
@@ -211,11 +211,11 @@ abstract class GenJVM extends SubComponent {
                           (parents contains ParcelableInterface.tpe)
 
       if (parents.isEmpty)
-        parents = ObjectClass.tpe :: parents;
+        parents = List(ObjectClass.tpe)
 
       for (annot <- c.symbol.annotations) annot match {
         case AnnotationInfo(tp, _, _) if tp.typeSymbol == SerializableAttr =>
-          parents = parents ::: List(definitions.SerializableClass.tpe)
+          parents = parents ::: List(SerializableClass.tpe)
         case AnnotationInfo(tp, _, _) if tp.typeSymbol == CloneableAttr =>
           parents = parents ::: List(CloneableClass.tpe)
         case AnnotationInfo(tp, Literal(const) :: _, _) if tp.typeSymbol == SerialVersionUIDAttr =>
@@ -267,7 +267,7 @@ abstract class GenJVM extends SubComponent {
         // add static forwarders if there are no name conflicts; see bugs #363 and #1735
         if (lmoc != NoSymbol && !c.symbol.hasFlag(Flags.INTERFACE)) {
           if (isCandidateForForwarders(lmoc) && !settings.noForwarders.value) {
-            log("Adding forwarders to existing class '%s' found in module '%s'".format(c.symbol, lmoc))
+            log("Adding static forwarders from '%s' to implementations in '%s'".format(c.symbol, lmoc))
             addForwarders(jclass, lmoc.moduleClass)
           }
         }
@@ -320,9 +320,9 @@ abstract class GenJVM extends SubComponent {
 
       val constructor = beanInfoClass.addNewMethod(ACC_PUBLIC, "<init>", JType.VOID, javaTypes(Nil), javaNames(Nil))
       val jcode = constructor.getCode().asInstanceOf[JExtendedCode]
-      val strKind = new JObjectType(javaName(definitions.StringClass))
+      val strKind = new JObjectType(javaName(StringClass))
       val stringArrayKind = new JArrayType(strKind)
-      val conType = new JMethodType(JType.VOID, Array(javaType(definitions.ClassClass), stringArrayKind, stringArrayKind))
+      val conType = new JMethodType(JType.VOID, Array(javaType(ClassClass), stringArrayKind, stringArrayKind))
 
       def push(lst:Seq[String]) {
         var fi = 0
@@ -498,17 +498,15 @@ abstract class GenJVM extends SubComponent {
         val memberTpe = atPhase(currentRun.erasurePhase)(owner.thisType.memberInfo(sym))
         // println("addGenericSignature sym: " + sym.fullName + " : " + memberTpe + " sym.info: " + sym.info)
         // println("addGenericSignature: "+ (sym.ownerChain map (x => (x.name, x.isImplClass))))
-        erasure.javaSig(sym, memberTpe) match {
-          case Some(sig) =>
-            val index = jmember.getConstantPool().addUtf8(sig).toShort
-            if (settings.debug.value && settings.verbose.value)
-              atPhase(currentRun.erasurePhase) {
-                println("add generic sig "+sym+":"+sym.info+" ==> "+sig+" @ "+index)
-              }
-            val buf = ByteBuffer.allocate(2)
-            buf.putShort(index)
-            addAttribute(jmember, nme.SignatureATTR, buf)
-          case None =>
+        erasure.javaSig(sym, memberTpe) foreach { sig =>
+          val index = jmember.getConstantPool().addUtf8(sig).toShort
+          if (settings.debug.value && settings.verbose.value)
+            atPhase(currentRun.erasurePhase) {
+              println("add generic sig "+sym+":"+sym.info+" ==> "+sig+" @ "+index)
+            }
+          val buf = ByteBuffer.allocate(2)
+          buf.putShort(index)
+          addAttribute(jmember, nme.SignatureATTR, buf)
         }
       }
     }
@@ -724,7 +722,7 @@ abstract class GenJVM extends SubComponent {
       sym.owner.hasFlag(Flags.SYNTHETIC) &&
       sym.owner.tpe.parents.exists { t =>
         val TypeRef(_, sym, _) = t;
-        definitions.FunctionClass exists sym.==
+        FunctionClass contains sym
       }
     }
 
@@ -757,17 +755,15 @@ abstract class GenJVM extends SubComponent {
           }
 
           // add serialVUID code
-          serialVUID match {
-            case Some(value) =>
-              import Flags._, definitions._
-              val fieldName = "serialVersionUID"
-              val fieldSymbol = clasz.symbol.newValue(NoPosition, newTermName(fieldName))
-                                  .setFlag(STATIC | FINAL)
-                                  .setInfo(longType)
-              clasz addField new IField(fieldSymbol)
-              lastBlock emit CONSTANT(Constant(value))
-              lastBlock emit STORE_FIELD(fieldSymbol, true)
-            case None => ()
+          serialVUID foreach { value =>
+            import Flags._, definitions._
+            val fieldName = "serialVersionUID"
+            val fieldSymbol = clasz.symbol.newValue(NoPosition, newTermName(fieldName))
+                                .setFlag(STATIC | FINAL)
+                                .setInfo(longType)
+            clasz addField new IField(fieldSymbol)
+            lastBlock emit CONSTANT(Constant(value))
+            lastBlock emit STORE_FIELD(fieldSymbol, true)
           }
 
           // add CREATOR code
@@ -799,7 +795,6 @@ abstract class GenJVM extends SubComponent {
     }
 
     private def legacyStaticInitializer(cls: JClass, clinit: JExtendedCode) {
-      import JAccessFlags._
       if (isStaticModule(clasz.symbol)) {
         clinit emitNEW cls.getName()
         clinit.emitINVOKESPECIAL(cls.getName(),
@@ -807,20 +802,18 @@ abstract class GenJVM extends SubComponent {
                                  JMethodType.ARGLESS_VOID_FUNCTION)
       }
 
-      serialVUID match {
-        case Some(value) =>
-          val fieldName = "serialVersionUID"
-          jclass.addNewField(PublicStaticFinal, fieldName, JType.LONG)
-          clinit emitPUSH value
-          clinit.emitPUSH(value)
-          clinit.emitPUTSTATIC(jclass.getName(), fieldName, JType.LONG)
-        case None => ()
+      serialVUID foreach { value =>
+        val fieldName = "serialVersionUID"
+        jclass.addNewField(PublicStaticFinal, fieldName, JType.LONG)
+        clinit emitPUSH value
+        clinit.emitPUSH(value)
+        clinit.emitPUTSTATIC(jclass.getName(), fieldName, JType.LONG)
       }
 
       if (isParcelableClass) {
         val fieldName = "CREATOR"
         val creatorType = javaType(CreatorClass)
-        jclass.addNewField(ACC_STATIC | ACC_PUBLIC | ACC_FINAL,
+        jclass.addNewField(PublicStaticFinal,
                            fieldName,
                            creatorType)
         val moduleName = javaName(clasz.symbol)+"$"
@@ -847,28 +840,23 @@ abstract class GenJVM extends SubComponent {
       jcode emitPUSH "bootstrapInvokeDynamic"
       jcode.emitGETSTATIC("java.dyn.Linkage", "BOOTSTRAP_METHOD_TYPE", MethodTypeType)
       jcode.emitDUP
-      jcode.emitINVOKESTATIC("scala.Console", "println", new JMethodType(JType.VOID, Array(JObjectType.JAVA_LANG_OBJECT)))
+      jcode.emitINVOKESTATIC("scala.Console", "println", new JMethodType(JType.VOID, Array(JAVA_LANG_OBJECT)))
       jcode.emitINVOKESTATIC("java.dyn.MethodHandles", "findStatic",
-                              new JMethodType(MethodHandleType, Array(JavaLangClassType, JObjectType.JAVA_LANG_STRING, MethodTypeType)))
+                              new JMethodType(MethodHandleType, Array(JavaLangClassType, JAVA_LANG_STRING, MethodTypeType)))
       jcode.emitINVOKESTATIC("java.dyn.Linkage", "registerBootstrapMethod",
                               new JMethodType(JType.VOID, Array(JavaLangClassType, MethodHandleType)))
     }
 
     /** Add a forwarder for method m */
-    def addForwarder(jclass: JClass, module: Symbol, m: Symbol) {
-      import JAccessFlags._
-      val moduleName = javaName(module) // + "$"
-      val mirrorName = moduleName dropRight 1
-      val methodInfo = module.thisType.memberInfo(m)
-
+    def addForwarder(jclass: JClass, module: Symbol, m: Symbol, accessFlags: Int) {
+      val moduleName     = javaName(module)
+      val mirrorName     = moduleName dropRight 1 // dropping '$'
+      val methodInfo     = module.thisType.memberInfo(m)
       val paramJavaTypes = methodInfo.paramTypes map toTypeKind
-      val paramNames: Array[String] = new Array[String](paramJavaTypes.length)
-
-      for (i <- 0 until paramJavaTypes.length)
-        paramNames(i) = "x_" + i
+      val paramNames     = 0 until paramJavaTypes.length map ("x_" + _) toArray
 
       val mirrorMethod = jclass.addNewMethod(
-        PublicStaticFinal,
+        accessFlags,
         javaName(m),
         javaType(methodInfo.resultType),
         javaTypes(paramJavaTypes),
@@ -877,6 +865,7 @@ abstract class GenJVM extends SubComponent {
       mirrorCode.emitGETSTATIC(moduleName,
                                nme.MODULE_INSTANCE_FIELD.toString,
                                new JObjectType(moduleName))
+
       var i = 0
       var index = 0
       var argTypes = mirrorMethod.getArgumentTypes()
@@ -903,69 +892,70 @@ abstract class GenJVM extends SubComponent {
     /** Add forwarders for all methods defined in `module' that don't conflict
      *  with methods in the companion class of `module'. A conflict arises when
      *  a method with the same name is defined both in a class and its companion
-     *  object (method signature is not taken into account).  If 3rd argument
-     *  `cond` is supplied, only symbols for which `cond(sym)` is true are
-     *  given forwarders.
+     *  object: method signature is not taken into account.
      */
-    def addForwarders(jclass: JClass, module: Symbol) { addForwarders(jclass, module, _ => true) }
-    def addForwarders(jclass: JClass, module: Symbol, cond: (Symbol) => Boolean) {
-      def conflictsIn(cls: Symbol, name: Name) =
-        cls.info.members exists (_.name == name)
-
-      /** List of parents shared by both class and module, so we don't add
-       *  forwarders for methods defined there - bug #1804 */
-      lazy val commonParents = {
-        val cps = module.info.baseClasses
-        val mps = module.companionClass.info.baseClasses
-        cps.filter(mps contains)
-      }
-      /* The setter doesn't show up in members so we inspect the name
-       * ... and clearly it helps to know how the name is encoded, see ticket #3004.
-       * This logic is grossly inadequate! Name mangling needs a devotee.
+    def addForwarders(jclass: JClass, moduleClass: Symbol) {
+      assert(moduleClass.isModuleClass)
+      val className    = jclass.getName
+      val linkedClass  = moduleClass.companionClass
+      val linkedModule = linkedClass.companionSymbol
+
+      /** If we use the usual algorithm for forwarders, we run into a problem if
+       *  an object extends its companion class.  However, there is an out: since
+       *  all the forwarders are static, inheriting from the class is no problem
+       *  so long as the methods aren't final (the JVM will not allow redefinition
+       *  of a final static method.) Thus the following.
        */
-      def conflictsInCommonParent(name: Name) =
-        commonParents exists { cp =>
-          (name startsWith (cp.name + "$")) || (name containsName ("$" + cp.name + "$"))
-        }
+      val isIncestuous = moduleClass.tpe <:< linkedClass.tpe
+      val accessFlags  = if (isIncestuous) PublicStatic else PublicStaticFinal
 
-      /** Should method `m' get a forwarder in the mirror class? */
-      def shouldForward(m: Symbol): Boolean =
-        atPhase(currentRun.picklerPhase) (
-          m.owner != ObjectClass
-          && m.isMethod
-          && !m.hasFlag(Flags.CASE | Flags.PRIVATE | Flags.PROTECTED | Flags.DEFERRED | Flags.SPECIALIZED)
-          && !m.isConstructor
-          && !m.isStaticMember
-          && !(m.owner == AnyClass)
-          && !module.isSubClass(module.companionClass)
-          && !conflictsIn(ObjectClass, m.name)
-          && !conflictsInCommonParent(m.name)
-          && !conflictsIn(module.companionClass, m.name)
-        )
-
-      assert(module.isModuleClass)
-      if (settings.debug.value)
-        log("Dumping mirror class for object: " + module);
+      /** There was a bit of a gordian logic knot here regarding forwarders.
+       *  All we really have to do is exclude certain categories of symbols and
+       *  then all matching names.
+       */
+      def memberNames(sym: Symbol) = sym.info.members map (_.name.toString) toSet
+      lazy val membersInCommon     = atPhase(currentRun.picklerPhase)(
+        memberNames(linkedModule) intersect memberNames(linkedClass)
+      )
 
-      for (m <- module.info.nonPrivateMembers if shouldForward(m) && cond(m)) {
-        log("Adding static forwarder '%s' to '%s'".format(m, module))
-        addForwarder(jclass, module, m)
+      /** Should method `m' get a forwarder in the mirror class? */
+      def shouldForward(m: Symbol): Boolean = atPhase(currentRun.picklerPhase)(
+        m.owner != ObjectClass
+        && m.isMethod
+        && m.isPublic
+        && !m.hasFlag(Flags.CASE | Flags.DEFERRED | Flags.SPECIALIZED | Flags.LIFTED)
+        && !m.isConstructor
+        && !m.isStaticMember
+        && !membersInCommon(m.name.toString)
+      )
+
+      for (m <- moduleClass.info.nonPrivateMembers) {
+        if (shouldForward(m)) {
+          log("Adding static forwarder for '%s' from %s to '%s'".format(m, className, moduleClass))
+          addForwarder(jclass, moduleClass, m, accessFlags)
+        }
+        else if (settings.debug.value) {
+          log("No forwarder for '%s' from %s to '%s'".format(m, className, moduleClass))
+        }
       }
     }
 
     /** Dump a mirror class for a top-level module. A mirror class is a class
      *  containing only static methods that forward to the corresponding method
-     *  on the MODULE instance of the given Scala object.
+     *  on the MODULE instance of the given Scala object.  It will only be
+     *  generated if there is no companion class: if there is, an attempt will
+     *  instead be made to add the forwarder methods to the companion class.
      */
     def dumpMirrorClass(clasz: Symbol, sourceFile: String) {
-      import JAccessFlags._
       val moduleName = javaName(clasz) // + "$"
       val mirrorName = moduleName dropRight 1
       val mirrorClass = fjbgContext.JClass(ACC_SUPER | ACC_PUBLIC | ACC_FINAL,
                                            mirrorName,
-                                           "java.lang.Object",
+                                           JAVA_LANG_OBJECT.getName,
                                            JClass.NO_INTERFACES,
                                            sourceFile)
+
+      log("Dumping mirror class for '%s'".format(mirrorClass.getName))
       addForwarders(mirrorClass, clasz)
       val ssa = scalaSignatureAddingMarker(mirrorClass, clasz.companionSymbol)
       addAnnotations(mirrorClass, clasz.annotations ++ ssa)
@@ -1016,7 +1006,7 @@ abstract class GenJVM extends SubComponent {
         if (settings.debug.value)
           log("Making labels for: " + method)
 
-        HashMap(bs map (_ -> jcode.newLabel) : _*)
+        mutable.HashMap(bs map (_ -> jcode.newLabel) : _*)
       }
 
       isModuleInitialized = false
@@ -1024,7 +1014,7 @@ abstract class GenJVM extends SubComponent {
       linearization = linearizer.linearize(m)
       val labels = makeLabels(linearization)
       /** local variables whose scope appears in this block. */
-      var varsInBlock: collection.mutable.Set[Local] = new HashSet
+      var varsInBlock: mutable.Set[Local] = new mutable.HashSet
 
       var nextBlock: BasicBlock = linearization.head
 
@@ -1232,7 +1222,7 @@ abstract class GenJVM extends SubComponent {
             jcode.emitINVOKESTATIC(BoxesRunTime, "boxTo" + boxedType.decodedName, mtype)
 
           case UNBOX(kind) =>
-            val mtype = new JMethodType(javaType(kind), Array(JObjectType.JAVA_LANG_OBJECT))
+            val mtype = new JMethodType(javaType(kind), Array(JAVA_LANG_OBJECT))
             jcode.emitINVOKESTATIC(BoxesRunTime, "unboxTo" + kind.toType.typeSymbol.decodedName, mtype)
 
           case NEW(REFERENCE(cls)) =>
@@ -1626,7 +1616,7 @@ abstract class GenJVM extends SubComponent {
 
         case StringConcat(el) =>
           val jtype = el match {
-            case REFERENCE(_) | ARRAY(_) => JObjectType.JAVA_LANG_OBJECT
+            case REFERENCE(_) | ARRAY(_) => JAVA_LANG_OBJECT
             case _ => javaType(el)
           }
           jcode.emitINVOKEVIRTUAL(StringBuilderClass,
@@ -1706,8 +1696,8 @@ abstract class GenJVM extends SubComponent {
 
 
     /** For each basic block, the first PC address following it. */
-    val endPC: HashMap[BasicBlock, Int] = new HashMap()
-    val conds: HashMap[TestOp, Int] = new HashMap()
+    val endPC = new mutable.HashMap[BasicBlock, Int]
+    val conds = new mutable.HashMap[TestOp, Int]
 
     conds += (EQ -> JExtendedCode.COND_EQ)
     conds += (NE -> JExtendedCode.COND_NE)
@@ -1716,7 +1706,7 @@ abstract class GenJVM extends SubComponent {
     conds += (LE -> JExtendedCode.COND_LE)
     conds += (GE -> JExtendedCode.COND_GE)
 
-    val negate: HashMap[TestOp, TestOp] = new HashMap()
+    val negate = new mutable.HashMap[TestOp, TestOp]
 
     negate += (EQ -> NE)
     negate += (NE -> EQ)
@@ -1727,18 +1717,17 @@ abstract class GenJVM extends SubComponent {
 
     /** Map from type kinds to the Java reference types. It is used for
      *  loading class constants. @see Predef.classOf. */
-    val classLiteral: Map[TypeKind, JObjectType] = new HashMap()
-
-    classLiteral += (UNIT   -> new JObjectType("java.lang.Void"))
-    classLiteral += (BOOL   -> new JObjectType("java.lang.Boolean"))
-    classLiteral += (BYTE   -> new JObjectType("java.lang.Byte"))
-    classLiteral += (SHORT  -> new JObjectType("java.lang.Short"))
-    classLiteral += (CHAR   -> new JObjectType("java.lang.Character"))
-    classLiteral += (INT    -> new JObjectType("java.lang.Integer"))
-    classLiteral += (LONG   -> new JObjectType("java.lang.Long"))
-    classLiteral += (FLOAT  -> new JObjectType("java.lang.Float"))
-    classLiteral += (DOUBLE -> new JObjectType("java.lang.Double"))
-
+    val classLiteral = immutable.Map[TypeKind, JObjectType](
+      UNIT   -> new JObjectType("java.lang.Void"),
+      BOOL   -> new JObjectType("java.lang.Boolean"),
+      BYTE   -> new JObjectType("java.lang.Byte"),
+      SHORT  -> new JObjectType("java.lang.Short"),
+      CHAR   -> new JObjectType("java.lang.Character"),
+      INT    -> new JObjectType("java.lang.Integer"),
+      LONG   -> new JObjectType("java.lang.Long"),
+      FLOAT  -> new JObjectType("java.lang.Float"),
+      DOUBLE -> new JObjectType("java.lang.Double")
+    )
 
     ////////////////////// local vars ///////////////////////
 
@@ -1821,6 +1810,7 @@ abstract class GenJVM extends SubComponent {
       syms foreach (s => { res(i) = javaName(s); i += 1 })
       res
     }
+    private def mkFlags(args: Int*) = args.foldLeft(0)(_ | _)
 
     /**
      * Return the Java modifiers for the given symbol.
@@ -1839,32 +1829,20 @@ abstract class GenJVM extends SubComponent {
      *      and they would fail verification after lifted.
      */
     def javaFlags(sym: Symbol): Int = {
-      import JAccessFlags._
-
-      var jf: Int = 0
-      val f = sym.flags
-/*      jf = jf | (if (sym hasFlag Flags.PRIVATE) ACC_PRIVATE else
-                  if (sym hasFlag Flags.PROTECTED) ACC_PROTECTED else ACC_PUBLIC)
-*/
-      jf = jf | (if (sym hasFlag Flags.PRIVATE) ACC_PRIVATE else  ACC_PUBLIC)
-      jf = jf | (if ((sym hasFlag Flags.ABSTRACT) ||
-                     (sym hasFlag Flags.DEFERRED)) ACC_ABSTRACT else 0)
-      jf = jf | (if (sym hasFlag Flags.INTERFACE) ACC_INTERFACE else 0)
-      jf = jf | (if ((sym hasFlag Flags.FINAL)
-                       && !sym.enclClass.hasFlag(Flags.INTERFACE)
-                       && !sym.isClassConstructor) ACC_FINAL else 0)
-      jf = jf | (if (sym.isStaticMember) ACC_STATIC else 0)
-      jf = jf | (if (sym hasFlag Flags.BRIDGE) ACC_BRIDGE else 0)
-
-      if (sym.isClass && !sym.hasFlag(Flags.INTERFACE))
-        jf |= ACC_SUPER
-
       // constructors of module classes should be private
-      if (sym.isPrimaryConstructor && isTopLevelModule(sym.owner)) {
-        jf |= ACC_PRIVATE
-        jf &= ~ACC_PUBLIC
-      }
-      jf
+      // PP: why are they only being marked private at this stage and not earlier?
+      val isConsideredPrivate =
+        sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModule(sym.owner))
+
+      mkFlags(
+        if (isConsideredPrivate) ACC_PRIVATE else ACC_PUBLIC,
+        if (sym.isDeferred || sym.hasFlag(Flags.ABSTRACT)) ACC_ABSTRACT else 0,
+        if (sym.isInterface) ACC_INTERFACE else 0,
+        if (sym.isFinal && !sym.enclClass.isInterface && !sym.isClassConstructor) ACC_FINAL else 0,
+        if (sym.isStaticMember) ACC_STATIC else 0,
+        if (sym.isBridge) ACC_BRIDGE else 0,
+        if (sym.isClass && !sym.isInterface) ACC_SUPER else 0
+      )
     }
 
     /** Calls to methods in 'sym' need invokeinterface? */