From 753e848f3d6ac453871450161292139902669695 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 18 Nov 2016 18:09:28 +1000 Subject: SI-8779 Enable inlining of code within a REPL session The REPL has a long running instance of Global which outputs classfiles by default to a VirtualDirectory. The inliner did not find any of these class files when compiling calls to methods defined in previous runs (ie, previous lines of input.) This commit: - Adds a hook to augment the classpath that the optimizer searches, and uses this in the REPL to add the output directory - Fixes the implementation of `findClassFile` in VirtualDirectory, which doesn't seem to have been used in anger before. I've factored out some common code into a new method on `AbstractFile`. - Fixes a similar problem getSubDir reported by Li Haoyi - Adds missing unit test coverage. This also fixes a bug in REPL autocompletion for types defined in packages >= 2 level deep (with the `:paste -raw` command). I've added a test for this case. --- src/compiler/scala/tools/nsc/Global.scala | 3 ++ .../tools/nsc/backend/jvm/BTypesFromSymbols.scala | 2 +- .../nsc/classpath/VirtualDirectoryClassPath.scala | 12 +++---- .../internal/util/AbstractFileClassLoader.scala | 28 ++++++++------- .../scala/tools/nsc/interpreter/ReplGlobal.scala | 13 +++++++ test/files/run/repl-inline.check | 6 ++++ test/files/run/repl-inline.scala | 21 +++++++++++ .../classpath/VirtualDirectoryClassPathTest.scala | 41 ++++++++++++++++++++++ .../tools/nsc/interpreter/CompletionTest.scala | 13 ++++++- 9 files changed, 118 insertions(+), 21 deletions(-) create mode 100644 test/files/run/repl-inline.check create mode 100644 test/files/run/repl-inline.scala create mode 100644 test/junit/scala/tools/nsc/classpath/VirtualDirectoryClassPathTest.scala diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index a7880c72d7..e58d2d3b43 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -95,6 +95,9 @@ class Global(var currentSettings: Settings, var reporter: Reporter) type ThisPlatform = JavaPlatform { val global: Global.this.type } lazy val platform: ThisPlatform = new GlobalPlatform + /* A hook for the REPL to add a classpath entry containing products of previous runs to inliner's bytecode repository*/ + // Fixes SI-8779 + def optimizerClassPath(base: ClassPath): ClassPath = base def classPath: ClassPath = platform.classPath diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala index edb75514e8..f7ee36c1ba 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala @@ -37,7 +37,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { val coreBTypes = new CoreBTypesProxy[this.type](this) import coreBTypes._ - val byteCodeRepository: ByteCodeRepository[this.type] = new ByteCodeRepository(global.classPath, this) + val byteCodeRepository: ByteCodeRepository[this.type] = new ByteCodeRepository(global.optimizerClassPath(global.classPath), this) val localOpt: LocalOpt[this.type] = new LocalOpt(this) diff --git a/src/compiler/scala/tools/nsc/classpath/VirtualDirectoryClassPath.scala b/src/compiler/scala/tools/nsc/classpath/VirtualDirectoryClassPath.scala index 8df0c3743d..6fefaf0da0 100644 --- a/src/compiler/scala/tools/nsc/classpath/VirtualDirectoryClassPath.scala +++ b/src/compiler/scala/tools/nsc/classpath/VirtualDirectoryClassPath.scala @@ -1,9 +1,11 @@ package scala.tools.nsc.classpath import scala.tools.nsc.util.ClassRepresentation -import scala.reflect.io.{Path, PlainFile, VirtualDirectory, AbstractFile} +import scala.reflect.io.{AbstractFile, Path, PlainFile, VirtualDirectory} import FileUtils._ import java.net.URL + +import scala.reflect.internal.util.AbstractFileClassLoader import scala.tools.nsc.util.ClassPath case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath with DirectoryLookup[ClassFileEntryImpl] with NoSourcePaths { @@ -11,7 +13,7 @@ case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath wi protected def emptyFiles: Array[AbstractFile] = Array.empty protected def getSubDir(packageDirName: String): Option[AbstractFile] = - Option(dir.lookupName(packageDirName, directory = true)) + Option(AbstractFileClassLoader.lookupPath(dir)(packageDirName.split('/'), directory = true)) protected def listChildren(dir: AbstractFile, filter: Option[AbstractFile => Boolean] = None): Array[F] = filter match { case Some(f) => dir.iterator.filter(f).toArray case _ => dir.toArray @@ -27,10 +29,8 @@ case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath wi override def findClass(className: String): Option[ClassRepresentation] = findClassFile(className) map ClassFileEntryImpl def findClassFile(className: String): Option[AbstractFile] = { - val relativePath = FileUtils.dirPath(className) - val classFile = new PlainFile(Path(s"$dir/$relativePath.class")) - if (classFile.exists) Some(classFile) - else None + val relativePath = FileUtils.dirPath(className) + ".class" + Option(AbstractFileClassLoader.lookupPath(dir)(relativePath split '/', directory = false)) } private[nsc] def classes(inPackage: String): Seq[ClassFileEntry] = files(inPackage) diff --git a/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala b/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala index b5030460b8..3cede1b3c5 100644 --- a/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala +++ b/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala @@ -12,6 +12,20 @@ import java.security.cert.Certificate import java.security.{ ProtectionDomain, CodeSource } import java.util.{ Collections => JCollections, Enumeration => JEnumeration } +object AbstractFileClassLoader { + // should be a method on AbstractFile, but adding in `internal.util._` for now as we're in a minor release + private[scala] final def lookupPath(base: AbstractFile)(pathParts: Seq[String], directory: Boolean): AbstractFile = { + var file: AbstractFile = base + for (dirPart <- pathParts.init) { + file = file.lookupName(dirPart, directory = true) + if (file == null) + return null + } + + file.lookupName(pathParts.last, directory = directory) + } +} + /** A class loader that loads files from a [[scala.reflect.io.AbstractFile]]. * * @author Lex Spoon @@ -25,19 +39,7 @@ class AbstractFileClassLoader(val root: AbstractFile, parent: ClassLoader) else s"${name.replace('.', '/')}.class" protected def findAbstractFile(name: String): AbstractFile = { - var file: AbstractFile = root - val pathParts = name split '/' - - for (dirPart <- pathParts.init) { - file = file.lookupName(dirPart, directory = true) - if (file == null) - return null - } - - file.lookupName(pathParts.last, directory = false) match { - case null => null - case file => file - } + AbstractFileClassLoader.lookupPath(root)(name split '/', directory = false) } protected def dirNameToPath(name: String): String = diff --git a/src/repl/scala/tools/nsc/interpreter/ReplGlobal.scala b/src/repl/scala/tools/nsc/interpreter/ReplGlobal.scala index cf055e0758..0bb9eb6a0b 100644 --- a/src/repl/scala/tools/nsc/interpreter/ReplGlobal.scala +++ b/src/repl/scala/tools/nsc/interpreter/ReplGlobal.scala @@ -6,6 +6,9 @@ package scala.tools.nsc package interpreter +import scala.tools.nsc.backend.JavaPlatform +import scala.tools.nsc.classpath.{AggregateClassPath, ClassPathFactory} +import scala.tools.nsc.util.ClassPath import typechecker.Analyzer /** A layer on top of Global so I can guarantee some extra @@ -31,4 +34,14 @@ trait ReplGlobal extends Global { new util.AbstractFileClassLoader(virtualDirectory, loader) {} } } + + override def optimizerClassPath(base: ClassPath): ClassPath = { + settings.outputDirs.getSingleOutput match { + case None => base + case Some(out) => + // Make bytecode of previous lines available to the inliner + val replOutClasspath = ClassPathFactory.newClassPath(settings.outputDirs.getSingleOutput.get, settings) + AggregateClassPath.createAggregate(platform.classPath, replOutClasspath) + } + } } diff --git a/test/files/run/repl-inline.check b/test/files/run/repl-inline.check new file mode 100644 index 0000000000..3b29f4d047 --- /dev/null +++ b/test/files/run/repl-inline.check @@ -0,0 +1,6 @@ +warning: there was one deprecation warning (since 2.11.0); re-run with -deprecation for details +callerOfCaller: String +g: String +h: String +g: String +h: String diff --git a/test/files/run/repl-inline.scala b/test/files/run/repl-inline.scala new file mode 100644 index 0000000000..5a5f205ad8 --- /dev/null +++ b/test/files/run/repl-inline.scala @@ -0,0 +1,21 @@ +import scala.tools.nsc._ + +object Test { + val testCode = """ +def callerOfCaller = Thread.currentThread.getStackTrace.drop(2).head.getMethodName +def g = callerOfCaller +def h = g +assert(h == "g", h) +@inline def g = callerOfCaller +def h = g +assert(h == "h", h) + """ + + def main(args: Array[String]) { + val settings = new Settings() + settings.processArgumentString("-opt:l:classpath") + settings.usejavacp.value = true + val repl = new interpreter.IMain(settings) + testCode.linesIterator.foreach(repl.interpret(_)) + } +} diff --git a/test/junit/scala/tools/nsc/classpath/VirtualDirectoryClassPathTest.scala b/test/junit/scala/tools/nsc/classpath/VirtualDirectoryClassPathTest.scala new file mode 100644 index 0000000000..234f575b79 --- /dev/null +++ b/test/junit/scala/tools/nsc/classpath/VirtualDirectoryClassPathTest.scala @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2014 Contributor. All rights reserved. + */ +package scala.tools.nsc.classpath + +import org.junit.Assert._ +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +import scala.reflect.io.VirtualDirectory + + +@RunWith(classOf[JUnit4]) +class VirtualDirectoryClassPathTest { + + @Test + def virtualDirectoryClassPath_findClassFile(): Unit = { + val base = new VirtualDirectory("base", None) + val p1 = base subdirectoryNamed "p1" + val p1_Test_class = p1.fileNamed("Test.class") + val p2 = base subdirectoryNamed "p2" + val p3 = p2 subdirectoryNamed "p3" + val p4 = p3 subdirectoryNamed "p4" + val p4_Test1_class = p4.fileNamed("Test.class") + val classPath = VirtualDirectoryClassPath(base) + + assertEquals(Some(p1_Test_class), classPath.findClassFile("p1/Test")) + + assertEquals(None, classPath.findClassFile("p1/DoesNotExist")) + assertEquals(None, classPath.findClassFile("DoesNotExist")) + assertEquals(None, classPath.findClassFile("p2")) + assertEquals(None, classPath.findClassFile("p2/DoesNotExist")) + assertEquals(None, classPath.findClassFile("p4/DoesNotExist")) + + assertEquals(List("p1", "p2"), classPath.packages("").toList.map(_.name).sorted) + assertEquals(List(), classPath.packages("p1").toList.map(_.name).sorted) + assertEquals(List("p2.p3"), classPath.packages("p2").toList.map(_.name).sorted) + assertEquals(List("p2.p3.p4"), classPath.packages("p2.p3").toList.map(_.name).sorted) + } +} diff --git a/test/junit/scala/tools/nsc/interpreter/CompletionTest.scala b/test/junit/scala/tools/nsc/interpreter/CompletionTest.scala index 78ebb7cf9c..1233e8b1cc 100644 --- a/test/junit/scala/tools/nsc/interpreter/CompletionTest.scala +++ b/test/junit/scala/tools/nsc/interpreter/CompletionTest.scala @@ -1,10 +1,11 @@ package scala.tools.nsc.interpreter -import java.io.{StringWriter, PrintWriter} +import java.io.{PrintWriter, StringWriter} import org.junit.Assert.assertEquals import org.junit.Test +import scala.reflect.internal.util.BatchSourceFile import scala.tools.nsc.Settings class CompletionTest { @@ -174,6 +175,16 @@ class CompletionTest { checkExact(completer, "case class D(a: Int, b: Int) { this.a")("a", "asInstanceOf") } + @Test + def replGeneratedCodeDeepPackages(): Unit = { + val intp = newIMain() + val completer = new PresentationCompilerCompleter(intp) + intp.compileSources(new BatchSourceFile("", "package p1.p2.p3; object Ping { object Pong }")) + checkExact(completer, "p1.p2.p")("p3") + checkExact(completer, "p1.p2.p3.P")("Ping") + checkExact(completer, "p1.p2.p3.Ping.Po")("Pong") + } + def checkExact(completer: PresentationCompilerCompleter, before: String, after: String = "")(expected: String*): Unit = { assertEquals(expected.toSet, completer.complete(before, after).candidates.toSet) } -- cgit v1.2.3