Skip to content

Commit

Permalink
Fix mismatch between apply methods and initializers
Browse files Browse the repository at this point in the history
When a case class is defined with no companion, the companion is
synthesized by the compiler and every object creation of that case class
is proxied to the synthesized apply method of the newly synthesized
companion. From this perspective, if we have a `case class A(x: Int)`
and a use site `A(1)`, `ExtractUsedNames` will extract the name to the
apply method in `A(1)` and mark it as used.

However, this is wrong. When the user changes the signature of the case
class, only the signature of the case class constructor changes and this
change is not propagated to the apply signature (`ExtractAPI` traverses
trees, and the synthesized module has no trees as it is added in namer).
Therefore, when we compare changed names in the old and new API, Zinc
concludes that only references to `A;<init>;` must be recompiled and
since the use site that contained `A(1)` had no such reference, then
it's ignored.

To fix this problem, we protect ourselves from this point of indirection
and extract the proper name of the case class constructor iff the
companion case class is indeed synthesized by the compiler. Note that
when the user defines `object A` alongside the definition of `A`, Zinc
does the right thing.

Note that this fixes sbt/sbt#4316 and also
fixes issues with default parameters in case classes.
  • Loading branch information
jvican committed Aug 14, 2018
1 parent c8ee193 commit 4463be9
Show file tree
Hide file tree
Showing 12 changed files with 75 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,19 @@ class ExtractUsedNamesPerformanceSpecification extends UnitSpec {
}
// format: off
val expectedNamesForTupler = Set("java;lang;Object;init;", "Object", "scala", "tupler", "TuplerInstances", "DepFn1", "HNil", "$anon", "Out", "Out0", "Tupler", "acme;Tupler;$anon;init;", "hnilTupler", "acme", "L", "Aux", "HList", "Serializable", "Unit")
val expectedNamesForTuplerInstances = Set("E", "Tuple4", "e", "case7", "Tuple15", "s", "case19", "T7", "x", "TuplerInstances", "matchEnd19", "T20", "Tuple11", "HNil", "matchEnd6", "p16", "$anon", "T19", "p20", "T2", "p10", "case22", "p19", "n", "Tuple12", "case11", "Tuple22", "p12", "matchEnd7", "N", "p4", "T13", "case26", "Tuple19", "p7", "p5", "j", "Out", "T", "p23", "case15", "matchEnd20", "t", "p21", "matchEnd15", "J", "head", "case13", "u", "matchEnd18", "U", "Tupler", "f", "T8", "T16", "F", "Tuple3", "case8", "case18", "case24", "Boolean", "matchEnd21", "A", "matchEnd26", "a", "Tuple14", "T1", "::", "Nothing", "p18", "case20", "m", "matchEnd10", "M", "matchEnd25", "tail", "Tuple2", "matchEnd5", "p15", "matchEnd23", "I", "i", "matchEnd14", "AnyRef", "Tuple8", "matchEnd8", "case25", "T12", "p3", "case14", "case23", "T5", "matchEnd22", "T17", "v", "p22", "Tuple18", "G", "Tuple13", "matchEnd12", "scala;MatchError;init;", "acme;TuplerInstances;$anon;init;", "java;lang;Object;init;", "V", "q", "p11", "Q", "case12", "L", "b", "apply", "Object", "g", "B", "l", "==", "Out0", "Tuple1", "matchEnd9", "P", "p2", "T15", "Aux", "matchEnd24", "p", "scala", "matchEnd11", "Tuple20", "HList", "case17", "T9", "p14", "Tuple7", "matchEnd17", "T4", "case28", "T22", "p17", "C", "Tuple6", "MatchError", "T11", "x1", "H", "case16", "matchEnd13", "c", "Tuple9", "h", "T6", "T18", "r", "K", "Tuple17", "p9", "R", "ne", "T14", "case21", "k", "case10", "Tuple21", "O", "case9", "Tuple10", "Any", "T10", "case27", "Tuple5", "D", "p13", "o", "p6", "p8", "matchEnd16", "S", "T21", "Tuple16", "d", "T3")
val expectedNamesForTuplerInstances = Set("E", "Tuple4", "scala;Tuple4;init;", "e", "case7", "Tuple15", "scala;Tuple15;init;", "s", "case19", "T7", "x", "TuplerInstances", "matchEnd19", "T20", "Tuple11", "scala;Tuple15;init;","HNil", "matchEnd6", "p16", "$anon", "T19", "p20", "T2", "p10", "case22", "p19", "n", "Tuple12", "scala;Tuple15;init;","case11", "Tuple22", "scala;Tuple15;init;","p12", "matchEnd7", "N", "p4", "T13", "case26", "Tuple19", "p7", "p5", "j", "Out", "T", "p23", "case15", "matchEnd20", "t", "p21", "matchEnd15", "J", "head", "case13", "u", "matchEnd18", "U", "Tupler", "f", "T8", "T16", "F", "Tuple3", "scala;Tuple15;init;", "case8", "case18", "case24", "Boolean", "matchEnd21", "A", "matchEnd26", "a", "Tuple14", "scala;Tuple15;init;","T1", "::", "Nothing", "p18", "case20", "m", "matchEnd10", "M", "matchEnd25", "tail", "Tuple2", "scala;Tuple15;init;","matchEnd5", "p15", "matchEnd23", "I", "i", "matchEnd14", "AnyRef", "Tuple8", "scala;Tuple15;init;", "matchEnd8", "case25", "T12", "p3", "case14", "case23", "T5", "matchEnd22", "T17", "v", "p22", "Tuple18", "G", "Tuple13", "matchEnd12", "scala;MatchError;init;", "acme;TuplerInstances;$anon;init;", "java;lang;Object;init;", "V", "q", "p11", "Q", "case12", "L", "b", "Object", "g", "B", "l", "==", "Out0", "Tuple1", "matchEnd9", "P", "p2", "T15", "Aux", "matchEnd24", "p", "scala", "matchEnd11", "Tuple20", "HList", "case17", "T9", "p14", "Tuple7", "matchEnd17", "T4", "case28", "T22", "p17", "C", "Tuple6", "MatchError", "T11", "x1", "H", "case16", "matchEnd13", "c", "Tuple9", "h", "T6", "T18", "r", "K", "Tuple17", "p9", "R", "ne", "T14", "case21", "k", "case10", "Tuple21", "O", "case9", "Tuple10", "Any", "T10", "case27", "Tuple5", "D", "p13", "o", "p6", "p8", "matchEnd16", "S", "T21", "Tuple16", "d", "T3")
val expectedTupleInits = Set("scala;Tuple1;init;", "scala;Tuple2;init;", "scala;Tuple3;init;", "scala;Tuple5;init;", "scala;Tuple6;init;", "scala;Tuple7;init;", "scala;Tuple8;init;", "scala;Tuple9;init;", "scala;Tuple10;init;", "scala;Tuple11;init;", "scala;Tuple12;init;", "scala;Tuple13;init;", "scala;Tuple14;init;", "scala;Tuple16;init;", "scala;Tuple17;init;", "scala;Tuple18;init;", "scala;Tuple19;init;", "scala;Tuple20;init;", "scala;Tuple21;init;", "scala;Tuple22;init;")
val expectedNamesForRefinement = Set("Out0")
val `expectedNamesFor::` = Set("x", "T2", "ScalaRunTime", "Iterator", "T", "head", "asInstanceOf", "Boolean", "A", "$" + "isInstanceOf", "T1", "||", "acme;::;init;", "::", "Nothing", "x$1", "any2stringadd", "acme", "typedProductIterator", "tail", "Tuple2", "AnyRef", "isInstanceOf", "Int", "java;lang;Object;init;", "_hashCode", "apply", "Object", "x$0", "==", "Some", "IndexOutOfBoundsException", "java;lang;IndexOutOfBoundsException;init;", "T0", "Predef", "scala", "matchEnd4", "HList", "None", "x1", "toString", "H", "+", "&&", "Serializable", "Product", "case6", "::$1", "eq", "Any", "runtime", "String")
val `expectedNamesFor::` = Set("x", "T2", "ScalaRunTime", "Iterator", "T", "head", "asInstanceOf", "Boolean", "A", "$" + "isInstanceOf", "T1", "||", "acme;::;init;", "::", "Nothing", "x$1", "any2stringadd", "acme", "typedProductIterator", "tail", "Tuple2", "AnyRef", "isInstanceOf", "Int", "java;lang;Object;init;", "_hashCode", "Object", "x$0", "==", "Some", "IndexOutOfBoundsException", "java;lang;IndexOutOfBoundsException;init;", "T0", "Predef", "scala", "matchEnd4", "HList", "None", "x1", "toString", "H", "+", "&&", "Serializable", "Product", "case6", "::$1", "eq", "Any", "runtime", "String", "scala;Some;init;", "scala;Tuple2;init;")
val expectedNamesForDepFn1 = Set("DepFn1", "Out", "T", "AnyRef", "Object", "scala")
val expectedNamesForHNil = Set("x", "HNil", "ScalaRunTime", "Iterator", "Boolean", "A", "T", "$" + "isInstanceOf", "::", "Nothing", "x$1", "acme", "typedProductIterator", "Int", "java;lang;Object;init;", "apply", "Object", "IndexOutOfBoundsException", "java;lang;IndexOutOfBoundsException;init;", "scala", "HList", "toString", "H", "Serializable", "h", "Product", "Any", "runtime", "matchEnd3", "String", "T0")
val expectedNamesForHNil = Set("x", "HNil", "ScalaRunTime", "Iterator", "Boolean", "A", "T", "$" + "isInstanceOf", "::", "Nothing", "x$1", "acme", "typedProductIterator", "Int", "java;lang;Object;init;", "acme;::;init;", "Object", "IndexOutOfBoundsException", "java;lang;IndexOutOfBoundsException;init;", "scala", "HList", "toString", "H", "Serializable", "h", "Product", "Any", "runtime", "matchEnd3", "String", "T0")
val expectedNamesForHList = Set("Tupler", "acme", "scala", "Serializable", "Product")
// format: on
assert(usedNames("acme.Tupler") -- scalaDiff === expectedNamesForTupler -- scalaDiff)
println((usedNames("acme.TuplerInstances") -- scalaDiff).diff((expectedNamesForTuplerInstances ++ expectedTupleInits) -- scalaDiff))
println((expectedNamesForTuplerInstances ++ expectedTupleInits -- scalaDiff).diff(usedNames("acme.TuplerInstances") -- scalaDiff))
assert(
usedNames("acme.TuplerInstances") -- scalaDiff === expectedNamesForTuplerInstances -- scalaDiff)
usedNames("acme.TuplerInstances") -- scalaDiff === (expectedNamesForTuplerInstances ++ expectedTupleInits) -- scalaDiff)
assert(
usedNames("acme.TuplerInstances.<refinement>") -- scalaDiff === expectedNamesForRefinement -- scalaDiff)
assert(usedNames("acme.$colon$colon") -- scalaDiff === `expectedNamesFor::` -- scalaDiff)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,29 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType)

case t if t.hasSymbolField =>
val symbol = t.symbol
if (symbol != rootMirror.RootPackage)
addSymbol(getNamesOfEnclosingScope, t.symbol)
if (symbol != rootMirror.RootPackage) {
/* When a case class is defined with no companion, the companion is synthesized by the
* compiler and every object creation of that case class is proxied to the synthesized
* apply method of the newly synthesized companion. From this perspective, if we have
* a `case class A(x: Int)` and a use site `A(1)`, `ExtractUsedNames` will extract the
* name to the apply method in `A(1)` and mark it as used.
*
* However, this is wrong. When the user changes the signature of the case class, only
* the signature of the case class constructor changes and this change is not propagated
* to the apply signature (`ExtractAPI` traverses trees, and the synthesized module has
* no trees as it is added in namer). Therefore, when we compare changed names in the
* old and new API, Zinc concludes that only references to `A;<init>;` must be recompiled
* and since the use site that contained `A(1)` had no such reference, then it's ignored.
*
* To fix this problem, we protect ourselves from this point of indirection and extract
* the proper name of the case class constructor iff the companion case class is indeed
* synthesized by the compiler. Note that when the user defines `object A` alongside the
* definition of `A`, Zinc does the right thing.
*/
if (symbol.isCaseApplyOrUnapply && symbol.name == nme.apply && symbol.owner.isSynthetic) {
addSymbol(getNamesOfEnclosingScope, symbol.owner.companionClass.primaryConstructor)
} else addSymbol(getNamesOfEnclosingScope, t.symbol)
}

val tpe = t.tpe
if (!ignoredType(tpe)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
case class A (
name: String
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object UseSite extends App {
println("hello world")
val y = A("5")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
case class A(
name: Int
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
case class A(
name: Int,
ms: Long = 0L
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
case class A(
name: Int,
ms: String = ""
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
case class A(
name: Int,
ms: Char = 'c'
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object UseSite extends App {
println("hello world")
val y = A(5)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object UseSite extends App {
println("hello world")
val y = A(5, "asdf")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
apiDebug = true
relationsDebug = true
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
> compile
$ copy-file changes/A.scala A.scala
-> compile
$ copy-file changes/UseSite.scala UseSite.scala
> run
$ copy-file changes/A2.scala A.scala
> run
$ copy-file changes/A3.scala A.scala
> run
$ copy-file changes/UseSite2.scala UseSite.scala
> run
$ copy-file changes/A4.scala A.scala
-> compile

0 comments on commit 4463be9

Please sign in to comment.