SI-7747 Support class based wrappers clean up

Simplified the code paths to just use one of two `Wrapper` types
for textual templating.

Simplified the class-based template to use the same `$iw` name
for the both the class and the wrapper value.  In addition,
the $read value is an object extending $read, instead of containing
an extra instance field, which keeps paths to values the same
for both templates.

Both styles trigger loading the value object by referencing the
value that immediately wraps the user code, although for the
class style, inner vals are eager and it would suffice to load
the enclosing `$read` object.

The proposed template included extra vals for values imported
from history, but this is not necessary since such an import
is always a stable path. (Or, counter-example to test is welcome.)

The test for t5148 is updated as a side effect. Probably internal
APIs don't make good test subjects.

Modify -Y option message.

7 files changed, 74 insertions, 102 deletions

diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
index ac73df930a..7568c789fb 100644
--- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
@@ -174,7 +174,7 @@ trait ScalaSettings extends AbsScalaSettings
   val Yreifycopypaste = BooleanSetting    ("-Yreify-copypaste", "Dump the reified trees in copypasteable representation.")
   val Ymacronoexpand  = BooleanSetting    ("-Ymacro-no-expand", "Don't expand macros. Might be useful for scaladoc and presentation compiler, but will crash anything which uses macros and gets past typer.")
   val Yreplsync       = BooleanSetting    ("-Yrepl-sync", "Do not use asynchronous code for repl startup")
-  val Yreplclassbased = BooleanSetting    ("-Yrepl-class-based", "Uses classes instead of objects ( which is default) to wrap repl code.")
+  val Yreplclassbased = BooleanSetting    ("-Yrepl-class-based", "Use classes to wrap REPL snippets instead of objects")
   val Yreploutdir     = StringSetting     ("-Yrepl-outdir", "path", "Write repl-generated classfiles to given output directory (use \"\" to generate a temporary dir)" , "")
   val YmethodInfer    = BooleanSetting    ("-Yinfer-argument-types", "Infer types for arguments of overriden methods.")
   val etaExpandKeepsStar = BooleanSetting ("-Yeta-expand-keeps-star", "Eta-expand varargs methods to T* rather than Seq[T].  This is a temporary option to ease transition.").withDeprecationMessage(removalIn212)
diff --git a/src/repl/scala/tools/nsc/interpreter/ClassBasedPaths.scala b/src/repl/scala/tools/nsc/interpreter/ClassBasedPaths.scala
deleted file mode 100644
index d242a6a0c0..0000000000
--- a/src/repl/scala/tools/nsc/interpreter/ClassBasedPaths.scala
+++ /dev/null
@@ -1,8 +0,0 @@
-package scala.tools.nsc.interpreter
-
-trait ClassBasedPaths {
-  self: IMain =>
-  override def transformPath(p: String): String = p replaceFirst("read", "read.INSTANCE") replaceAll("iwC", "iw")
-
-  override def readRootPath(readPath: String) = getClassIfDefined(readPath)
-}
diff --git a/src/repl/scala/tools/nsc/interpreter/ILoop.scala b/src/repl/scala/tools/nsc/interpreter/ILoop.scala
index 4b90128a99..a96bed4696 100644
--- a/src/repl/scala/tools/nsc/interpreter/ILoop.scala
+++ b/src/repl/scala/tools/nsc/interpreter/ILoop.scala
@@ -119,9 +119,8 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter)
   def createInterpreter() {
     if (addedClasspath != "")
       settings.classpath append addedClasspath
-    if (settings.Yreplclassbased.value)
-      intp = new ILoopInterpreter with ClassBasedPaths
-    else intp = new ILoopInterpreter
+
+    intp = new ILoopInterpreter
   }
 
   /** print a friendly help message */
diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala
index b2d5a0a149..0d55423247 100644
--- a/src/repl/scala/tools/nsc/interpreter/IMain.scala
+++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala
@@ -81,8 +81,6 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
   private var _classLoader: util.AbstractFileClassLoader = null                              // active classloader
   private val _compiler: ReplGlobal                 = newCompiler(settings, reporter)   // our private compiler
 
-  val classBasedWrappers = settings.Yreplclassbased.value
-
   def compilerClasspath: Seq[java.net.URL] = (
     if (isInitializeComplete) global.classPath.asURLs
     else new PathResolver(settings).result.asURLs  // the compiler's classpath
@@ -274,14 +272,13 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
       case s                                 => s
     } mkString "."
   )
-  def transformPath(p: String): String = p
   def readRootPath(readPath: String) = getModuleIfDefined(readPath)
 
   abstract class PhaseDependentOps {
     def shift[T](op: => T): T
 
     def path(name: => Name): String = shift(path(symbolOfName(name)))
-    def path(sym: Symbol): String = backticked(transformPath(shift(sym.fullName)))
+    def path(sym: Symbol): String = backticked(shift(sym.fullName))
     def sig(sym: Symbol): String  = shift(sym.defString)
   }
   object typerOp extends PhaseDependentOps {
@@ -852,54 +849,69 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
     /** Code to import bound names from previous lines - accessPath is code to
       * append to objectName to access anything bound by request.
       */
-    val ComputedImports(importsPreamble, importsTrailer, accessPath) =
-      exitingTyper(importsCode(referencedNames.toSet))
+    lazy val ComputedImports(importsPreamble, importsTrailer, accessPath) =
+      exitingTyper(importsCode(referencedNames.toSet, ObjectSourceCode))
 
     /** the line of code to compute */
     def toCompute = line
 
-    def fullPath(vname: String) = s"${codeWrapper.fullAccessPath}.`$vname`"
-
-    trait ClassBasedWrappers {
-      self: Wrappers =>
-      override def preambleHeader = "class %s extends Serializable {"
-
-      override def fullAccessPath = s"${lineRep.readPath}.INSTANCE$accessPath"
-
-      override def postamble = importsTrailer + "\n}" + s"""
-                                      |object ${lineRep.readName} {
-                                      |val INSTANCE = new ${lineRep.readName}();
-                                      |}""".stripMargin
-    }
-
-    class Wrappers {
-      def preambleHeader = "object %s {"
+    /** The path of the value that contains the user code. */
+    def fullAccessPath = s"${lineRep.readPath}$accessPath"
 
-      def fullAccessPath = s"${lineRep.readPath}$accessPath"
-
-      def postamble = importsTrailer + "\n}"
-    }
-
-    val codeWrapper = if (!classBasedWrappers) new Wrappers else new Wrappers with ClassBasedWrappers
+    /** The path of the given member of the wrapping instance. */
+    def fullPath(vname: String) = s"$fullAccessPath.`$vname`"
 
     /** generate the source code for the object that computes this request */
-    private object ObjectSourceCode extends IMain.CodeAssembler[MemberHandler] {
+    abstract class Wrapper extends IMain.CodeAssembler[MemberHandler] {
       def path = originalPath("$intp")
       def envLines = {
         if (!isReplPower) Nil // power mode only for now
         else List("def %s = %s".format("$line", tquoted(originalLine)), "def %s = Nil".format("$trees"))
       }
-      val preamble = s"""
-        |${codeWrapper.preambleHeader}
+      def preamble = s"""
+        |$preambleHeader
         |%s%s%s
       """.stripMargin.format(lineRep.readName, envLines.map("  " + _ + ";\n").mkString,
         importsPreamble, indentCode(toCompute))
 
-      val postamble = codeWrapper.postamble
-
       val generate = (m: MemberHandler) => m extraCodeToEvaluate Request.this
+
+      /** A format string with %s for $read, specifying the wrapper definition. */
+      def preambleHeader: String
+
+      /** Like preambleHeader for an import wrapper. */
+      def prewrap: String = preambleHeader + "\n"
+
+      /** Like postamble for an import wrapper. */
+      def postwrap: String
     }
 
+    private class ObjectBasedWrapper extends Wrapper {
+      def preambleHeader = "object %s {"
+
+      def postamble = importsTrailer + "\n}"
+
+      def postwrap = "}\n"
+    }
+
+    private class ClassBasedWrapper extends Wrapper {
+      def preambleHeader = "class %s extends Serializable {"
+
+      /** Adds an object that instantiates the outer wrapping class. */
+      def postamble  = s"""$importsTrailer
+                          |}
+                          |object ${lineRep.readName} extends ${lineRep.readName}
+                          |""".stripMargin
+
+      import nme.{ INTERPRETER_IMPORT_WRAPPER => iw }
+
+      /** Adds a val that instantiates the wrapping class. */
+      def postwrap = s"}\nval $iw = new $iw\n"
+    }
+
+    private lazy val ObjectSourceCode: Wrapper =
+      if (settings.Yreplclassbased) new ClassBasedWrapper else new ObjectBasedWrapper
+
     private object ResultObjectSourceCode extends IMain.CodeAssembler[MemberHandler] {
       /** We only want to generate this code when the result
        *  is a value which can be referred to as-is.
@@ -918,7 +930,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
       |    (""
       """.stripMargin.format(
         lineRep.evalName, evalResult, lineRep.printName,
-        executionWrapper, codeWrapper.fullAccessPath
+        executionWrapper, fullAccessPath
       )
 
       val postamble = """
@@ -1247,7 +1259,7 @@ object IMain {
   // $line3.$read.$iw.$iw.Bippy =
   //   $line3.$read$$iw$$iw$Bippy@4a6a00ca
   private def removeLineWrapper(s: String) = s.replaceAll("""\$line\d+[./]\$(read|eval|print)[$.]""", "")
-  private def removeIWPackages(s: String)  = s.replaceAll("""\$(iw|iwC|read|eval|print)[$.]""", "")
+  private def removeIWPackages(s: String)  = s.replaceAll("""\$(iw|read|eval|print)[$.]""", "")
   def stripString(s: String)               = removeIWPackages(removeLineWrapper(s))
 
   trait CodeAssembler[T] {
@@ -1267,7 +1279,6 @@ object IMain {
     def stripImpl(str: String): String
     def strip(str: String): String = if (isStripping) stripImpl(str) else str
   }
-
   trait TruncatingWriter {
     def maxStringLength: Int
     def isTruncating: Boolean
@@ -1277,7 +1288,6 @@ object IMain {
       else str
     }
   }
-
   abstract class StrippingTruncatingWriter(out: JPrintWriter)
           extends JPrintWriter(out)
              with StrippingWriter
diff --git a/src/repl/scala/tools/nsc/interpreter/Imports.scala b/src/repl/scala/tools/nsc/interpreter/Imports.scala
index c8f9a677b2..5244858a62 100644
--- a/src/repl/scala/tools/nsc/interpreter/Imports.scala
+++ b/src/repl/scala/tools/nsc/interpreter/Imports.scala
@@ -92,7 +92,7 @@ trait Imports {
    * last one imported is actually usable.
    */
   case class ComputedImports(prepend: String, append: String, access: String)
-  protected def importsCode(wanted: Set[Name]): ComputedImports = {
+  protected def importsCode(wanted: Set[Name], wrapper: Request#Wrapper): ComputedImports = {
     /** Narrow down the list of requests from which imports
      *  should be taken.  Removes requests which cannot contribute
      *  useful imports for the specified set of wanted names.
@@ -130,23 +130,10 @@ trait Imports {
 
     // add code for a new object to hold some imports
     def addWrapper() {
-      if (classBasedWrappers) addClassBasedWrapper()
-      else addObjectBasedWrapper()
-    }
-
-    def addObjectBasedWrapper() {
-      val impname = nme.INTERPRETER_IMPORT_WRAPPER
-      code append "object %s {\n".format(impname)
-      trailingBraces append "}\n"
-      accessPath append ("." + impname)
-      currentImps.clear()
-    }
-
-    def addClassBasedWrapper() {
-      val impname = nme.INTERPRETER_IMPORT_WRAPPER
-      code append "class %sC extends Serializable {\n".format(impname)
-      trailingBraces append "}\nval " + impname + " = new " + impname + "C;\n"
-      accessPath append ("." + impname)
+      import nme.{ INTERPRETER_IMPORT_WRAPPER => iw }
+      code append (wrapper.prewrap format iw)
+      trailingBraces append wrapper.postwrap
+      accessPath append s".$iw"
       currentImps.clear()
     }
 
@@ -178,14 +165,7 @@ trait Imports {
           case x =>
             for (sym <- x.definedSymbols) {
               maybeWrap(sym.name)
-              if (classBasedWrappers) {
-                if (!code.toString.endsWith(".`" + sym.name + "`;\n")) {
-                  val valName = "$VAL" + req.lineRep.lineId
-                  code.append("val " + valName + " = " + req.lineRep.readPath + ".INSTANCE;\n")
-                  code.append("import " + valName + req.accessPath + ".`" + sym.name + "`;\n")
-                }
-              } else
-                code append s"import ${x.path}\n"
+              code append s"import ${x.path}\n"
               currentImps += sym.name
             }
         }
diff --git a/test/files/neg/t5148.check b/test/files/neg/t5148.check
index 25107c4dbe..f426bd2010 100644
--- a/test/files/neg/t5148.check
+++ b/test/files/neg/t5148.check
@@ -1,9 +1,5 @@
-error: bad symbolic reference. A signature in Imports.class refers to term global
+error: bad symbolic reference. A signature in Imports.class refers to type Request
 in class scala.tools.nsc.interpreter.IMain which is not available.
 It may be completely missing from the current classpath, or the version on
 the classpath might be incompatible with the version used when compiling Imports.class.
-error: bad symbolic reference. A signature in Imports.class refers to term memberHandlers
-in class scala.tools.nsc.interpreter.IMain which is not available.
-It may be completely missing from the current classpath, or the version on
-the classpath might be incompatible with the version used when compiling Imports.class.
-two errors found
+one error found
diff --git a/test/files/run/t7747-repl.check b/test/files/run/t7747-repl.check
index ae7ea67e61..ad924f482c 100644
--- a/test/files/run/t7747-repl.check
+++ b/test/files/run/t7747-repl.check
@@ -57,15 +57,10 @@ defined class Ruminant
 res3: Int = 216
 
 scala> Bovine.x = List(Ruminant(5), Cow, new Moo)
-<console>:8: error: $VAL10 is already defined as value $VAL10
-val $VAL10 = INSTANCE;
-    ^
-<console>:12: error: $VAL11 is already defined as value $VAL11
-val $VAL11 = INSTANCE;
-    ^
+Bovine.x: List[Any] = List(Ruminant(5), Cow, Moooooo)
 
 scala> Bovine.x
-res4: List[Any] = null
+res4: List[Any] = List(Ruminant(5), Cow, Moooooo)
 
 scala> 
 
@@ -117,7 +112,7 @@ scala> 55 ; ((2 + 2)) ; (1, 2, 3)
 res15: (Int, Int, Int) = (1,2,3)
 
 scala> 55 ; (x: Int) => x + 1 ; () => ((5))
-<console>:9: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+<console>:8: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
               55 ; (x: Int) => x + 1 ;;
               ^
 res16: () => Int = <function0>
@@ -185,6 +180,7 @@ val z = x * y
   2 +
   3 } ; bippy+88+11
 object Bovine { var x: List[_] = null } ; case class Ruminant(x: Int) ; bippy * bippy * bippy
+Bovine.x = List(Ruminant(5), Cow, new Moo)
 Bovine.x
 (2)
 (2 + 2)
@@ -257,34 +253,33 @@ class $read extends Serializable {
     super.<init>;
     ()
   };
-  class $iwC extends Serializable {
+  class $iw extends Serializable {
     def <init>() = {
       super.<init>;
       ()
     };
-    val $VAL44 = $line44.$read.INSTANCE;
-    import $VAL44.$iw.$iw.BippyBups;
-    val $VAL45 = $line45.$read.INSTANCE;
-    import $VAL45.$iw.$iw.PuppyPups;
-    val $VAL46 = $line46.$read.INSTANCE;
-    import $VAL46.$iw.$iw.Bingo;
-    class $iwC extends Serializable {
+    import $line44.$read.$iw.$iw.BippyBups;
+    import $line44.$read.$iw.$iw.BippyBups;
+    import $line45.$read.$iw.$iw.PuppyPups;
+    import $line45.$read.$iw.$iw.PuppyPups;
+    import $line46.$read.$iw.$iw.Bingo;
+    import $line46.$read.$iw.$iw.Bingo;
+    class $iw extends Serializable {
       def <init>() = {
         super.<init>;
         ()
       };
       val res3 = List(BippyBups, PuppyPups, Bingo)
     };
-    val $iw = new $iwC.<init>
+    val $iw = new $iw.<init>
   };
-  val $iw = new $iwC.<init>
+  val $iw = new $iw.<init>
 }
-object $read extends scala.AnyRef {
+object $read extends $read {
   def <init>() = {
     super.<init>;
     ()
-  };
-  val INSTANCE = new $read.<init>
+  }
 }
 res3: List[Product with Serializable] = List(BippyBups(), PuppyPups(), Bingo())