Linked up $class visibility to symbol redefinition.

In what feels like divine intervention as I spent my entire day
yesterday unsuccessfully attempting to understand why running atop my
new classpath code, trunk would compile and then fail like this:

  build.xml:1683: Could not create type partest due to java.lang.NoSuchMethodError:
  scala.tools.ant.sabbus.CompilationPathProperty$class.$init$(Lscala/tools/ant/sabbus/CompilationPathProperty;)V

I discovered the link by trying to debug a seemingly completely
unrelated problem reported by pvlugter.

On the one hand you have PathResolver/ClassPath, which by default does
not place trait implementation classes on the compilation classpath, but
does do so under -optimise (as I understand it, this is so inlining can
be performed, so let us ignore the fact that methods in traits are never
inlined, as outlined in SI-4767.)

  object DefaultJavaContext extends JavaContext {
    override def isValidName(name: String) = !isTraitImplementation(name)
  }

Then on the other hand you have this logic in AddInterfaces:

  if (impl != NoSymbol && settings.optimise.value) {
    log("unlinking impl class " + impl)
    ...
  }

The test in AddInterfaces is hardcoded to only consider the value of
-optimise. Which means that in the completely default -optimise setup,
it corresponds to the answers given by "isValidName" in JavaContext,
but nothing keeps those elements in sync. The connection to my lost day
was that, thinking I was "simplifying" my development, I had commented
out the override in DefaultJavaContext so that all classes were on the
compilation path. This caused the state of settings.optimise (still
false) and whether impl classes were on the classpath (now true) to fall
into fatal disagreement.

4 files changed, 17 insertions, 10 deletions

diff --git a/src/compiler/scala/tools/ant/Scalac.scala b/src/compiler/scala/tools/ant/Scalac.scala
index 7aff4e3e8e..04ff0c440d 100644
--- a/src/compiler/scala/tools/ant/Scalac.scala
+++ b/src/compiler/scala/tools/ant/Scalac.scala
@@ -608,7 +608,7 @@ class Scalac extends ScalaMatchingTask with ScalacShared {
     if (!deprecation.isEmpty) settings.deprecation.value = deprecation.get
     if (!nobootcp.isEmpty) settings.nobootcp.value = nobootcp.get
     if (!nowarn.isEmpty) settings.nowarn.value = nowarn.get
-    if (!optimise.isEmpty) settings.XO.value = optimise.get
+    if (!optimise.isEmpty) settings.optimise.value = optimise.get
     if (!unchecked.isEmpty) settings.unchecked.value = unchecked.get
     if (!usejavacp.isEmpty) settings.usejavacp.value = usejavacp.get
 
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index 15b4c8c708..badf5d70d1 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -1528,7 +1528,7 @@ abstract class GenICode extends SubComponent  {
       if (mustUseAnyComparator) {
         // when -optimise is on we call the @inline-version of equals, found in ScalaRunTime
         val equalsMethod =
-          if (!settings.XO.value) {
+          if (!settings.optimise.value) {
             def default = platform.externalEquals
             platform match {
               case x: JavaPlatform =>
@@ -1550,7 +1550,7 @@ abstract class GenICode extends SubComponent  {
 
         val ctx1 = genLoad(l, ctx, ObjectReference)
         val ctx2 = genLoad(r, ctx1, ObjectReference)
-        ctx2.bb.emit(CALL_METHOD(equalsMethod, if (settings.XO.value) Dynamic else Static(false)))
+        ctx2.bb.emit(CALL_METHOD(equalsMethod, if (settings.optimise.value) Dynamic else Static(false)))
         ctx2.bb.emit(CZJUMP(thenCtx.bb, elseCtx.bb, NE, BOOL))
         ctx2.bb.close
       }
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
index 099145d3ae..b818927ceb 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -606,7 +606,7 @@ abstract class ClassfileParser {
   def parseField() {
     val jflags = in.nextChar
     var sflags = toScalaFieldFlags(jflags)
-    if ((sflags & PRIVATE) != 0L && !global.settings.XO.value) {
+    if ((sflags & PRIVATE) != 0L && !global.settings.optimise.value) {
       in.skip(4); skipAttributes()
     } else {
       val name    = pool.getName(in.nextChar)
@@ -637,7 +637,7 @@ abstract class ClassfileParser {
   def parseMethod() {
     val jflags = in.nextChar.toInt
     var sflags = toScalaMethodFlags(jflags)
-    if (isPrivate(jflags) && !global.settings.XO.value) {
+    if (isPrivate(jflags) && !global.settings.optimise.value) {
       val name = pool.getName(in.nextChar)
       if (name == nme.CONSTRUCTOR)
         sawPrivateConstructor = true
@@ -645,7 +645,7 @@ abstract class ClassfileParser {
     } else {
       if ((jflags & JAVA_ACC_BRIDGE) != 0)
         sflags |= BRIDGE
-      if ((sflags & PRIVATE) != 0L && global.settings.XO.value) {
+      if ((sflags & PRIVATE) != 0L && global.settings.optimise.value) {
         in.skip(4); skipAttributes()
       } else {
         val name = pool.getName(in.nextChar)
diff --git a/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala b/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala
index 8f5d308b8f..2244a4bf8b 100644
--- a/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala
+++ b/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala
@@ -81,10 +81,17 @@ abstract class AddInterfaces extends InfoTransform {
     atPhase(implClassPhase) {
       val implName = nme.implClassName(iface.name)
       var impl = if (iface.owner.isClass) iface.owner.info.decl(implName) else NoSymbol
-      if (impl != NoSymbol && settings.XO.value) {
-        log("unlinking impl class " + impl)
-        iface.owner.info.decls.unlink(impl)
-        impl = NoSymbol
+      if (impl != NoSymbol) {
+        // Unlink a pre-existing symbol only if the implementation class is
+        // visible on the compilation classpath.  In general this is true under
+        // -optimise and not otherwise, but the classpath can use arbitrary
+        // logic so the classpath must be queried.
+        if (classPath.context.isValidName(implName + ".class")) {
+          log("unlinking impl class " + impl)
+          iface.owner.info.decls.unlink(impl)
+          impl = NoSymbol
+        }
+        else log("not unlinking existing " + impl + " as the impl class is not visible on the classpath.")
       }
       if (impl == NoSymbol) {
         impl = iface.cloneSymbolImpl(iface.owner)