From 69aeb11d1707c65d93fba9384326b05a66818a99 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Sat, 9 Mar 2024 18:48:10 -0500 Subject: [PATCH 1/2] Fix undercompilation upon ctor change **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** This PR reproduces the issue by adding package name to the test. This also fixes the problem by changing the `zincMangedName` to `sym.owner.name ++ ";init;"`. [Cherry-picked 157ed43c74c72cf31db4da3889befd891bc65041] --- compiler/src/dotty/tools/dotc/sbt/package.scala | 2 +- sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala | 2 +- sbt-test/source-dependencies/constructors/A.scala | 2 ++ sbt-test/source-dependencies/constructors/B.scala | 2 ++ sbt-test/source-dependencies/constructors/changes/A2.scala | 2 ++ sbt-test/source-dependencies/constructors/changes/B2.scala | 2 ++ 6 files changed, 10 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/sbt/package.scala b/compiler/src/dotty/tools/dotc/sbt/package.scala index 379a2e45ce40..7c24319005ed 100644 --- a/compiler/src/dotty/tools/dotc/sbt/package.scala +++ b/compiler/src/dotty/tools/dotc/sbt/package.scala @@ -12,7 +12,7 @@ inline val InlineParamHash = 1997 // 302nd prime extension (sym: Symbol) def constructorName(using Context) = - sym.owner.fullName ++ ";init;" + sym.owner.name ++ ";init;" /** Mangle a JVM symbol name in a format better suited for internal uses by sbt. */ def zincMangledName(using Context): Name = diff --git a/sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala b/sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala index 2b2b7d26c716..d6cc3ac6339d 100644 --- a/sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala +++ b/sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala @@ -306,7 +306,7 @@ class ExtractUsedNamesSpecification { // All classes extend Object "Object", // All classes have a default constructor called - "java.lang.Object;init;", + "Object;init;", // the return type of the default constructor is Unit "Unit" ) diff --git a/sbt-test/source-dependencies/constructors/A.scala b/sbt-test/source-dependencies/constructors/A.scala index b81025bada4d..0c5a5efa5b25 100644 --- a/sbt-test/source-dependencies/constructors/A.scala +++ b/sbt-test/source-dependencies/constructors/A.scala @@ -1 +1,3 @@ +package example + class A(a: Int) diff --git a/sbt-test/source-dependencies/constructors/B.scala b/sbt-test/source-dependencies/constructors/B.scala index e44b1d4c7852..b66f04320f13 100644 --- a/sbt-test/source-dependencies/constructors/B.scala +++ b/sbt-test/source-dependencies/constructors/B.scala @@ -1 +1,3 @@ +package example + class B { val y = new A(2) } diff --git a/sbt-test/source-dependencies/constructors/changes/A2.scala b/sbt-test/source-dependencies/constructors/changes/A2.scala index edd9e160e7bf..5f72892e6f26 100644 --- a/sbt-test/source-dependencies/constructors/changes/A2.scala +++ b/sbt-test/source-dependencies/constructors/changes/A2.scala @@ -1 +1,3 @@ +package example + class A(a: String) diff --git a/sbt-test/source-dependencies/constructors/changes/B2.scala b/sbt-test/source-dependencies/constructors/changes/B2.scala index 701f0514685f..7b1399a8ac49 100644 --- a/sbt-test/source-dependencies/constructors/changes/B2.scala +++ b/sbt-test/source-dependencies/constructors/changes/B2.scala @@ -1 +1,3 @@ +package example + class B { val y = new A("a") } From 49e7b4c8f512a21d4fdef890f13b50965324af9a Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Wed, 13 Mar 2024 16:17:04 +0100 Subject: [PATCH 2/2] Partial revert of previous commit. Instead of avoiding fully qualified names, use a different separator in zincMangledName. [Cherry-picked 0f7de67245a8519842f2ef25b985fe967a24d561] --- .../src/dotty/tools/dotc/sbt/package.scala | 20 ++++++++++++------- .../xsbt/ExtractUsedNamesSpecification.scala | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/sbt/package.scala b/compiler/src/dotty/tools/dotc/sbt/package.scala index 7c24319005ed..dc0df381f08f 100644 --- a/compiler/src/dotty/tools/dotc/sbt/package.scala +++ b/compiler/src/dotty/tools/dotc/sbt/package.scala @@ -4,6 +4,7 @@ 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 @@ -11,10 +12,15 @@ inline val InlineParamHash = 1997 // 302nd prime extension (sym: Symbol) - def constructorName(using Context) = - sym.owner.name ++ ";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 diff --git a/sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala b/sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala index d6cc3ac6339d..e47371175de6 100644 --- a/sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala +++ b/sbt-bridge/test/xsbt/ExtractUsedNamesSpecification.scala @@ -306,7 +306,7 @@ class ExtractUsedNamesSpecification { // All classes extend Object "Object", // All classes have a default constructor called - "Object;init;", + "java;lang;Object;init;", // the return type of the default constructor is Unit "Unit" )