Skip to content

Commit

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

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 aca8dfa
Show file tree
Hide file tree
Showing 11 changed files with 66 additions and 394 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 aca8dfa

Please sign in to comment.