Merge pull request #1289 from dotty-staging/fix/partest-separate

partest: Enable separate compilation

Author: smarter

src/dotty/tools/dotc/core/Types.scala

@@ -2645,6 +2645,9 @@ object Types {
       if (ctx.period != validSuper) {
         cachedSuper = tycon match {
           case tp: TypeLambda => defn.AnyType
+          case tp: TypeVar if !tp.inst.exists =>
+            // supertype not stable, since underlying might change
+            return tp.underlying.applyIfParameterized(args)
           case tp: TypeProxy => tp.superType.applyIfParameterized(args)
           case _ => defn.AnyType
         }

src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -580,9 +580,6 @@ class ClassfileParser(
    *  parameters. For Java annotations we need to fake it by making up the constructor.
    *  Note that default getters have type Nothing. That's OK because we need
    *  them only to signal that the corresponding parameter is optional.
-   *  If the constructor takes as last parameter an array, it can also accept
-   *  a vararg argument. We solve this by creating two constructors, one with
-   *  an array, the other with a repeated parameter.
    */
   def addAnnotationConstructor(classInfo: Type, tparams: List[TypeSymbol] = Nil)(implicit ctx: Context): Unit = {
     def addDefaultGetter(attr: Symbol, n: Int) =
@@ -618,13 +615,26 @@ class ClassfileParser(
         }
 
         addConstr(paramTypes)
+
+        // The code below added an extra constructor to annotations where the
+        // last parameter of the constructor is an Array[X] for some X, the
+        // array was replaced by a vararg argument. Unfortunately this breaks
+        // inference when doing:
+        //   @Annot(Array())
+        // The constructor is overloaded so the expected type of `Array()` is
+        // WildcardType, and the type parameter of the Array apply method gets
+        // instantiated to `Nothing` instead of `X`.
+        // I'm leaving this commented out in case we improve inference to make this work.
+        // Note that if this is reenabled then JavaParser will also need to be modified
+        // to add the extra constructor (this was not implemented before).
+        /*
         if (paramTypes.nonEmpty)
           paramTypes.last match {
             case defn.ArrayOf(elemtp) =>
               addConstr(paramTypes.init :+ defn.RepeatedParamType.appliedTo(elemtp))
             case _ =>
         }
-
+        */
     }
   }
 

src/dotty/tools/dotc/transform/ExpandPrivate.scala

@@ -17,10 +17,14 @@ import Decorators._
 import ast.Trees._
 import TreeTransforms._
 import java.io.File.separatorChar
+import ValueClasses._
 
 /** Make private term members that are accessed from another class
  *  non-private by resetting the Private flag and expanding their name.
  *
+ *  Make private accessor in value class not-private. Ihis is necessary to unbox
+ *  the value class when accessing it from separate compilation units
+ *
  *  Also, make non-private any private parameter forwarders that forward to an inherited
  *  public or protected parameter accessor with the same name as the forwarder.
  *  This is necessary since private methods are not allowed to have the same name
@@ -52,13 +56,18 @@ class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { t
     }
   }
 
+  private def isVCPrivateParamAccessor(d: SymDenotation)(implicit ctx: Context) =
+    d.isTerm && d.is(PrivateParamAccessor) && isDerivedValueClass(d.owner)
+
   /** Make private terms accessed from different classes non-private.
    *  Note: this happens also for accesses between class and linked module class.
    *  If we change the scheme at one point to make static module class computations
    *  static members of the companion class, we should tighten the condition below.
    */
   private def ensurePrivateAccessible(d: SymDenotation)(implicit ctx: Context) =
-    if (d.is(PrivateTerm) && d.owner != ctx.owner.enclosingClass) {
+    if (isVCPrivateParamAccessor(d))
+      d.ensureNotPrivate.installAfter(thisTransform)
+    else if (d.is(PrivateTerm) && d.owner != ctx.owner.enclosingClass) {
       // Paths `p1` and `p2` are similar if they have a common suffix that follows
       // possibly different directory paths. That is, their common suffix extends
       // in both cases either to the start of the path or to a file separator character.
@@ -94,6 +103,8 @@ class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { t
       if sym.is(PrivateParamAccessor) && sel.symbol.is(ParamAccessor) && sym.name == sel.symbol.name =>
         sym.ensureNotPrivate.installAfter(thisTransform)
       case _ =>
+        if (isVCPrivateParamAccessor(sym))
+          sym.ensureNotPrivate.installAfter(thisTransform)
     }
     tree
   }

src/dotty/tools/dotc/transform/ExplicitOuter.scala

@@ -47,7 +47,7 @@ class ExplicitOuter extends MiniPhaseTransform with InfoTransformer { thisTransf
 
   /** Add outer accessors if a class always needs an outer pointer */
   override def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context) = tp match {
-    case tp @ ClassInfo(_, cls, _, decls, _) if needsOuterAlways(cls) =>
+    case tp @ ClassInfo(_, cls, _, decls, _) if needsOuterAlways(cls) && !sym.is(JavaDefined) =>
       val newDecls = decls.cloneScope
       newOuterAccessors(cls).foreach(newDecls.enter)
       tp.derivedClassInfo(decls = newDecls)

src/strawman/collections/CollectionStrawMan4.scala

@@ -218,7 +218,7 @@ object CollectionStrawMan4 {
     def fromIterable[B](coll: Iterable[B]): ListBuffer[B] = coll match {
       case pd @ View.Partitioned(partition: View.Partition[B]) =>
         partition.distribute(new ListBuffer[B]())
-        pd.forced.get.asInstanceOf[ListBuffer[B]]
+        new ListBuffer[B] ++= pd.forced.get
       case _ =>
         new ListBuffer[B] ++= coll
     }

test/dotty/partest/DPConsoleRunner.scala

@@ -134,6 +134,62 @@ class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runn
   // override to provide DottyCompiler
   override def newCompiler = new dotty.partest.DPDirectCompiler(this)
 
+  // Adapted from nest.Runner#javac because:
+  // - Our classpath handling is different and we need to pass extraClassPath
+  //   to java to get the scala-library which is required for some java tests
+  // - The compiler output should be redirected to cLogFile, like the output of
+  //   dotty itself
+  override def javac(files: List[File]): TestState = {
+    import fileManager._
+    import suiteRunner._
+    import FileManager.joinPaths
+    // compile using command-line javac compiler
+    val args = Seq(
+      javacCmdPath,
+      "-d",
+      outDir.getAbsolutePath,
+      "-classpath",
+      joinPaths(outDir :: extraClasspath ++ testClassPath)
+    ) ++ files.map(_.getAbsolutePath)
+
+    pushTranscript(args mkString " ")
+
+    val captured = StreamCapture(runCommand(args, cLogFile))
+    if (captured.result) genPass() else {
+      cLogFile appendAll captured.stderr
+      cLogFile appendAll captured.stdout
+      genFail("java compilation failed")
+    }
+  }
+
+  // FIXME: This is copy-pasted from nest.Runner where it is private
+  // Remove this once https://github.com/scala/scala-partest/pull/61 is merged
+  /** Runs command redirecting standard out and
+   *  error out to output file.
+   */
+  def runCommand(args: Seq[String], outFile: File): Boolean = {
+    import scala.sys.process.{ Process, ProcessLogger }
+    //(Process(args) #> outFile !) == 0 or (Process(args) ! pl) == 0
+    val pl = ProcessLogger(outFile)
+    val nonzero = 17     // rounding down from 17.3
+    def run: Int = {
+      val p = Process(args) run pl
+      try p.exitValue
+      catch {
+        case e: InterruptedException =>
+          NestUI verbose s"Interrupted waiting for command to finish (${args mkString " "})"
+          p.destroy
+          nonzero
+        case t: Throwable =>
+          NestUI verbose s"Exception waiting for command to finish: $t (${args mkString " "})"
+          p.destroy
+          throw t
+      }
+      finally pl.close()
+    }
+    (pl buffer run) == 0
+  }
+
   // override to provide default dotty flags from file in directory
   override def flagsForCompilation(sources: List[File]): List[String] = {
     val specificFlags = super.flagsForCompilation(sources)
@@ -245,32 +301,6 @@ class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runn
     } getOrElse true
   }
 
-  // override because Dotty currently doesn't handle separate compilation well,
-  // so we ignore groups (tests suffixed with _1 and _2)
-  override def groupedFiles(sources: List[File]): List[List[File]] = {
-    val grouped = sources groupBy (_.group)
-    val flatGroup = List(grouped.keys.toList.sorted.map({ k => grouped(k) sortBy (_.getName) }).flatten)
-    try { // try/catch because of bug in partest that throws exception
-      if (flatGroup != super.groupedFiles(sources))
-        throw new java.lang.UnsupportedOperationException()
-    } catch {
-      case e: java.lang.UnsupportedOperationException =>
-        val genlogFWriter = new FileWriter(DPConfig.genLog.jfile, true)
-        val genlogWriter = new PrintWriter(genlogFWriter, true)
-        genlogWriter.println("Warning: Overriding compilation groups for tests: " + sources)
-        genlogWriter.close
-        genlogFWriter.close
-    }
-    flatGroup
-  }
-
-  // override to avoid separate compilation of scala and java sources
-  override def mixedCompileGroup(allFiles: List[File]): List[CompileRound] = List(OnlyDotty(allFiles))
-  case class OnlyDotty(fs: List[File]) extends CompileRound {
-    def description = s"dotc $fsString"
-    lazy val result = { pushTranscript(description) ; attemptCompile(fs) }
-  }
-
   // override to add dotty and scala jars to classpath
   override def extraClasspath = suiteRunner.fileManager.asInstanceOf[DottyFileManager].extraJarList ::: super.extraClasspath
 

test/test/CompilerTest.scala

@@ -410,7 +410,8 @@ abstract class CompilerTest {
       nr: Int = 0, oldOutput: String = defaultOutputDir): Unit = {
 
     val partestOutput = dest.jfile.getParentFile + JFile.separator + dest.stripExtension + "-" + kind + ".obj"
-    val flags = oldFlags.map(f => if (f == oldOutput) partestOutput else f)
+    val flags = oldFlags.map(f => if (f == oldOutput) partestOutput else f) ++
+      List(s"-classpath $partestOutput") // Required for separate compilation tests
 
     getExisting(dest).isDifferent(source, flags, nerr) match {
       case NotExists => copyFiles(source, dest, partestOutput, flags, nerr, kind)

tests/pos/java-interop/t1263/Test.java renamed to tests/pending/pos/java-interop/t1263/Test.java

@@ -9,7 +9,7 @@ class Test {
 
   @SuppressWarnings(Array("hi", "foo")) def foo2() = ??? //can be deferred as there is a non-generic method
 
-  @SuppressWarnings("hi") def foo3() = ??? // can be written in java and is serialized this way in bytecode. doesn't typecheck
+  @SuppressWarnings(Array("hi")) def foo3() = ??? // can be written in java and is serialized this way in bytecode. doesn't typecheck
 
   @Transient(false) def bar = ???
 

tests/pos/java-interop/t1263/test.scala renamed to tests/pending/pos/java-interop/t1263/test.scala

@@ -1,3 +1,4 @@
+package colltest4
 package strawman.collections
 
 import Predef.{augmentString => _, wrapString => _, _}
@@ -216,7 +217,7 @@ object CollectionStrawMan4 {
     def fromIterable[B](coll: Iterable[B]): ListBuffer[B] = coll match {
       case pd @ View.Partitioned(partition: View.Partition[B]) =>
         partition.distribute(new ListBuffer[B]())
-        pd.forced.get.asInstanceOf[ListBuffer[B]]
+        new ListBuffer[B] ++= pd.forced.get
       case _ =>
         new ListBuffer[B] ++= coll
     }

tests/pos/java-interop/t1745/J.java renamed to tests/pending/pos/java-interop/t1745/J.java

@@ -2,8 +2,8 @@ import Predef.{augmentString => _, wrapString => _, _}
 import scala.reflect.ClassTag
 
 object Test {
-  import strawman.collections._
-  import CollectionStrawMan5._
+  import colltest4.strawman.collections._
+  import CollectionStrawMan4._
 
   def seqOps(xs: Seq[Int]) = {
     val x1 = xs.foldLeft("")(_ + _)

tests/pos/java-interop/t1745/S.scala renamed to tests/pending/pos/java-interop/t1745/S.scala

@@ -1,3 +1,4 @@
+package colltest5
 package strawman.collections
 
 import Predef.{augmentString => _, wrapString => _, _}

tests/pos/annot.scala

@@ -2,7 +2,7 @@ import Predef.{augmentString => _, wrapString => _, _}
 import scala.reflect.ClassTag
 
 object Test {
-  import strawman.collections._
+  import colltest5.strawman.collections._
   import CollectionStrawMan5._
 
   def seqOps(xs: Seq[Int]) = {