Enable generation of TASTy files readable for older compilers

[prolativ]

This is the first step towards achieving forward binary compatibility between different Scala minor releases (implementation of the first part of this proposal)

• Mark all parts of the the stdlib added after 3.0 with @since annotation specifying the minor release introducing the specific element of the API
• Add -Yscala-release compiler flag to specify the minimal target Scala release (minor version) which should be able to use the produced TASTy files (per analogy to -release setting the target version of JDK). This both sets the version of the generated TASTy files to the specified value and checks that the compiled code contains no runtime dependencies to parts of the stdlib introduced in a release with a higher version than specified (assuming their definitions have been properly marked with @since) or coming from other TASTy files with a higher version. If no -Yscala-release is specified then the current (latest) release version is used. This takes into account the fact that unstable versions of the compiler might have their TASTy version set to a higher value than a later stable version so e.g. compilation with 3.2.0-RC1-SNAPSHOT and -Yscala-release 3.2 (or no -Yscala-release specified) will produce TASTy files with an experimental TASTy version while changing the compiler version to stable 3.2.0 or -Yscala-release to 3.1 or 3.0 will result in using a stable format. -Yscala-release should not change the semantics of the compiled code (and it might prevent some code from compiling if the same semantics are not achievable for an older compiler - e.g. if a resolved implicit instance was added in a newer release)
• Modify Vulpix to enable constructing tests which require compiling or running some sources with different version of the compiler or different value of -Yscala-release. These versions are specified by adding appropriate suffixes to file names - similarly to _1, _2 suffixes for specifying order of compilation (which are still valid) one can add _r${release} and _c${compilerVersion} suffixes (e.g. _r3.0, _c3.1.0). Different kinds of suffixes can be mixed together on a single file name when needed

For future minor Scala versions (starting from 3.2.0) some additional changes might be necessary to keep this feature working:

• Mark all new APIs added to stdlib with @since (when removing @experimental)
• If the TASTy format actually changes (besides just extending stdlib) make sure that the produced TASTy doesn't contain anything that couldn't be read by the specified older version of the compiler. If some newer language features are unexpressible in older TASTy, the compilation should fail
• Check usages of new language imports (if they might break TASTy like described above)
• Add the new release in ScalaRelease.scala

Co-authored by @Kordyjan

[prolativ]

One thing that makes me wonder now is whether we should rename -scala-release to -Yscala-release for now and then drop Y for 3.2.0

[smarter]

This seems to be lacking tests that check that the scala 3.0 compiler is actually able to use a library published by the new compiler with -scala-release 3.0. This kind of test could be done as sbt scripted tests, similar to how we already test scala 2 compatibility (https://github.com/lampepfl/dotty/tree/master/sbt-test/scala2-compat).

If the TASTy format actually changes (besides just extending stdlib)

Even though tasty didn't technically change between 3.0 and 3.1, the same code might now generate different tasty, whenever that happens we need tests that ensure the newer compiler output will still be interpreted correctly by the older compiler. Since we didn't keep track of situations where this happened while developing 3.1, we now need to go through every PR that was merged to check for these situations. The one I'm aware of is #12979

[smarter]

whether we should rename -scala-release to -Yscala-release for now and then drop Y for 3.2.0

Possibly yes, but even if it's a -Y, that probably won't stop people from publishing libraries using it, so if we're saying that this is not ready for production usage yet then we need to carefully communicate this with library maintainers.

[smarter]

We should also have integration tests on whole projects I think, for example:

1. Using the latest compiler, compile cats with -scala-release 3.0
2. Using Scala 3.0.2, compile and run the tests of cats-effect with the output of step 1 on the classpath.

[bishabosha]

I'm not sure what this escape hatch helps with, shouldn't readFullHeader() already throw if the version is not compatible?

Edit: ok so we may be able to read the file, but this checks if it is safe to depend on with -scala-release version.

I am concerned about other libraries out there that use the scala package, such as Scalameta libraries

[smarter]

Even if only the scala stdlib used the scala package, this check still seems too broad: if the scala 3.2 compiler tries to read the stdlib of scala 3.3 and we actually changed tasty, it will crash when parsing the file.

[sjrd]

I was suspicious of the @since treatment before, and I think I now understand better why.

Clearly, as noted by @smarter, it is not sufficient to guarantee that an older compiler can read the TASTy of the file containing the new definition. In fact, it does not even guarantee that it could read the other stuff present in that file, even if it could jump over the new definitions entirely. It could very well be that the rest of the code in the file produces new TASTy as the result of different language rules, or because TASTy got a new feature that simplifies encoding, etc. Since the library is not compiled with -scala-release, its TASTy will not be tailored to be readable by an older compiler.

[sjrd]

Another, deeper problem with the @since treatment is that things like #14136 are fundamentally incompatible with it.

This wouldn't be an issue with the approach proposed by @julienrf at https://contributors.scala-lang.org/t/improving-scala-3-forward-compatibility/5298/4?u=sjrd (emit parts of the stdlib itself with -scala-release, and allow to depend on a newer stdlib than the compiler).

[prolativ]

@smarter maybe I missed something but in what situation would scala 3.2 compiler have to read TASTy of stdlib compiled with scala 3.3?

[prolativ]

If you add a dependency to a library compiled for 3.3 but you're compiling with 3.2 you should get an error while reading TASTy files of classes defined inside this library anyway so this shouldn't really matter that much, right?

[smarter]

You might end up reading a tasty file from the standard library on the class path before reading one from the library you depend on.

[prolativ]

OK, so we could add an extra check, something like

val isTastyCompatible =
              TastyFormat.isVersionCompatible(fileTastyVersion, TastyVersion.compilerVersion) && (
                  TastyFormat.isVersionCompatible(fileTastyVersion, ctx.tastyVersion) ||
                  classRoot.symbol.showFullName.startsWith("scala.")
              )

or maybe handling these cases with different error messages. This should handle both the case when e.g.
a) we compile something with scala 3.3 compiler but with -scala-release 3.2 and we want the compilation to fail when the code has some dependency to a library targeting 3.3
b) we use scala 3.2 compiler to compile code which for whatever reason brings scala 3.3 stdlib to the classpath

[prolativ]

Regarding a potential improvement of the check for whether something is indeed a part of stdlib: maybe we could be more specific in terms of the allowed package prefixes? I.e. check whether something starts with scala.annotation., scala.quoted. and so on? That might require slightly more work but still it shouldn't be that hard to maintain. Of course someone might still try to add something to these existing packages but that would be a very bad practice and I think we shouldn't focus on situations when someone writes such bad code on purpose

[prolativ]

I gave up the idea of more precise checks for prefixes. This seems not to be very flexible as some definitions are added directly to scala package so having to keep track of that is probably not worth the effort

[wilzbach]

I presume you have, but have you considered more type-safe alternatives like e.g.?

• since(majorVersion: Int, minorVersion)
• since(scalaRelease: "3.0" | "3.1")
• since(scalaRelease: ScalaRelease)

[prolativ]

The variant with two ints doesn't really give us much more safety. It would be still possible do use something invalid like since(1, -100).
A union of string literals would require changes to the signature of since for every minor release, which is not nice, as since is technically a part of stdlib (even though it's private).
ScalaRelease can't be used here because it's a part of compiler's internals.
So I think we'll have to live with strings although I've added some additional checks to validate their correctness

[bishabosha]

could this be in the scala.annotation.internal package?

[smarter]

Since we didn't keep track of situations where this happened while developing 3.1, we now need to go through every PR that was merged to check for these situations. The one I'm aware of is #12979

I had a quick look through all commits between 3.0.2 and 3.1 and didn't find anything else suspicious, but it'd be good if someone else did a pass too.

[bishabosha]

Even though tasty didn't technically change between 3.0 and 3.1, the same code might now generate different tasty, whenever that happens we need tests that ensure the newer compiler output will still be interpreted correctly by the older compiler. Since we didn't keep track of situations where this happened while developing 3.1, we now need to go through every PR that was merged to check for these situations.

Another possible candidate - #11686 (addition of an ordinal method and deriving.Mirror.Sum parent to a companion object) - If we wanted we could even replicate the old behaviour by the companion to have the same API as 3.0 we can avoid caching the mirror in the companion for a generic sum type with a child that is also a generic sum and -scala-release is 3.0

[sjrd]

The point should not be to replicate the old behavior. If users wanted the old behavior, they would use the old compiler.

We should in all cases compile code using the semantics of the new compiler. The only thing that the flag should change is how we encode those same semantics using older TASTy, and what valid code to reject because we can't.

[prolativ]

#12949 seems to work correctly because the definitions from exports which were missing are added during the compilation of the class into which they are exported.
In case of #11686 it's not only about the TASTy that gets generated during compilation of sealed traits because summoned mirrors are synthesized on the fly at the place where they are used. So scala 3.0 compiler will still complain when compiling summon[Mirror.Of[Foo]] where Foo is the root of a sealed types hierarchy even if Foo was compiled with scala 3.1 and -scala-release 3.0. There's no crash at runtime so that's not worse than if Foo was compiled with scala 3.0. However if we add one intermediate level i.e. compile Foo with 3.1 and -scala-release 3.0 and capture the summoned instance in a val which gets compiled with the same compiler and release and then we refer to this value from code compiled with 3.0 then the compilation is successful and it also works at runtime. So I think this is what we should expect. I'll commit some tests for that.

[therealcisse]

latestRelease.majorVersion used twice.

[prolativ]

Good catch. Actually I spotted that later too but haven't pushed my changes yet.

[prolativ]

We should also have integration tests on whole projects I think, for example:

1. Using the latest compiler, compile cats with -scala-release 3.0
2. Using Scala 3.0.2, compile and run the tests of cats-effect with the output of step 1 on the classpath.

@smarter
If we want to test a project with scala 3.0.2 we would have to compile all of its transitive dependencies with -scala-release 3.0. And cats-effect has quite a lot of them. Do you insist on testing this particular project or should I choose something more lightweight not to slow down the community build too much?

[prolativ]

Not sure how this would work with our infrastructure but if we decide to add a bunch of new projects to the community build as mentioned before (or rather old projects but compiled in a different way) would it make sense to create a new group of projects that would be tested in parallel to the existing groups A, B and C?

[smarter]

Do you insist on testing this particular project

No, that was just an example. Alternatively we could compile cats (or some other project) with -scala-release 3 but then compile and run the tests of that project with scala 3.0.2, so we don't need to deal with leaf projects with many dependencies. I don't have any specific idea of how this should be implemented and whether it should be implemented in the current community build or the jenkins-based one you've been working on.

[prolativ]

Actually compiling one project with a specific scala release set and then running the tests of the same project using an older scala runtime would be more difficult to implement in our current community build (or at least I haven't found an easy way to use a different version of scala in Compile and Test in sbt). But I think I already have most of the building blocks needed to implement what you requested at first without introducing too many modifications to the existing community build so now it's mainly the question how to get real value from adding these tests without making the community build swell too much.
Regarding the new community build the plans for the nearest future are that it is going to co-exist with the current build rather than replace it. Also the scenario of checking how binary compatibility works forwards and backwards shouldn't be too difficult to implement in the new approach.
And besides these test do you have any other remarks to my PR? It should get merged before the branch cut-off for 3.1.2-RC1 and I wouldn't want it to be a blocker.

[smarter]

you have any other remarks to my PR?

I don't have time to review this in more details, sorry. Perhaps @bishabosha can take over.

[smarter]

now it's mainly the question how to get real value from adding these tests without making the community build swell too much.

I suggest only running them on nightly builds.

[prolativ]

@smarter I slightly extended our community build to include the requested tests although I didn't manage to fully test the intended behaviour because it turned out it would probably require some extra support from tooling.
The problem is that when a project A is compiled with scala 3.1.x and -Yscala-release 3.0 sbt doesn't handle this option in any special way and it automatically adds scala 3.1.x stdlib as a library dependency to the project, which is visible in its pom after publishing. Then if we try to build project B using scala 3.0.2 and B depends on A scala 3.1.x stdlib gets added to the classpath but its TASTy files can't be read by the older compiler.
To make the tests pass for now I build cats-effect and all of its dependencies with the latest compiler and -Yscala-release 3.0 but there's no project built with scala 3.0.2.
I would suggest merging these changes and then, after 3.1.2-RC1 gets released, we might start thinking how to provide proper support for -Yscala-release 3.0 in build tools and update the tests in our community build.

@bishabosha could you have a look at the PR?

[smarter]

I'm still not a fan of this kind of check, as I said jars can have any names and it'd be weird if we suddenly started imposing undocumented restriction on jar names in some situations.

[smarter]

one situation where this could potentially fail: if someone uses sbt-assembly to package all their dependencies together and also uses staging, the compiler used at runtime will have only the sbt-assembly jar on its classpath which won't be called scala3-library

[prolativ]

Not sure if I understood you correctly. Let's consider a simple scenario.
Project A depends on project B.
Project B is packaged into sbt-assembly jar together with all of its dependencies (including stdlib)

Now we have a few possibilities of what version of the compiler A and B are compiled with:
a) Both A and B use the same minor version - OK
b) A uses 3.x1.y1 without -Yscala-release and B uses 3.x2.y2 if x1 > x2 - OK no matter if B uses -Yscala-release
c) A uses 3.x1.y1 and B uses 3.x2.y2 with -Yscala-release 3.x1 if x1 < x2 - here we encounter a problem because the fat jar will contain stdlib classes from 3.x2.y2 instead of 3.x1.y1 and 3.x1.y1 compiler won't be able to read them.
This isn't specifically a problem of this one check because the problem would still occur if x1 = 0 and x2 = 1. This seems all about the fact that build tools are not aware of -Yscala-release. But still we would need to release this feature as experimental first to let people play with it and then fix the build tools

[smarter]

I don't have a specific scenario in mind, I just don't think it's wise to have a check like this.

[smarter]

I really think we should just warn people to not use package scala, it's already dangerous because of package private scala definitions.

[smarter]

(also the reason why we're special casing the standard library here should be documented in the code)

[smarter]

I'm not opposed to having a filename-based check for now if it avoids accidentally using a 3.1 dependency from a -Yscala-release 3.0 project, but we should treat it as a temporary hack until we have a reliable way of detecting tasty files from the standard library (we could have a flag in the tasty file for that for example).

[prolativ]

Actually I thought about adding such kind of a flag to TASTy but for this proper solution we would have to wait until the next minor release. And even now in the worst case our checks would be too strict. So while checking TASTy version we make an exception if we're pretty sure some class comes from the stdlib while checking only the package prefix would allow some other libraries leak references to the newer stdlib API

[smarter]

This makes it sound like any minor version of Scala 3 might break binary compatibility, but we have no plans to do that so far and won't break it for many years hopefully. Moreover, if/when we do break bincompat, there's a good chance we'll call it Scala 4 to avoid confusion although this isn't settled (cf #10244 (comment))

[lrytz]

When I use 3.2 and compile a library with -Yscala-target 3.1, what does the POM say? Will it depend on scala3-library_3:3.1.x or 3.2.x? Is this the reason for the exception in the ClassfileParser?

[prolativ]

@lrytz as already mentioned, -Yscala-release doesn't by itself change the dependencies declared in POM files. We would need to make build tools aware of this flag to handle this properly. Until that time users will have to use the workaround given in the docs

[lrytz]

OK, thank you. If users of an older compiler need to manually enforce the Scala 3 library version, why is the exception in the ClassfileParser needed?

[prolativ]

As explained in the code comment: special handling of stdlib in ClassfileParser is needed because by setting -Yscala-release 3.0 we make scala 3.1 compiler reject all TASTy files in a version newer than 28.0-0 (corresponding to scala 3.0). However When compiling code with scala 3.1 the stdlib from 3.1 is still on the classpath and this shouldn't cause the compiler to crash. And this is safe because during compilation of sources we check that all references to stdlib classes (and their methods etc.) will remain legal when the binaries of stdlib get replaced by an older version at runtime

[prolativ]

Actually we first needed to find the workaround with dependencyOverrides while trying to test -Yscala-release in our community build

[lrytz]

Got it.

• Library Authors using 3.2 with -Yscala-release 3.1 will have the Scala 3.2 library on the classpath, the exception in the classfile parser allows that.
• Users of 3.1 need to override the Scala 3 library version in the build tool to 3.1, otherwise they might end up with a 3.2 library on the classpath. An exception for this case would not be safe because the 3.2 Tasty might have entries that the 3.1 compiler doesn't understand.

Having -Yscala-release change the POM dependency in the build tools would be nice...

[griggt]

It seems that the new Vulpix-based compat tests are failing on Windows, see test_windows_full workflow on merge:

https://github.com/lampepfl/dotty/runs/4869516643?check_suite_focus=true#step:4:900

Fatal compiler crash when compiling: tests\run\forwardCompat-strictEquals:
Cannot run program "C:\Windows\ServiceProfiles\NetworkService\.cache\dotty\test\compilers\scala3-3.0.2/bin/scalac": CreateProcess error=193, %1 is not a valid Win32 application
java.lang.ProcessBuilder.start(ProcessBuilder.java:1048)
java.lang.Runtime.exec(Runtime.java:621)
java.lang.Runtime.exec(Runtime.java:486)
dotty.tools.vulpix.ParallelTesting$Test.compileWithOtherCompiler(ParallelTesting.scala:539)

[prolativ]

This should be fixed by #14301

[SethTisue]

This was removed by #15146, due to the considerations summarized in https://www.scala-lang.org/blog/2022/08/17/long-term-compatibility-plans.html