Skip to content

Commit

Permalink
Include used types in the set of used names
Browse files Browse the repository at this point in the history
When `B2.scala` replaces `B.scala` in the new test
`types-in-used-names-a`, the name hash of `listb` does not change because
the signature of `C.listb` is still `List[B]`, however users of
`C.listb` have to be recompiled since the subtyping relationships of its
type have changed.

This commit does this by extending the definition of "used names" to
also include the names of the types of trees, even if these types
do not appear in the source like `List[B]` in `D.scala` (since `B` has
been invalidated, this will force the recompilation of `D.scala`).

This commit does not fix every issue with used types as illustrated by
the pending test `types-in-used-names-b`, `B.scala` is not recompiled
because it uses the type `T` whose hash has not changed, but `T` is
bounded by `S` and `S` has changed, so it should be recompiled.
This should be fixable by including the type bounds underlying a
`TypeRef` in `symbolsInType`.

The test `as-seen-from-a` that did not work before shows that we may not
have to worry about tracking prefixes in `ExtractAPI` anymore, see the
discussion in sbt#87 for more information.
  • Loading branch information
smarter committed Apr 8, 2016
1 parent 8fb6322 commit 350afa7
Show file tree
Hide file tree
Showing 29 changed files with 188 additions and 40 deletions.
24 changes: 10 additions & 14 deletions internal/compiler-bridge/src-2.10/main/scala/xsbt/Dependency.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ object Dependency {
* where it originates from. The Symbol->Classfile mapping is implemented by
* LocateClassFile that we inherit from.
*/
final class Dependency(val global: CallbackGlobal) extends LocateClassFile {
final class Dependency(val global: CallbackGlobal) extends LocateClassFile with GlobalHelpers {
import global._

def newPhase(prev: Phase): Phase = new DependencyPhase(prev)
Expand Down Expand Up @@ -156,6 +156,12 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile {
}
}

private def addTreeDependency(tree: Tree): Unit = {
addDependency(tree.symbol)
if (tree.tpe != null)
symbolsInType(tree.tpe).foreach(addDependency)
()
}
private def addDependency(dep: Symbol): Unit = {
val (fromClass, _) = resolveDependencySource
if (fromClass == NoSymbol || fromClass.hasPackageFlag) {
Expand Down Expand Up @@ -210,11 +216,11 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile {
* this looks fishy, see this thread:
* https://groups.google.com/d/topic/scala-internals/Ms9WUAtokLo/discussion
*/
case id: Ident => addDependency(id.symbol)
case id: Ident => addTreeDependency(id)
case sel @ Select(qual, _) =>
traverse(qual); addDependency(sel.symbol)
traverse(qual); addTreeDependency(sel)
case sel @ SelectFromTypeTree(qual, _) =>
traverse(qual); addDependency(sel.symbol)
traverse(qual); addTreeDependency(sel)

case Template(parents, self, body) =>
// use typeSymbol to dealias type aliases -- we want to track the dependency on the real class in the alias's RHS
Expand Down Expand Up @@ -249,16 +255,6 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile {
super.traverse(tree)
case other => super.traverse(other)
}

private def symbolsInType(tp: Type): Set[Symbol] = {
val typeSymbolCollector =
new CollectTypeCollector({
case tpe if (tpe != null) && !tpe.typeSymbolDirect.hasPackageFlag => tpe.typeSymbolDirect
})

typeSymbolCollector.collect(tp).toSet

}
}

def firstClassOrModuleDef(tree: Tree): Option[Tree] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package xsbt
*
* Extracts simple (unqualified) names mentioned in given in non-definition position by collecting
* all symbols associated with non-definition trees and extracting names from all collected symbols.
* Also extract the names of the types of non-definition trees (see source-dependencies/types-in-used-names-*
* and source-dependencies/as-seen-from-* for examples where this is required).
*
* If given symbol is mentioned both in definition and in non-definition position (e.g. in member
* selection) then that symbol is collected. It means that names of symbols defined and used in the
Expand Down Expand Up @@ -36,7 +38,7 @@ package xsbt
* The tree walking algorithm walks into TypeTree.original explicitly.
*
*/
class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) extends Compat with ClassName {
class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) extends Compat with ClassName with GlobalHelpers {
import global._

def extract(unit: CompilationUnit): Map[String, Set[String]] = {
Expand Down Expand Up @@ -96,9 +98,9 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
super.traverse(tree)
}

private def addSymbol(symbol: Symbol): Unit = {
addName(symbol.name)
}
private def addSymbol(symbol: Symbol): Unit =
if (eligibleAsUsedName(symbol))
addName(symbol.name)

private def addName(name: Name, enclosingNonLocalClass: Symbol = resolveEnclosingNonLocalClass): Unit = {
val nameAsString = name.decode.trim
Expand Down Expand Up @@ -135,8 +137,10 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
// not what we need
case t: TypeTree if t.original != null =>
t.original.foreach(traverse)
case t if t.hasSymbol && eligibleAsUsedName(t.symbol) =>
case t if t.hasSymbol =>
addSymbol(t.symbol)
if (t.tpe != null)
symbolsInType(t.tpe).foreach(addSymbol)
case _ =>
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package xsbt

import scala.tools.nsc.Global

trait GlobalHelpers {
val global: CallbackGlobal
import global._

def symbolsInType(tp: Type): Set[Symbol] = {
val typeSymbolCollector =
new CollectTypeCollector({
case tpe if (tpe != null) && !tpe.typeSymbolDirect.hasPackageFlag => tpe.typeSymbolDirect
})

typeSymbolCollector.collect(tp).toSet
}
}
22 changes: 9 additions & 13 deletions internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,12 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
}
}

private def addTreeDependency(tree: Tree): Unit = {
addDependency(tree.symbol)
if (tree.tpe != null)
symbolsInType(tree.tpe).foreach(addDependency)
()
}
private def addDependency(dep: Symbol): Unit = {
val (fromClass, _) = resolveDependencySource
if (fromClass == NoSymbol || fromClass.hasPackageFlag) {
Expand Down Expand Up @@ -210,11 +216,11 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
* this looks fishy, see this thread:
* https://groups.google.com/d/topic/scala-internals/Ms9WUAtokLo/discussion
*/
case id: Ident => addDependency(id.symbol)
case id: Ident => addTreeDependency(id)
case sel @ Select(qual, _) =>
traverse(qual); addDependency(sel.symbol)
traverse(qual); addTreeDependency(sel)
case sel @ SelectFromTypeTree(qual, _) =>
traverse(qual); addDependency(sel.symbol)
traverse(qual); addTreeDependency(sel)

case Template(parents, self, body) =>
// use typeSymbol to dealias type aliases -- we want to track the dependency on the real class in the alias's RHS
Expand Down Expand Up @@ -249,16 +255,6 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
super.traverse(tree)
case other => super.traverse(other)
}

private def symbolsInType(tp: Type): Set[Symbol] = {
val typeSymbolCollector =
new CollectTypeCollector({
case tpe if (tpe != null) && !tpe.typeSymbolDirect.hasPackageFlag => tpe.typeSymbolDirect
})

typeSymbolCollector.collect(tp).toSet

}
}

def firstClassOrModuleDef(tree: Tree): Option[Tree] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package xsbt
*
* Extracts simple (unqualified) names mentioned in given in non-definition position by collecting
* all symbols associated with non-definition trees and extracting names from all collected symbols.
* Also extract the names of the types of non-definition trees (see source-dependencies/types-in-used-names-*
* and source-dependencies/as-seen-from-* for examples where this is required).
*
* If given symbol is mentioned both in definition and in non-definition position (e.g. in member
* selection) then that symbol is collected. It means that names of symbols defined and used in the
Expand Down Expand Up @@ -96,9 +98,9 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
super.traverse(tree)
}

private def addSymbol(symbol: Symbol): Unit = {
addName(symbol.name)
}
private def addSymbol(symbol: Symbol): Unit =
if (eligibleAsUsedName(symbol))
addName(symbol.name)

private def addName(name: Name, enclosingNonLocalClass: Symbol = resolveEnclosingNonLocalClass): Unit = {
val nameAsString = name.decode.trim
Expand Down Expand Up @@ -137,8 +139,10 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
// not what we need
case t: TypeTree if t.original != null =>
t.original.foreach(traverse)
case t if t.hasSymbolField && eligibleAsUsedName(t.symbol) =>
case t if t.hasSymbolField =>
addSymbol(t.symbol)
if (t.tpe != null)
symbolsInType(t.tpe).foreach(addSymbol)
case _ =>
}

Expand Down
11 changes: 10 additions & 1 deletion internal/compiler-bridge/src/main/scala/xsbt/GlobalHelpers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,16 @@ import scala.tools.nsc.Global

trait GlobalHelpers {
val global: Global
import global.{ analyzer, Tree }
import global._

def symbolsInType(tp: Type): Set[Symbol] = {
val typeSymbolCollector =
new CollectTypeCollector({
case tpe if (tpe != null) && !tpe.typeSymbolDirect.hasPackageFlag => tpe.typeSymbolDirect
})

typeSymbolCollector.collect(tp).toSet
}

object MacroExpansionOf {
def unapply(tree: Tree): Option[Tree] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,62 @@ class ExtractUsedNamesSpecification extends UnitSpec {
|}""".stripMargin
val compilerForTesting = new ScalaCompilerForUnitTesting(nameHashing = true)
val usedNames = compilerForTesting.extractUsedNamesFromSrc(srcA, srcB)
val expectedNames = standardNames ++ Set("A", "a", "B", "=")
val expectedNames = standardNames ++ Set("A", "a", "B", "=", "Int")
assert(usedNames("B") === expectedNames)
}

// See source-dependencies/types-in-used-names-a for an example where
// this is required.
it should "extract names in the types of trees" in {
val src1 = """|class X0
|class X1 extends X0
|class Y
|class A {
| type T >: X1 <: X0
|}
|class M
|class N
|class P0
|class P1 extends P0
|object B {
| type S = Y
| val lista: List[A] = ???
| val at: A#T = ???
| val as: S = ???
| def foo(m: M): N = ???
| def bar[Param >: P1 <: P0](p: Param): Param = ???
|}""".stripMargin
val src2 = """|object Test_lista {
| val x = B.lista
|}
|object Test_at {
| val x = B.at
|}
|object Test_as {
| val x = B.as
|}
|object Test_foo {
| val x = B.foo(???)
|}
|object Test_bar {
| val x = B.bar(???)
|}""".stripMargin
val compilerForTesting = new ScalaCompilerForUnitTesting(nameHashing = true)
val usedNames = compilerForTesting.extractUsedNamesFromSrc(src1, src2)
val expectedNames_lista = standardNames ++ Set("Test_lista", "x", "B", "lista", "package", "List", "A")
val expectedNames_at = standardNames ++ Set("Test_at", "x", "B", "at", "A", "T")
val expectedNames_as = standardNames ++ Set("Test_as", "x", "B", "as", "S")
val expectedNames_foo = standardNames ++ Set("Test_foo", "x", "B", "foo", "M", "N",
"Predef", "???", "Nothing")
val expectedNames_bar = standardNames ++ Set("Test_bar", "x", "B", "bar", "Param", "P1", "P0",
"Predef", "???", "Nothing")
assert(usedNames("Test_lista") === expectedNames_lista)
assert(usedNames("Test_at") === expectedNames_at)
assert(usedNames("Test_as") === expectedNames_as)
assert(usedNames("Test_foo") === expectedNames_foo)
assert(usedNames("Test_bar") === expectedNames_bar)
}

// test for https://github.com/gkossakowski/sbt/issues/3
it should "extract used names from the same compilation unit" in {
val src = "class A { def foo: Int = 0; def bar: Int = foo }"
Expand Down Expand Up @@ -88,8 +140,9 @@ class ExtractUsedNamesSpecification extends UnitSpec {
* definition.
*/
private val standardNames = Set(
// AnyRef is added as default parent of a class
"scala", "AnyRef",
"scala",
// The default parent of a class is "AnyRef" which is an alias for "Object"
"AnyRef", "Object",
// class receives a default constructor which is internally called "<init>"
"<init>"
)
Expand Down
6 changes: 6 additions & 0 deletions zinc/src/sbt-test/source-dependencies/as-seen-from-a/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
abstract class A {
type T
object X {
def foo(x: T): T = x
}
}
3 changes: 3 additions & 0 deletions zinc/src/sbt-test/source-dependencies/as-seen-from-a/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class B extends A {
type T = Int
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
object C extends B
3 changes: 3 additions & 0 deletions zinc/src/sbt-test/source-dependencies/as-seen-from-a/D.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object D {
C.X.foo(12)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class B extends A {
type T = String
}
3 changes: 3 additions & 0 deletions zinc/src/sbt-test/source-dependencies/as-seen-from-a/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
> compile
$ copy-file changes/B2.scala B.scala
-> compile
7 changes: 7 additions & 0 deletions zinc/src/sbt-test/source-dependencies/as-seen-from-b/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
abstract class A {
type T <: S
type S
object X {
def foo: T = null.asInstanceOf[T]
}
}
3 changes: 3 additions & 0 deletions zinc/src/sbt-test/source-dependencies/as-seen-from-b/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class B extends A {
type S <: Int
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
object C extends B
3 changes: 3 additions & 0 deletions zinc/src/sbt-test/source-dependencies/as-seen-from-b/D.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object D {
val x: Int = C.X.foo
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class B extends A {
type S <: String
}
3 changes: 3 additions & 0 deletions zinc/src/sbt-test/source-dependencies/as-seen-from-b/pending
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
> compile
$ copy-file changes/B2.scala B.scala
-> compile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class A
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class B extends A
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object C {
val listb: List[B] = List(new B)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object D {
val lista: List[A] = C.listb
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class B
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
> compile
$ copy-file changes/B2.scala B.scala
# Compilation of D.scala should fail because B is no longer a subtype of A
-> compile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class A {
type T <: S
type S <: Int
def foo: T = null.asInstanceOf[T]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object B {
val x: Int = (new A).foo
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class A {
type T <: S
type S <: String
def foo: T = null.asInstanceOf[T]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
> compile
$ copy-file changes/A2.scala A.scala
# Compilation of B.scala should fail because A#S is no longer a subtype of Int
-> compile

0 comments on commit 350afa7

Please sign in to comment.