Merge pull request scala#4613 from dotty-staging/fix/denot-validity

Fix scala#1895: Use proper validity period for extension methods

compiler/src/dotty/tools/dotc/core/DenotTransformers.scala

@@ -24,9 +24,9 @@ object DenotTransformers {
     /** The last phase during which the transformed denotations are valid */
     def lastPhaseId(implicit ctx: Context) = ctx.nextDenotTransformerId(id + 1)
 
-    /** The validity period of the transformer in the given context */
+    /** The validity period of the transformed denotations in the given context */
     def validFor(implicit ctx: Context): Period =
-      Period(ctx.runId, id, lastPhaseId)
+      Period(ctx.runId, id + 1, lastPhaseId)
 
     /** The transformation method */
     def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -51,13 +51,9 @@ trait SymDenotations { this: Context =>
     else {
       val initial = denot.initial
       val firstPhaseId = initial.validFor.firstPhaseId.max(ctx.typerPhase.id)
-      if ((initial ne denot) || ctx.phaseId != firstPhaseId) {
-        ctx.withPhase(firstPhaseId).stillValidInOwner(initial) ||
-         // Workaround #1895: A symbol might not be entered into an owner
-         // until the second phase where it exists
-         (denot.validFor.containsPhaseId(firstPhaseId + 1)) &&
-           ctx.withPhase(firstPhaseId + 1).stillValidInOwner(initial)
-      } else
+      if ((initial ne denot) || ctx.phaseId != firstPhaseId)
+        ctx.withPhase(firstPhaseId).stillValidInOwner(initial)
+      else
         stillValidInOwner(denot)
     }
 

compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala

@@ -62,51 +62,38 @@ class ExtensionMethods extends MiniPhase with DenotTransformer with FullParamete
           val decls1 = cinfo.decls.cloneScope
           val moduleSym = moduleClassSym.symbol.asClass
 
-          var newSuperClass: Type = null
-
-          ctx.atPhase(thisPhase.next) { implicit ctx =>
-            // In Scala 2, extension methods are added before pickling so we should
-            // not generate them again.
-            if (!(valueClass is Scala2x)) ctx.atPhase(thisPhase) { implicit ctx =>
-              for (decl <- valueClass.classInfo.decls) {
-                if (isMethodWithExtension(decl)) {
-                  val meth = createExtensionMethod(decl, moduleClassSym.symbol)
-                  decls1.enter(meth)
-                  // Workaround #1895: force denotation of `meth` to be
-                  // at phase where `meth` is entered into the decls of a class
-                  meth.denot(ctx.withPhase(thisPhase.next))
-                }
-              }
-            }
-
-            val underlying = valueErasure(underlyingOfValueClass(valueClass))
-            val evt = ErasedValueType(valueClass.typeRef, underlying)
-            val u2evtSym = ctx.newSymbol(moduleSym, nme.U2EVT, Synthetic | Method,
-              MethodType(List(nme.x_0), List(underlying), evt))
-            val evt2uSym = ctx.newSymbol(moduleSym, nme.EVT2U, Synthetic | Method,
-              MethodType(List(nme.x_0), List(evt), underlying))
-
-            val defn = ctx.definitions
-
-            val underlyingCls = underlying.classSymbol
-            val underlyingClsName =
-              if (underlyingCls.isNumericValueClass || underlyingCls == defn.BooleanClass) underlyingCls.name
-              else nme.Object
-
-            val syp = ctx.requiredClass(s"dotty.runtime.vc.VC${underlyingClsName}Companion").asClass
-
-            newSuperClass = tpd.ref(syp).select(nme.CONSTRUCTOR).appliedToType(valueClass.typeRef).tpe.resultType
-
-            decls1.enter(u2evtSym)
-            decls1.enter(evt2uSym)
+          def enterInModuleClass(sym: Symbol): Unit = {
+            decls1.enter(sym)
+            // This is tricky: in this denotation transformer, we transform
+            // companion modules of value classes by adding methods to them.
+            // Running the transformer will create these methods, but they're
+            // only valid once it has finished running. This means we cannot use
+            // `ctx.withPhase(thisPhase.next)` here without potentially running
+            // into cycles. Instead, we manually set their validity after having
+            // created them to match the validity of the owner transformed
+            // denotation.
+            sym.validFor = thisPhase.validFor
           }
 
-          // Add the extension methods, the cast methods u2evt$ and evt2u$, and a VC*Companion superclass
-          moduleClassSym.copySymDenotation(info =
-            cinfo.derivedClassInfo(
-              // FIXME: use of VC*Companion superclasses is disabled until the conflicts with SyntheticMethods are solved.
-              //classParents = List(newSuperClass)
-              decls = decls1))
+          // Create extension methods, except if the class comes from Scala 2
+          // because it adds extension methods before pickling.
+          if (!(valueClass.is(Scala2x)))
+            for (decl <- valueClass.classInfo.decls)
+              if (isMethodWithExtension(decl))
+                enterInModuleClass(createExtensionMethod(decl, moduleClassSym.symbol))
+
+          // Create synthetic methods to cast values between the underlying type
+          // and the ErasedValueType. These methods are removed in ElimErasedValueType.
+          val underlying = valueErasure(underlyingOfValueClass(valueClass))
+          val evt = ErasedValueType(valueClass.typeRef, underlying)
+          val u2evtSym = ctx.newSymbol(moduleSym, nme.U2EVT, Synthetic | Method,
+            MethodType(List(nme.x_0), List(underlying), evt))
+          val evt2uSym = ctx.newSymbol(moduleSym, nme.EVT2U, Synthetic | Method,
+            MethodType(List(nme.x_0), List(evt), underlying))
+          enterInModuleClass(u2evtSym)
+          enterInModuleClass(evt2uSym)
+
+          moduleClassSym.copySymDenotation(info = cinfo.derivedClassInfo(decls = decls1))
         case _ =>
           moduleClassSym
       }