From 855bdcc5210402fbb87ce10fe7da3f532fa3049f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 1 Jun 2016 02:35:00 +0200 Subject: [PATCH 01/11] partest: Enable separate compilation and javac tests partest can separately compile files based on their suffix (_1, _2, ...), it turns out that this feature was never enabled in the dotty version of partest and no one noticed (it prints warnings in ./tests/partest-generated/gen.log which no one reads), tests with *.java files should be compiled both with javac and dotty, but compiling with javac was also disabled. Enabling this revealed some latent bugs that will be fixed in the next few commits. --- test/dotty/partest/DPConsoleRunner.scala | 26 ------------------------ test/test/CompilerTest.scala | 3 ++- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/test/dotty/partest/DPConsoleRunner.scala b/test/dotty/partest/DPConsoleRunner.scala index baa62579c28a..c939e6704ee7 100644 --- a/test/dotty/partest/DPConsoleRunner.scala +++ b/test/dotty/partest/DPConsoleRunner.scala @@ -245,32 +245,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 diff --git a/test/test/CompilerTest.scala b/test/test/CompilerTest.scala index 1d8fb9bf5789..56b9e1099e3f 100644 --- a/test/test/CompilerTest.scala +++ b/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) From 216511fb74af60369c6049e0380b39898cf55e0a Mon Sep 17 00:00:00 2001 From: VladimirNik Date: Thu, 2 Jun 2016 15:37:22 +0200 Subject: [PATCH 02/11] Make private accessor in value class not-private. This is necessary to unbox the value class when accessing it from separate compilation units --- src/dotty/tools/dotc/transform/ExpandPrivate.scala | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/ExpandPrivate.scala b/src/dotty/tools/dotc/transform/ExpandPrivate.scala index 2e0759b8981f..83cd395ff03e 100644 --- a/src/dotty/tools/dotc/transform/ExpandPrivate.scala +++ b/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 } From 50c80e83d301720795a40a2b3347dad45774e4e7 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 2 Jun 2016 16:10:42 +0200 Subject: [PATCH 03/11] Fix colltest5 test under separate compilation This test failed before because strawman.collections.CollectionStrawMan5 is defined in two places: - src/strawman/collections/CollectionStrawMan5.scala - tests/run/colltest5/CollectionStrawMan5_1.scala The first will be compiled by scalac (unless the tests are run through a bootstrapped dotty) and the second will be compiled by dotty, the value class encoding of scalac and dotty are not binary compatible. This would not be a problem if we always used the `CollectionStrawMan5` coming from the partest output directory and ignored the one in the dotty sources, but which one gets picked depends on the classpath and whether compilation is joined or separate, see #1301. For now, it's safer and simpler to just avoid having tests which define a class that is also defined in the sources of dotty. Also, fix a bug in colltest4 where it was importing CollectionStrawMan5 instead of CollectionStrawMan4 --- tests/run/colltest4/CollectionStrawMan4_1.scala | 1 + tests/run/colltest4/CollectionTests_2.scala | 4 ++-- tests/run/colltest5/CollectionStrawMan5_1.scala | 1 + tests/run/colltest5/CollectionTests_2.scala | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/run/colltest4/CollectionStrawMan4_1.scala b/tests/run/colltest4/CollectionStrawMan4_1.scala index 874f67a2d86f..108d266ab40b 100644 --- a/tests/run/colltest4/CollectionStrawMan4_1.scala +++ b/tests/run/colltest4/CollectionStrawMan4_1.scala @@ -1,3 +1,4 @@ +package colltest4 package strawman.collections import Predef.{augmentString => _, wrapString => _, _} diff --git a/tests/run/colltest4/CollectionTests_2.scala b/tests/run/colltest4/CollectionTests_2.scala index 0961180ed4df..45da9d8a5e20 100644 --- a/tests/run/colltest4/CollectionTests_2.scala +++ b/tests/run/colltest4/CollectionTests_2.scala @@ -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("")(_ + _) diff --git a/tests/run/colltest5/CollectionStrawMan5_1.scala b/tests/run/colltest5/CollectionStrawMan5_1.scala index 1a89d9659c2b..bb6ed4426066 100644 --- a/tests/run/colltest5/CollectionStrawMan5_1.scala +++ b/tests/run/colltest5/CollectionStrawMan5_1.scala @@ -1,3 +1,4 @@ +package colltest5 package strawman.collections import Predef.{augmentString => _, wrapString => _, _} diff --git a/tests/run/colltest5/CollectionTests_2.scala b/tests/run/colltest5/CollectionTests_2.scala index 0961180ed4df..1a0bf37acb00 100644 --- a/tests/run/colltest5/CollectionTests_2.scala +++ b/tests/run/colltest5/CollectionTests_2.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]) = { From 1fa136f3be21dce9eae2d09d7cee959ede48e177 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 2 Jun 2016 18:49:05 +0200 Subject: [PATCH 04/11] Remove overloaded constructor for annotations This lead to inference failures when separately compiling t1751 and t294, this did not happen under joint compilation because JavaParser does not create the overloaded constructor --- .../dotc/core/classfile/ClassfileParser.scala | 18 ++++++++++++++---- tests/pos/annot.scala | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index 81337665507e..a6d381693783 100644 --- a/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/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 _ => } - + */ } } diff --git a/tests/pos/annot.scala b/tests/pos/annot.scala index e6e4f8051ba9..dec6af94564c 100644 --- a/tests/pos/annot.scala +++ b/tests/pos/annot.scala @@ -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 = ??? From 8c35854f9167a7cedd85e4601cadb9f422a83a5e Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 2 Jun 2016 23:58:42 +0200 Subject: [PATCH 05/11] partest: put more stuff on javac classpath Some java tests require the scala-library to be present on the classpath, this fixes tests/pos/java-interop/{t1186, t1235, t1254, t1642}. Also correctly redirect the output of javac so that it will be displayed by partest --verbose --- test/dotty/partest/DPConsoleRunner.scala | 56 ++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/test/dotty/partest/DPConsoleRunner.scala b/test/dotty/partest/DPConsoleRunner.scala index c939e6704ee7..d445722c9588 100644 --- a/test/dotty/partest/DPConsoleRunner.scala +++ b/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) From 301a956b615698af4b2479999bcf6ea46aaff11b Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 3 Jun 2016 00:04:06 +0200 Subject: [PATCH 06/11] pos/java-interop/volatile: Fix compilation with javac javac wants the public class name to match the filename. --- tests/pos/java-interop/volatile/{volatile.java => Foo.java} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/pos/java-interop/volatile/{volatile.java => Foo.java} (100%) diff --git a/tests/pos/java-interop/volatile/volatile.java b/tests/pos/java-interop/volatile/Foo.java similarity index 100% rename from tests/pos/java-interop/volatile/volatile.java rename to tests/pos/java-interop/volatile/Foo.java From a452fa2b85da4fc44af444bfe3f5d32afe6f5503 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 3 Jun 2016 00:18:50 +0200 Subject: [PATCH 07/11] Move java tests relying on type parameters to pending Currently, the classfiles emitted by dotty do not contain the type parameters information that javac relies on. Fixing this is tracked by #1303. --- tests/{ => pending}/pos/java-interop/t1263/Test.java | 0 tests/{ => pending}/pos/java-interop/t1263/test.scala | 0 tests/{ => pending}/pos/java-interop/t1745/J.java | 0 tests/{ => pending}/pos/java-interop/t1745/S.scala | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename tests/{ => pending}/pos/java-interop/t1263/Test.java (100%) rename tests/{ => pending}/pos/java-interop/t1263/test.scala (100%) rename tests/{ => pending}/pos/java-interop/t1745/J.java (100%) rename tests/{ => pending}/pos/java-interop/t1745/S.scala (100%) diff --git a/tests/pos/java-interop/t1263/Test.java b/tests/pending/pos/java-interop/t1263/Test.java similarity index 100% rename from tests/pos/java-interop/t1263/Test.java rename to tests/pending/pos/java-interop/t1263/Test.java diff --git a/tests/pos/java-interop/t1263/test.scala b/tests/pending/pos/java-interop/t1263/test.scala similarity index 100% rename from tests/pos/java-interop/t1263/test.scala rename to tests/pending/pos/java-interop/t1263/test.scala diff --git a/tests/pos/java-interop/t1745/J.java b/tests/pending/pos/java-interop/t1745/J.java similarity index 100% rename from tests/pos/java-interop/t1745/J.java rename to tests/pending/pos/java-interop/t1745/J.java diff --git a/tests/pos/java-interop/t1745/S.scala b/tests/pending/pos/java-interop/t1745/S.scala similarity index 100% rename from tests/pos/java-interop/t1745/S.scala rename to tests/pending/pos/java-interop/t1745/S.scala From 40efc354548b05b2cc5cfbd1f1694e0f79e31ac5 Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Fri, 3 Jun 2016 13:28:36 +0200 Subject: [PATCH 08/11] Fix colltest4: ListBuffer[A]#fromIterable had an incorrect cast --- src/strawman/collections/CollectionStrawMan4.scala | 2 +- tests/run/colltest4/CollectionStrawMan4_1.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/strawman/collections/CollectionStrawMan4.scala b/src/strawman/collections/CollectionStrawMan4.scala index ec20849ff1f7..fb95ea59f98d 100644 --- a/src/strawman/collections/CollectionStrawMan4.scala +++ b/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 } diff --git a/tests/run/colltest4/CollectionStrawMan4_1.scala b/tests/run/colltest4/CollectionStrawMan4_1.scala index 108d266ab40b..8f0bdc841ee0 100644 --- a/tests/run/colltest4/CollectionStrawMan4_1.scala +++ b/tests/run/colltest4/CollectionStrawMan4_1.scala @@ -217,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 } From 4b0cc8aabf0dbbd74979faedee7371c41177b755 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 27 Jul 2016 19:29:53 +0200 Subject: [PATCH 09/11] Don't generate outer accessors for Java innner classes. Java's naming convention is different from Scala's. --- src/dotty/tools/dotc/transform/ExplicitOuter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/ExplicitOuter.scala b/src/dotty/tools/dotc/transform/ExplicitOuter.scala index 7ec0739c194f..6a52b128c141 100644 --- a/src/dotty/tools/dotc/transform/ExplicitOuter.scala +++ b/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) From 16a688fb24b2926aad47e2977444e0b9249db50b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 27 Jul 2016 22:10:15 +0200 Subject: [PATCH 10/11] Fix HkApply#superType if type constructor is a TypeVar In this case, supertype is not stable and should not be cached. --- src/dotty/tools/dotc/core/Types.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 37342810835a..3e8a8da21554 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/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 => + // supertype not stable, since underlying might change + return tp.underlying.applyIfParameterized(args) case tp: TypeProxy => tp.superType.applyIfParameterized(args) case _ => defn.AnyType } From 04e6d5e5ad39d046a977de1bfd4563287e5b0f41 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 28 Jul 2016 00:37:38 +0200 Subject: [PATCH 11/11] Refine HKApply#superType --- src/dotty/tools/dotc/core/Types.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 3e8a8da21554..46da207122f9 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -2645,7 +2645,7 @@ object Types { if (ctx.period != validSuper) { cachedSuper = tycon match { case tp: TypeLambda => defn.AnyType - case tp: TypeVar => + 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)