Skip to content

partest: Enable separate compilation #1289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 28, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The supertype should be stable if tp.inst.exists

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Fixed in latest commit.

return tp.underlying.applyIfParameterized(args)
case tp: TypeProxy => tp.superType.applyIfParameterized(args)
case _ => defn.AnyType
}
Expand Down
18 changes: 14 additions & 4 deletions src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down Expand Up @@ -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 _ =>
}

*/
}
}

Expand Down
13 changes: 12 additions & 1 deletion src/dotty/tools/dotc/transform/ExpandPrivate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/transform/ExplicitOuter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/strawman/collections/CollectionStrawMan4.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
82 changes: 56 additions & 26 deletions test/dotty/partest/DPConsoleRunner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion test/test/CompilerTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/pos/annot.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ???

Expand Down
3 changes: 2 additions & 1 deletion tests/run/colltest4/CollectionStrawMan4_1.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
package colltest4
package strawman.collections

import Predef.{augmentString => _, wrapString => _, _}
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions tests/run/colltest4/CollectionTests_2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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("")(_ + _)
Expand Down
1 change: 1 addition & 0 deletions tests/run/colltest5/CollectionStrawMan5_1.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
package colltest5
package strawman.collections

import Predef.{augmentString => _, wrapString => _, _}
Expand Down
2 changes: 1 addition & 1 deletion tests/run/colltest5/CollectionTests_2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]) = {
Expand Down