Fix classloader confusion in the REPL

The REPL constructed a classloader containing the user classpath but
having the classloader used to run the REPL as a parent, meaning that
the scala-library we actually used came from the JVM classpath, so of
course everything exploded when using a repl compiled with the 2.12
stdlib and running with the 2.13 stdlib. Fixed by having
Rendering#classpath be constructed without any parent, just based on the
user classpath. This required some additional changes:

- We called ScalaRuntime#replStringOf using the ScalaRunTime class from
the scala-library used by the REPL, switch to calling it reflectively
through
- ReplDriver#withRedirectedOutput used `Console.withOut/Err` but that only
affects the Console object in the current classloader, not the one in
the user classloader, use `System.setOut/Err` instead (the class
`System` comes from the system classloader so there's only one of it).
- The macro tests now need to be run with the compiler on the user
classpath, this required some refactoring (This would be simpler to
handle if ReplTest didn't extend ReplDriver, but I'll let someone else
do that refactoring)

Author: smarter

compiler/src/dotty/tools/repl/Rendering.scala

@@ -5,8 +5,6 @@ import java.io.{ StringWriter, PrintWriter }
 import java.lang.{ ClassLoader, ExceptionInInitializerError }
 import java.lang.reflect.InvocationTargetException
 
-import scala.runtime.ScalaRunTime
-
 import dotc.core.Contexts.Context
 import dotc.core.Denotations.Denotation
 import dotc.core.Flags
@@ -23,25 +21,42 @@ import dotc.core.StdNames.str
  */
 private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {
 
+  private[this] val MaxStringElements: Int = 1000  // no need to mkString billions of elements
+
   private[this] var myClassLoader: ClassLoader = _
 
+  private[this] var myReplStringOf: Object => String = _
+
+
   /** Class loader used to load compiled code */
   private[repl] def classLoader()(implicit ctx: Context) =
     if (myClassLoader != null) myClassLoader
     else {
       val parent = parentClassLoader.getOrElse {
-        // the compiler's classpath, as URL's
         val compilerClasspath = ctx.platform.classPath(ctx).asURLs
-        new java.net.URLClassLoader(compilerClasspath.toArray, classOf[ReplDriver].getClassLoader)
+        new java.net.URLClassLoader(compilerClasspath.toArray, null)
       }
 
       myClassLoader = new AbstractFileClassLoader(ctx.settings.outputDir.value, parent)
-      // Set the current Java "context" class loader to this rendering class loader
-      Thread.currentThread.setContextClassLoader(myClassLoader)
+      myReplStringOf = {
+        // We need to use the ScalaRunTime class coming from the scala-library
+        // on the user classpath, and not the one avilable in the current
+        // classloader, so we use reflection instead of simply calling
+        // `ScalaRunTime.replStringOf`.
+        val scalaRuntime = Class.forName("scala.runtime.ScalaRunTime", true, myClassLoader)
+        val meth = scalaRuntime.getMethod("replStringOf", classOf[Object], classOf[Int])
+
+        (value: Object) => meth.invoke(null, value, Integer.valueOf(MaxStringElements)).asInstanceOf[String]
+      }
       myClassLoader
     }
 
-  private[this] def MaxStringElements = 1000  // no need to mkString billions of elements
+  /** Return a String representation of a value we got from `classLoader()`. */
+  private[repl] def replStringOf(value: Object)(implicit ctx: Context): String = {
+    assert(myReplStringOf != null,
+      "replStringOf should only be called on values creating using `classLoader()`, but `classLoader()` has not been called so far")
+    myReplStringOf(value)
+  }
 
   /** Load the value of the symbol using reflection.
    *
@@ -55,7 +70,7 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {
       resObj
         .getDeclaredMethods.find(_.getName == sym.name.encode.toString)
         .map(_.invoke(null))
-    val string = value.map(ScalaRunTime.replStringOf(_, MaxStringElements).trim)
+    val string = value.map(replStringOf(_).trim)
     if (!sym.is(Flags.Method) && sym.info == defn.UnitType)
       None
     else

compiler/src/dotty/tools/repl/ReplDriver.scala

@@ -139,8 +139,19 @@ class ReplDriver(settings: Array[String],
   // TODO: i5069
   final def bind(name: String, value: Any)(implicit state: State): State = state
 
-  private def withRedirectedOutput(op: => State): State =
-    Console.withOut(out) { Console.withErr(out) { op } }
+  private def withRedirectedOutput(op: => State): State = {
+    val savedOut = System.out
+    val savedErr = System.err
+    try {
+      System.setOut(out)
+      System.setErr(out)
+      op
+    }
+    finally {
+      System.setOut(savedOut)
+      System.setErr(savedErr)
+    }
+  }
 
   private def newRun(state: State) = {
     val run = compiler.newRun(rootCtx.fresh.setReporter(newStoreReporter), state)

compiler/test-resources/repl/i5551 renamed to compiler/test-resources/repl-macros/i5551

@@ -3,19 +3,27 @@ package repl
 
 import vulpix.TestConfiguration
 
-import java.io.{ByteArrayOutputStream, PrintStream}
+import java.lang.System.{lineSeparator => EOL}
+import java.io.{ByteArrayOutputStream, File => JFile, PrintStream}
+import scala.io.Source
+
+import scala.collection.mutable.ArrayBuffer
 
 import dotty.tools.dotc.reporting.MessageRendering
 import org.junit.{After, Before}
+import org.junit.Assert._
 
 
-class ReplTest private (out: ByteArrayOutputStream) extends ReplDriver(
-  Array("-classpath", TestConfiguration.basicClasspath, "-color:never"),
+class ReplTest(withCompiler: Boolean = false, out: ByteArrayOutputStream = new ByteArrayOutputStream) extends ReplDriver(
+  Array(
+    "-classpath",
+    if (withCompiler)
+      TestConfiguration.withCompilerClasspath
+    else
+      TestConfiguration.basicClasspath,
+    "-color:never"),
   new PrintStream(out)
 ) with MessageRendering {
-
-  def this() = this(new ByteArrayOutputStream)
-
   /** Get the stored output from `out`, resetting the buffer */
   def storedOutput(): String = {
     val output = stripColor(out.toString)
@@ -37,4 +45,77 @@ class ReplTest private (out: ByteArrayOutputStream) extends ReplDriver(
   implicit class TestingState(state: State) {
     def andThen[A](op: State => A): A = op(state)
   }
+
+  def scripts(path: String): Array[JFile] = {
+    val dir = new JFile(getClass.getResource(path).getPath)
+    assert(dir.exists && dir.isDirectory, "Couldn't load scripts dir")
+    dir.listFiles
+  }
+
+  def testFile(f: JFile): Unit = {
+    val prompt = "scala>"
+    val lines = Source.fromFile(f, "UTF-8").getLines().buffered
+
+    assert(lines.head.startsWith(prompt),
+           s"""Each file has to start with the prompt: "$prompt"""")
+
+    def extractInputs(prompt: String): List[String] = {
+      val input = lines.next()
+
+      if (!input.startsWith(prompt)) extractInputs(prompt)
+      else if (lines.hasNext) {
+        // read lines and strip trailing whitespace:
+        while (lines.hasNext && !lines.head.startsWith(prompt))
+          lines.next()
+
+        input :: { if (lines.hasNext) extractInputs(prompt) else Nil }
+      }
+      else Nil
+    }
+
+    def evaluate(state: State, input: String, prompt: String) =
+      try {
+        val nstate = run(input.drop(prompt.length))(state)
+        val out = input + EOL + storedOutput()
+        (out, nstate)
+      }
+      catch {
+        case ex: Throwable =>
+          System.err.println(s"failed while running script: $f, on:\n$input")
+          throw ex
+      }
+
+    def filterEmpties(line: String): List[String] =
+      line.replaceAll("""(?m)\s+$""", "") match {
+        case "" => Nil
+        case nonEmptyLine => nonEmptyLine :: Nil
+      }
+
+    val expectedOutput =
+      Source.fromFile(f, "UTF-8").getLines().flatMap(filterEmpties).mkString(EOL)
+    val actualOutput = {
+      resetToInitial()
+      val inputRes = extractInputs(prompt)
+      val buf = new ArrayBuffer[String]
+      inputRes.foldLeft(initialState) { (state, input) =>
+        val (out, nstate) = evaluate(state, input, prompt)
+        buf.append(out)
+
+        assert(out.endsWith("\n"),
+               s"Expected output of $input to end with newline")
+
+        nstate
+      }
+      buf.flatMap(filterEmpties).mkString(EOL)
+    }
+
+    if (expectedOutput != actualOutput) {
+      println("expected =========>")
+      println(expectedOutput)
+      println("actual ===========>")
+      println(actualOutput)
+
+      fail(s"Error in file $f, expected output did not match actual")
+    }
+  }
 }

compiler/test/dotty/tools/repl/ReplTest.scala

@@ -2,98 +2,16 @@ package dotty
 package tools
 package repl
 
-import java.io.{File => JFile}
-import java.lang.System.{lineSeparator => EOL}
-
-import org.junit.Assert._
 import org.junit.Test
-import org.junit.experimental.categories.Category
-
-import scala.collection.mutable.ArrayBuffer
-import scala.io.Source
-
-import dotc.reporting.MessageRendering
 
 /** Runs all tests contained in `compiler/test-resources/repl/` */
-class ScriptedTests extends ReplTest with MessageRendering {
-
-  private def scripts(path: String): Array[JFile] = {
-    val dir = new JFile(getClass.getResource(path).getPath)
-    assert(dir.exists && dir.isDirectory, "Couldn't load scripts dir")
-    dir.listFiles
-  }
-
-  private def testFile(f: JFile): Unit = {
-    val prompt = "scala>"
-    val lines = Source.fromFile(f, "UTF-8").getLines().buffered
-
-    assert(lines.head.startsWith(prompt),
-           s"""Each file has to start with the prompt: "$prompt"""")
-
-    def extractInputs(prompt: String): List[String] = {
-      val input = lines.next()
-
-      if (!input.startsWith(prompt)) extractInputs(prompt)
-      else if (lines.hasNext) {
-        // read lines and strip trailing whitespace:
-        while (lines.hasNext && !lines.head.startsWith(prompt))
-          lines.next()
-
-        input :: { if (lines.hasNext) extractInputs(prompt) else Nil }
-      }
-      else Nil
-    }
-
-    def evaluate(state: State, input: String, prompt: String) =
-      try {
-        val nstate = run(input.drop(prompt.length))(state)
-        val out = input + EOL + storedOutput()
-        (out, nstate)
-      }
-      catch {
-        case ex: Throwable =>
-          System.err.println(s"failed while running script: $f, on:\n$input")
-          throw ex
-      }
-
-    def filterEmpties(line: String): List[String] =
-      line.replaceAll("""(?m)\s+$""", "") match {
-        case "" => Nil
-        case nonEmptyLine => nonEmptyLine :: Nil
-      }
-
-    val expectedOutput =
-      Source.fromFile(f, "UTF-8").getLines().flatMap(filterEmpties).mkString(EOL)
-    val actualOutput = {
-      resetToInitial()
-      val inputRes = extractInputs(prompt)
-      val buf = new ArrayBuffer[String]
-      inputRes.foldLeft(initialState) { (state, input) =>
-        val (out, nstate) = evaluate(state, input, prompt)
-        buf.append(out)
-
-        assert(out.endsWith("\n"),
-               s"Expected output of $input to end with newline")
-
-        nstate
-      }
-      buf.flatMap(filterEmpties).mkString(EOL)
-    }
-
-    if (expectedOutput != actualOutput) {
-      println("expected =========>")
-      println(expectedOutput)
-      println("actual ===========>")
-      println(actualOutput)
-
-      fail(s"Error in file $f, expected output did not match actual")
-    }
-  }
+class ScriptedTests extends ReplTest {
+  import ScriptedTests._
 
   @Test def replTests = scripts("/repl").foreach(testFile)
 
   @Test def typePrinterTests = scripts("/type-printer").foreach(testFile)
+}
 
-  @Category(Array(classOf[BootstrappedOnlyTests]))
-  @Test def replMacrosTests = scripts("/repl-macros").foreach(testFile)
+object ScriptedTests {
 }