Skip to content

Commit

Permalink
Fix #97: Avoid spurious recompilations when unrelated constructor cha…
Browse files Browse the repository at this point in the history
…nges

The name hashing algorithm is designed to take implicit conversions into
account: when a name "foo" is changed somewhere in a dependency of X,
you have to recompile X if it uses the name "foo", even if the usage of
"foo" in X is completely unrelated, just because this might have an
effect on available implicit conversions. However, there is one case
where we can be sure that implicit conversions will not kick in: when we
call a constructor. A constructor name is always "<init>", this PR now
replaces this name by "pkgA;pkgB;className;init;", this mean that we no
longer recompile classes when an unrelated constructor in a used class
changed (see the new test `constructors-unrelated` for an example).
  • Loading branch information
smarter authored and jvican committed May 5, 2017
1 parent a461b99 commit 60d7978
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 395 deletions.
31 changes: 31 additions & 0 deletions internal/compiler-bridge/src/main/scala/xsbt/ClassName.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,37 @@ trait ClassName extends Compat {
*/
protected def classNameAsString(s: Symbol): String = pickledNameAsString(s)

/**
* Given a class symbol `cls`, construct a name representing this constructor.
* For a class:
*
* a.b.Foo
*
* this is:
*
* a;b;Foo;init;
*
* The prefix is important to avoid name hashing all constructors together
* (see #97), the weird format is necessary to avoid scalac or zinc trying to
* interpret this name (in particular we should not use '.' and we should not
* use '<init>'), we use ';' because it is one of the few characters that
* cannot appear in a valid JVM name.
*/
protected def constructorName(cls: Symbol): Name =
newTermName(constructorNameAsString(cls))

protected def constructorNameAsString(cls: Symbol): String =
cls.fullName(';') ++ ";init;"

/**
* Mangle a JVM symbol name in a format better suited for internal uses by sbt.
*/
protected def mangledName(s: Symbol): Name =
if (s.name == nme.CONSTRUCTOR)
constructorName(s.enclClass)
else
s.name

/**
* Create a (source) name for the class symbol `s` with a prefix determined by the class symbol `in`.
*
Expand Down
4 changes: 2 additions & 2 deletions internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala
Original file line number Diff line number Diff line change
Expand Up @@ -691,8 +691,8 @@ class ExtractAPI[GlobalType <: Global](

private def simpleName(s: Symbol): String = {
val n = s.unexpandedName
val n2 = if (n.toString == "<init>") n else n.decode
n2.toString.trim
val n2 = if (n == nme.CONSTRUCTOR) constructorNameAsString(s.enclClass) else n.decode.toString
n2.trim
}

private def staticAnnotations(annotations: List[AnnotationInfo]): List[AnnotationInfo] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,9 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType)
}

val addSymbol: (JavaSet[Name], Symbol) => Unit = { (names: JavaSet[Name], symbol: Symbol) =>
if (!ignoredSymbol(symbol)) {
val name = symbol.name
// Synthetic names are no longer included. See https://github.com/sbt/sbt/issues/2537
if (!isEmptyName(name))
names.add(name)
// Synthetic names are no longer included. See https://github.com/sbt/sbt/issues/2537
if (!ignoredSymbol(symbol) && !isEmptyName(symbol.name)) {
names.add(mangledName(symbol))
()
}
}
Expand Down Expand Up @@ -209,7 +207,7 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType)
private object PatMatDependencyTraverser extends TypeDependencyTraverser {
override def addDependency(symbol: global.Symbol): Unit = {
if (!ignoredSymbol(symbol) && symbol.isSealed) {
val name = symbol.name
val name = mangledName(symbol)
if (!isEmptyName(name)) {
val existingScopes = _currentScopedNamesCache.get(name)
if (existingScopes == null)
Expand Down
Loading

0 comments on commit 60d7978

Please sign in to comment.