Merge pull request #221 from jvican/fix-asf

Fix #113 and apply several other fixes

internal/compiler-bridge/src-2.10/main/scala/xsbt/ExtractAPI.scala

@@ -136,18 +136,15 @@ class ExtractAPI[GlobalType <: Global](
     def renaming(symbol: Symbol): Option[String] = renameTo.get(symbol)
   }
 
-  // call back to the xsbti.SafeLazy class in main sbt code to construct a SafeLazy instance
-  //   we pass a thunk, whose class is loaded by the interface class loader (this class's loader)
-  //   SafeLazy ensures that once the value is forced, the thunk is nulled out and so
-  //   references to the thunk's classes are not retained.  Specifically, it allows the interface classes
-  //   (those in this subproject) to be garbage collected after compilation.
-  private[this] val safeLazy = Class.forName("xsbti.SafeLazy").getMethod("apply", classOf[xsbti.F0[_]])
-  private def lzy[S <: AnyRef](s: => S): xsbti.api.Lazy[S] =
-    {
-      val z = safeLazy.invoke(null, Message(s)).asInstanceOf[xsbti.api.Lazy[S]]
-      pending += z
-      z
-    }
+  /**
+   * Construct a lazy instance from a by-name parameter that will null out references to once
+   * the value is forced and therefore references to thunk's classes will be garbage collected.
+   */
+  private def lzy[S <: AnyRef](s: => S): xsbti.api.Lazy[S] = {
+    val lazyImpl = xsbti.api.SafeLazy.apply(Message(s))
+    pending += lazyImpl
+    lazyImpl
+  }
 
   /**
    * Force all lazy structures.  This is necessary so that we see the symbols/types at this phase and

internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala

@@ -58,7 +58,7 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
           dependencyExtractor.localInheritanceDependencies foreach processDependency(context = LocalDependencyByInheritance)
           processTopLevelImportDependencies(dependencyExtractor.topLevelImportDependencies)
         } else {
-          throw new UnsupportedOperationException("Turning off name hashing is not supported in class-based dependency trackging.")
+          throw new UnsupportedOperationException(Feedback.NameHashingDisabled)
         }
         /*
          * Registers top level import dependencies as coming from a first top level class/trait/object declared
@@ -75,13 +75,7 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
               deps foreach { dep =>
                 processDependency(context = DependencyByMemberRef)(ClassDependency(firstClassSymbol, dep))
               }
-            case None =>
-              reporter.warning(
-                unit.position(0),
-                """|Found top level imports but no class, trait or object is defined in the compilation unit.
-                  |The incremental compiler cannot record the dependency information in such case.
-                  |Some errors like unused import referring to a non-existent class might not be reported.""".stripMargin
-              )
+            case None => reporter.warning(unit.position(0), Feedback.OrphanTopLevelImports)
           }
         }
         /*
@@ -163,10 +157,7 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
       }
     }
     private def addClassDependency(deps: HashSet[ClassDependency], fromClass: Symbol, dep: Symbol): Unit = {
-      assert(
-        fromClass.isClass,
-        s"The ${fromClass.fullName} defined at ${fromClass.fullLocationString} is not a class symbol."
-      )
+      assert(fromClass.isClass, Feedback.expectedClassSymbol(fromClass))
       val depClass = enclOrModuleClass(dep)
       if (fromClass.associatedFile != depClass.associatedFile) {
         deps += ClassDependency(fromClass, depClass)
@@ -182,25 +173,18 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
       }
     }
 
-    @inline
-    def ignoreType(tpe: Type) =
-      tpe == null ||
-        tpe == NoType ||
-        tpe.typeSymbol == EmptyPackageClass
-
     private def addTreeDependency(tree: Tree): Unit = {
       addDependency(tree.symbol)
       val tpe = tree.tpe
-      if (!ignoreType(tpe))
-        foreachSymbolInType(tpe)(addDependency)
+      if (!ignoredType(tpe))
+        foreachNotPackageSymbolInType(tpe)(addDependency)
       ()
     }
     private def addDependency(dep: Symbol): Unit = {
       val fromClass = resolveDependencySource().fromClass
-      if (fromClass == NoSymbol || fromClass.hasPackageFlag) {
+      if (ignoredSymbol(fromClass) || fromClass.hasPackageFlag) {
         if (inImportNode) addTopLevelImportDependency(dep)
-        else
-          devWarning(s"No enclosing class. Discarding dependency on $dep (currentOwner = $currentOwner).")
+        else devWarning(Feedback.missingEnclosingClass(dep, currentOwner))
       } else {
         addClassDependency(_memberRefDependencies, fromClass, dep)
       }
@@ -276,12 +260,12 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
         traverseTrees(body)
 
       // In some cases (eg. macro annotations), `typeTree.tpe` may be null. See sbt/sbt#1593 and sbt/sbt#1655.
-      case typeTree: TypeTree if !ignoreType(typeTree.tpe) =>
-        symbolsInType(typeTree.tpe) foreach addDependency
+      case typeTree: TypeTree if !ignoredType(typeTree.tpe) =>
+        foreachNotPackageSymbolInType(typeTree.tpe)(addDependency)
       case m @ MacroExpansionOf(original) if inspectedOriginalTrees.add(original) =>
         traverse(original)
         super.traverse(m)
-      case _: ClassDef | _: ModuleDef if tree.symbol != null && tree.symbol != NoSymbol =>
+      case _: ClassDef | _: ModuleDef if !ignoredSymbol(tree.symbol) =>
         // make sure we cache lookups for all classes declared in the compilation unit; the recorded information
         // will be used in Analyzer phase
         val sym = if (tree.symbol.isModule) tree.symbol.moduleClass else tree.symbol
@@ -295,7 +279,7 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
       addDependency(symbol)
     }
     val addSymbolsFromType: Type => Unit = { tpe =>
-      foreachSymbolInType(tpe)(addDependency)
+      foreachNotPackageSymbolInType(tpe)(addDependency)
     }
   }
 

internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala

@@ -136,18 +136,15 @@ class ExtractAPI[GlobalType <: Global](
     def renaming(symbol: Symbol): Option[String] = renameTo.get(symbol)
   }
 
-  // call back to the xsbti.SafeLazy class in main sbt code to construct a SafeLazy instance
-  //   we pass a thunk, whose class is loaded by the interface class loader (this class's loader)
-  //   SafeLazy ensures that once the value is forced, the thunk is nulled out and so
-  //   references to the thunk's classes are not retained.  Specifically, it allows the interface classes
-  //   (those in this subproject) to be garbage collected after compilation.
-  private[this] val safeLazy = Class.forName("xsbti.SafeLazy").getMethod("apply", classOf[xsbti.F0[_]])
-  private def lzy[S <: AnyRef](s: => S): xsbti.api.Lazy[S] =
-    {
-      val z = safeLazy.invoke(null, Message(s)).asInstanceOf[xsbti.api.Lazy[S]]
-      pending += z
-      z
-    }
+  /**
+   * Construct a lazy instance from a by-name parameter that will null out references to once
+   * the value is forced and therefore references to thunk's classes will be garbage collected.
+   */
+  private def lzy[S <: AnyRef](s: => S): xsbti.api.Lazy[S] = {
+    val lazyImpl = xsbti.api.SafeLazy.apply(Message(s))
+    pending += lazyImpl
+    lazyImpl
+  }
 
   /**
    * Force all lazy structures.  This is necessary so that we see the symbols/types at this phase and
@@ -624,4 +621,4 @@ class ExtractAPI[GlobalType <: Global](
     implicit def compat(ann: AnnotationInfo): IsStatic = new IsStatic(ann)
     annotations.filter(_.isStatic)
   }
-}
+}

internal/compiler-bridge/src/main/scala/xsbt/ExtractUsedNames.scala

@@ -22,11 +22,9 @@ import scala.collection.mutable
  * Names mentioned in Import nodes are handled properly but require some special logic for two
  * reasons:
  *
- *   1. import node itself has a term symbol associated with it with a name `<import`>.
- *      I (gkossakowski) tried to track down what role this symbol serves but I couldn't.
- *      It doesn't look like there are many places in Scala compiler that refer to
- *      that kind of symbols explicitly.
- *   2. ImportSelector is not subtype of Tree therefore is not processed by `Tree.foreach`
+ *   1. The `termSymbol` of Import nodes point to the symbol of the prefix it imports from
+ *      (not the actual members that we import, that are represented as names).
+ *   2. ImportSelector is not subtype of Tree therefore is not processed by `Tree.foreach`.
  *
  * Another type of tree nodes that requires special handling is TypeTree. TypeTree nodes
  * has a little bit odd representation:
@@ -65,12 +63,7 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
           val firstClassName = className(firstClassSymbol)
           traverser.usedNamesFromClass(firstClassName) ++= namesUsedAtTopLevel.map(decodeName)
         case None =>
-          reporter.warning(
-            unit.position(0),
-            """|Found names used at the top level but no class, trait or object is defined in the compilation unit.
-              |The incremental compiler cannot record used names in such case.
-              |Some errors like unused import referring to a non-existent class might not be reported.""".stripMargin
-          )
+          reporter.warning(unit.position(0), Feedback.OrphanNames)
       }
     }
 
@@ -106,14 +99,15 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
     }
 
     /** Returns mutable set with all names from given class used in current context */
-    def usedNamesFromClass(className: String): collection.mutable.Set[String] =
+    def usedNamesFromClass(className: String): collection.mutable.Set[String] = {
       usedNamesFromClasses.get(className) match {
         case None =>
           val emptySet = scala.collection.mutable.Set.empty[String]
           usedNamesFromClasses.put(className, emptySet)
           emptySet
         case Some(setForClass) => setForClass
       }
+    }
 
     /*
      * Some macros appear to contain themselves as original tree.
@@ -130,10 +124,6 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
 
     private def handleClassicTreeNode(tree: Tree): Unit = tree match {
       case _: DefTree | _: Template => ()
-      // turns out that Import node has a TermSymbol associated with it
-      // I (Grzegorz) tried to understand why it's there and what does it represent but
-      // that logic was introduced in 2005 without any justification I'll just ignore the
-      // import node altogether and just process the selectors in the import node
       case Import(_, selectors: List[ImportSelector]) =>
         val enclosingNonLocalClass = resolveEnclosingNonLocalClass()
         def usedNameInImportSelector(name: Name): Unit =
@@ -154,7 +144,7 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
       case t if t.hasSymbolField =>
         addSymbol(t.symbol)
         if (t.tpe != null)
-          foreachSymbolInType(t.tpe)(addSymbol)
+          foreachNotPackageSymbolInType(t.tpe)(addSymbol)
       case _ =>
     }
 
@@ -205,22 +195,8 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
       if (s.isModule) s.moduleClass else s.enclClass
   }
 
-  /**
-   * Needed for compatibility with Scala 2.8 which doesn't define `tpnme`
-   */
-  private object tpnme {
-    val EMPTY = nme.EMPTY.toTypeName
-    val EMPTY_PACKAGE_NAME = nme.EMPTY_PACKAGE_NAME.toTypeName
-  }
-
   private def eligibleAsUsedName(symbol: Symbol): Boolean = {
-    def emptyName(name: Name): Boolean = name match {
-      case nme.EMPTY | nme.EMPTY_PACKAGE_NAME | tpnme.EMPTY | tpnme.EMPTY_PACKAGE_NAME => true
-      case _ => false
-    }
-
     // Synthetic names are no longer included. See https://github.com/sbt/sbt/issues/2537
-    (symbol != NoSymbol) &&
-      !emptyName(symbol.name)
+    !ignoredSymbol(symbol) && !isEmptyName(symbol.name)
   }
 }

internal/compiler-bridge/src/main/scala/xsbt/GlobalHelpers.scala

@@ -6,16 +6,31 @@ trait GlobalHelpers {
   val global: Global
   import global._
 
-  def symbolsInType(tp: Type): Set[Symbol] = {
-    val typeSymbolCollector =
-      new CollectTypeCollector({
-        case tpe if (tpe != null) && !tpe.typeSymbolDirect.hasPackageFlag => tpe.typeSymbolDirect
-      })
+  /** Return true if type shall be ignored, false otherwise. */
+  @inline def ignoredType(tpe: Type) = {
+    tpe == null ||
+      tpe == NoType ||
+      tpe.typeSymbol == EmptyPackageClass
+  }
+
+  /** Return true if symbol shall be ignored, false otherwise. */
+  @inline def ignoredSymbol(symbol: Symbol) = {
+    symbol == null ||
+      symbol == NoSymbol ||
+      symbol == EmptyPackageClass
+  }
 
-    typeSymbolCollector.collect(tp).toSet
+  /** Return true if name is empty, false otherwise. */
+  def isEmptyName(name: Name): Boolean = {
+    name match {
+      case nme.EMPTY | nme.EMPTY_PACKAGE_NAME |
+        tpnme.EMPTY | tpnme.EMPTY_PACKAGE_NAME => true
+      case _ => false
+    }
   }
 
-  def foreachSymbolInType(tpe: Type)(op: Symbol => Unit): Unit = {
+  /** Apply `op` on every type symbol which doesn't represent a package. */
+  def foreachNotPackageSymbolInType(tpe: Type)(op: Symbol => Unit): Unit = {
     new ForEachTypeTraverser(_ match {
       case null =>
       case tpe =>
@@ -45,4 +60,21 @@ trait GlobalHelpers {
       }.headOption
     }
   }
+
+  /** Define common error messages for error reporting and assertions. */
+  object Feedback {
+    val NameHashingDisabled = "Turning off name hashing is not supported in class-based dependency trackging."
+    val OrphanTopLevelImports = noTopLevelMember("top level imports")
+    val OrphanNames = noTopLevelMember("names")
+
+    def expectedClassSymbol(culprit: Symbol): String =
+      s"The ${culprit.fullName} defined at ${culprit.fullLocationString} is not a class symbol."
+    def missingEnclosingClass(culprit: Symbol, owner: Symbol): String =
+      s"No enclosing class. Discarding dependency on $culprit (currentOwner = $owner)."
+    def noTopLevelMember(found: String) = s"""
+      |Found $found but no class, trait or object is defined in the compilation unit.
+      |The incremental compiler cannot record the dependency information in such case.
+      |Some errors like unused import referring to a non-existent class might not be reported.
+    """.stripMargin
+  }
 }

internal/compiler-interface/src/main/java/xsbti/api/SafeLazy.java

@@ -0,0 +1,58 @@
+package xsbti.api;
+
+/**
+ * Implement a Scala `lazy val` in Java for the facing sbt interface.
+ *
+ * It holds a reference to a thunk that is lazily evaluated and then
+ * its reference is clear to avoid memory leaks in memory-intensive code.
+ * It needs to be defined in [[xsbti]] or a subpackage, see
+ * [[xsbti.api.Lazy]] or [[xsbti.F0]] for similar definitions.
+ */
+public final class SafeLazy {
+
+  /* We do not use conversions from and to Scala functions because [[xsbti]]
+   * cannot hold any reference to Scala code nor the Scala library. */
+
+  /** Return a sbt [[xsbti.api.Lazy]] from a given Scala parameterless function. */
+  public static <T> xsbti.api.Lazy<T> apply(xsbti.F0<T> sbtThunk) {
+    return new Impl<T>(sbtThunk);
+  }
+
+  /** Return a sbt [[xsbti.api.Lazy]] from a strict value. */
+  public static <T> xsbti.api.Lazy<T> strict(T value) {
+    // Convert strict parameter to sbt function returning it
+    return apply(new xsbti.F0<T>() {
+      public T apply() {
+        return value;
+      }
+    });
+  }
+
+  private static final class Impl<T> extends xsbti.api.AbstractLazy<T> {
+    private xsbti.F0<T> thunk;
+    private T result;
+    private boolean flag = false;
+
+    Impl(xsbti.F0<T> thunk) {
+      this.thunk = thunk;
+    }
+
+    /**
+     * Return cached result or force lazy evaluation.
+     * 
+     * Don't call it in a multi-threaded environment.
+     */
+    public T get() {
+      if (flag) return result;
+      else {
+        result = thunk.apply();
+        flag = true;
+        // Clear reference so that thunk is GC'ed
+        thunk = null;
+        return result;
+      }
+    }
+  }
+}
+
+