Skip to content

Commit

Permalink
Fix undercompilation upon ctor change (scala#19911)
Browse files Browse the repository at this point in the history
Fixes scala#19910
Fixes sbt/zinc#1334

## Problem
Scala 3 compiler registers special `zincMangledName` for constructors
for the purpose of incremental compilation. Currently the
`zincMangledName` contains the package name, which does not match the
use site tracking,
thereby causing undercompilation during incremental compilation after a
ctor change, like adding a parameter.

There is an existing scripted test, but coincidentally the test class
does NOT include packages, so the test ends up passing.

## Solution
1. This PR reproduces the issue by adding package name to the test.
2. This also fixes the problem by changing the `zincMangedName` to
`sym.owner.name ++ ";init;"`.

## Note about zincMangedName
`zincMangedName` in general is a good idea, which adds the class name
into otherwise common name `<init>`.
By making the name more unique it prevents overcompilation when one of
the ctors changes in a file.
  • Loading branch information
bishabosha authored Mar 17, 2024
2 parents 9ef24bf + 0f7de67 commit b782cbf
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 8 deletions.
20 changes: 13 additions & 7 deletions compiler/src/dotty/tools/dotc/sbt/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,23 @@ import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.Symbols.Symbol
import dotty.tools.dotc.core.NameOps.stripModuleClassSuffix
import dotty.tools.dotc.core.Names.Name
import dotty.tools.dotc.core.Names.termName

inline val TermNameHash = 1987 // 300th prime
inline val TypeNameHash = 1993 // 301st prime
inline val InlineParamHash = 1997 // 302nd prime

extension (sym: Symbol)

def constructorName(using Context) =
sym.owner.fullName ++ ";init;"

/** Mangle a JVM symbol name in a format better suited for internal uses by sbt. */
def zincMangledName(using Context): Name =
if (sym.isConstructor) constructorName
else sym.name.stripModuleClassSuffix
/** Mangle a JVM symbol name in a format better suited for internal uses by sbt.
* WARNING: output must not be written to TASTy, as it is not a valid TASTy name.
*/
private[sbt] def zincMangledName(using Context): Name =
if sym.isConstructor then
// TODO: ideally we should avoid unnecessarily caching these Zinc specific
// names in the global chars array. But we would need to restructure
// ExtractDependencies caches to avoid expensive `toString` on
// each member reference.
termName(sym.owner.fullName.mangledString.replace(".", ";").nn ++ ";init;")
else
sym.name.stripModuleClassSuffix
2 changes: 1 addition & 1 deletion sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ class ExtractUsedNamesSpecification {
// All classes extend Object
"Object",
// All classes have a default constructor called <init>
"java.lang.Object;init;",
"java;lang;Object;init;",
// the return type of the default constructor is Unit
"Unit"
)
Expand Down
2 changes: 2 additions & 0 deletions sbt-test/source-dependencies/constructors/A.scala
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
package example

class A(a: Int)
2 changes: 2 additions & 0 deletions sbt-test/source-dependencies/constructors/B.scala
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
package example

class B { val y = new A(2) }
2 changes: 2 additions & 0 deletions sbt-test/source-dependencies/constructors/changes/A2.scala
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
package example

class A(a: String)
2 changes: 2 additions & 0 deletions sbt-test/source-dependencies/constructors/changes/B2.scala
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
package example

class B { val y = new A("a") }

0 comments on commit b782cbf

Please sign in to comment.