-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix #9970: Reconcile how Scala 3 and 2 decide whether a trait has $in…
…it$. Scala 2 uses a very simple mechanism to detect traits that have or don't have a `$init$` method: * At parsing time, a `$init$` method is created if and only if there at least one member in the body that "requires" it. * `$init$` are otherwise never created nor removed. * A call to `T.$init$` in a subclass `C` is generated if and only if `T.$init$` exists. Scala 3 has a flags-based approach to the above: * At parsing time, a `<init>` method is always created for traits. * The `NoInits` flag is added if the body does not contain any member that "requires" an initialization. * In addition, the constructor receives the `StableRealizable` flag if the trait *and all its base classes/traits* have the `NoInits` flag. This is then used for purity analysis in the inliner. * The `Constructors` phase creates a `$init$` method for traits that do not have the `NoInits` flag, and generates calls based on the same criteria. Now, one might think that this is all fine, except it isn't when we mix Scala 2 and 3, and in particular when a Scala 2 class extend a Scala 3 trait. Indeed, since Scala 3 *always* defines a `<init>` method in traits, which Scala 2 translates as `$init$`, Scala 2 would think that it always needs to emit a call to `$init$` (even for traits where Scala 3 does not, in fact, emit a `$init$` method in the end). This was mitigated in the TASTy reader of Scala 2 by removing `$init$` if it has the `STABLE` flag (coming from `StableRealizable`). This would have been fine if `StableRealizable` was present if and only if the owner trait has `NoInits`. But until this commit, that was not the case: a trait without initialization in itself, but extending a trait with initialization code, would have the flag `NoInits` but its constructor would not have the `StableRealizable` flag. Therefore, this commit basically reconciles the `NoInits` and `StableRealizable` flags, so that Scala 2 understands whether or not a Scala 3 trait has a `$init$` method. We also align those flags when reading from Scala 2 traits, so that Scala 3 also understands whether or not a Scala 2 trait has a `$init$` trait. This solves the compatibility issue between Scala 2 and Scala 3. One detail remains. The attentive reader may have noticed the quotes in 'an element of the body that "requires" initialization'. Scala 2 and Scala 3 do not agree on what requires initialization: notably, Scala 2 thinks that a concrete `def` requires initialization, whereas Scala 3 knows that this is not the case. This is not an issue for synchronous interoperability between Scala 2 and 3, since each decides on its own which of its traits has a `$init$` method, and communicates that fact to the other version. However, this still poses an issue for "diachronic" compatibility: if a library defines a trait with a concrete `def` and is compiled by Scala 2 in version v1, that trait will have a `$init$`. When the library upgrades to Scala 3 in version v2, the trait will *lose* the `$init$`, which can break third-party subclasses. This commit does not address this issue. There are two ways we could do so: * Precisely align the detection of whether a trait should have a `$init$` method with Scala 2 (notably, adding one when there is a concrete `def`), or * *Always* emit an empty static `$init$` method, even for traits that have the `NoInits` flag (but do not generate calls to them, nor have that affect whether or not subclasses are considered pure).
- Loading branch information
Showing
8 changed files
with
128 additions
and
23 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| import scala.quoted._ | ||
| import scala.tasty.inspector._ | ||
|
|
||
| /* Test that the constructor of a trait has the StableRealizable trait if and | ||
| * only if the trait has NoInits. This is used by the TASTy reader in Scala 2 | ||
| * to determine whether the constructor should be kept or discarded, and | ||
| * consequently whether calls to $init$ should be emitted for the trait or not. | ||
| */ | ||
|
|
||
| // Definitions to be inspected | ||
|
|
||
| trait I9970IOApp { | ||
| protected val foo = 23 | ||
|
|
||
| def run(args: List[String]): Int | ||
|
|
||
| final def main(args: Array[String]): Unit = { | ||
| sys.exit(run(args.toList)) | ||
| } | ||
| } | ||
|
|
||
| object I9970IOApp { | ||
| trait Simple extends I9970IOApp { | ||
| def run: Unit | ||
|
|
||
| final def run(args: List[String]): Int = { | ||
| run | ||
| 0 | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // TASTy Inspector test boilerplate | ||
|
|
||
| object Test { | ||
| def main(args: Array[String]): Unit = { | ||
| // Artefact of the current test infrastructure | ||
| // TODO improve infrastructure to avoid needing this code on each test | ||
| val classpath = dotty.tools.dotc.util.ClasspathFromClassloader(this.getClass.getClassLoader).split(java.io.File.pathSeparator).find(_.contains("runWithCompiler")).get | ||
| val allTastyFiles = dotty.tools.io.Path(classpath).walkFilter(_.extension == "tasty").map(_.toString).toList | ||
| val tastyFiles = allTastyFiles.filter(_.contains("I9970")) | ||
|
|
||
| new TestInspector().inspectTastyFiles(tastyFiles) | ||
| } | ||
| } | ||
|
|
||
| // Inspector that performs the actual tests | ||
|
|
||
| class TestInspector() extends TastyInspector: | ||
|
|
||
| private var foundIOApp: Boolean = false | ||
| private var foundSimple: Boolean = false | ||
|
|
||
| protected def processCompilationUnit(using Quotes)(root: quotes.reflect.Tree): Unit = | ||
| foundIOApp = false | ||
| foundSimple = false | ||
| inspectClass(root) | ||
| // Sanity check to make sure that our pattern matches are not simply glossing over the things we want to test | ||
| assert(foundIOApp, "the inspector did not encounter IOApp") | ||
| assert(foundSimple, "the inspect did not encounter IOApp.Simple") | ||
|
|
||
| private def inspectClass(using Quotes)(tree: quotes.reflect.Tree): Unit = | ||
| import quotes.reflect._ | ||
| tree match | ||
| case t: PackageClause => | ||
| t.stats.foreach(inspectClass(_)) | ||
|
|
||
| case t: ClassDef if t.name.endsWith("$") => | ||
| t.body.foreach(inspectClass(_)) | ||
|
|
||
| case t: ClassDef => | ||
| t.name match | ||
| case "I9970IOApp" => | ||
| foundIOApp = true | ||
| // Cannot test the following because NoInits is not part of the quotes API | ||
| //assert(!t.symbol.flags.is(Flags.NoInits)) | ||
| assert(!t.constructor.symbol.flags.is(Flags.StableRealizable)) | ||
|
|
||
| case "Simple" => | ||
| foundSimple = true | ||
| // Cannot test the following because NoInits is not part of the quotes API | ||
| //assert(t.symbol.flags.is(Flags.NoInits)) | ||
| assert(t.constructor.symbol.flags.is(Flags.StableRealizable)) | ||
|
|
||
| case _ => | ||
| assert(false, s"unexpected ClassDef '${t.name}'") | ||
|
|
||
| case _ => | ||
| end inspectClass |