Cannot test with r3.1 and then c3.1.0

[nicolasstucki]

Compiler version

300a8b6

Minimized example

// tests/pos/mytest/A_1_r3.1.scala
class Foo

// tests/pos/mytest/B_2_c3.1.0.scala
class Bar extends Foo

Output

sbt> testCompilation tests/pos/mytest
... 
No files matched "mytest" in test                                   
[info] Test dotty.tools.dotc.CompilationTests.posMacros started
Compilation failed for: 'tests/pos/mytest'                   
[=======================================] completed (1/1, 1 failed, 3s)
...

Expectation

Should pass.

Should also show the error (see #14303).

[nicolasstucki]

I reproduced it manually an got the following error.

> git checkout 300a8b688b9d671341dad58b097367037015b656
> mkdir out2
> sbt "scalac -d out2 -Yrelease 3.1 tests/pos-macros/backwardCompat-3.1/Macro_1_r3.1.scala"
...
> git checkout release-3.1.0
> mkdir out3
> sbt "scalac -classpath out2 -d out3 tests/pos/mytest/B_2_c3.1.0.scala"
...
error while loading Foo,
class file out2/Foo.class is broken, reading aborted with class dotty.tools.tasty.UnpickleException
TASTy signature has wrong version.
 expected: {majorVersion: 28, minorVersion: 1}
 found   : {majorVersion: 28, minorVersion: 2 [unstable release: 1]}

This TASTy file was produced by an unstable release.
To read this TASTy file, your tooling must be at the same version.
The TASTy file was produced by Scala 3.1.2-RC1-bin-SNAPSHOT-nonbootstrapped-git-c3eaadd.
-- [E006] Not Found Error: tests/pos/mytest/B_2_c3.1.0.scala:1:18 --------------
1 |class Bar extends Foo
  |                  ^^^
  |                  Not found: type Foo

I see that the older compiler cannot read this artifact because we generated non-release version of TASTy.

@prolativ does this make sense from the testing framework perspective?

The same example passes on 3.0.x (as in forwardCompat-nestedSumMirror/forwardCompat-refinedGivens/forwardCompat-strictEquals/forwardCompat-unusedImport) because we did not have this check yet. We need to figure out how to overcome this limitation or we won't be able to use these test after 3.0.2.

[prolativ]

@nicolasstucki This looks like a broader question how we understand the correspondence between versions of TASTy and scala. E.g. scala 3.1.0 emits TASTy 28.1-0, so do 3.1.1-RC1, 3.1.1-RC2 and 3.1.1, and so will all nonsnapshot 3.1.x versions. However in our master branch TASTy minor and experimental versions are temporarily bumped to higher values and then downgraded during release. This makes sense when we're preparing a new 3.x.0 release because the TASTy format has not been fixed for this release. But I'm wondering if later snapshot versions like 3.1.2-RC1-SNAPSHOT should produce experimental TASTy instead of the stable version as theoretically they cannt change the format.
Initially 3.1.2-RC1-SNAPSHOT with -Yscala-release 3.1 indeed produced stable 28.1-0 TASTy, similarly as it produced 28.0-0 with -Yscala-release 3.0 but this caused some other problems in tests and was against the intuition that Yscala-release 3.x should work as noop for any 3.x.y* compiler

[prolativ]

As far as I remember there were problems when we tried to compile something like A_1_r3.1.scala with B_2.scala because the latter file was compiled with the latest snapshot compiler from the branch and TASTY 28.1-0 and 28.2-1 are incompatible. In your case it's the other way round - A would produces unstable TASTy 28.2-1 and B uses the compiler using TASTy 28.1-0

[nicolasstucki]

A possible solution would be to not fetch the published artifacts but have a submodule for each supported version containing the release branches (for example https://github.com/lampepfl/dotty/tree/release-3.0.2) and then compile it locally. This would allow us to compile locally a version of the compiler that is equivalent to the released version but would be marked as a snapshot. Therefore it would be able to consume nonstable artifacts.

This would also help in another direction we haven't considered in the past. If a test fails and we need to debug it we might need to debug what the released version of the compiler receives to fully understand the interaction between the two compilers. Therefore if we have a submodule we could modify it (for example to enable the Printers) and run the tests on that version. I have already hit this situation.

[prolativ]

That makes sense. Although all the previously released versions of the compiler that we would like to use in tests should are probably cached in our CI in order not to make the builds last too long

[smarter]

I'm wary of using submodules here, we already have a hard time managing our current usage of submodules without adding even more complexity in the mix. I would definitely prefer an escape hatch for the tests than this.

[prolativ]

Maybe we don't actually need submodules and additional tags/branches like test-3.0.2 (per analogy to release-3.0.2) would be enough as this is all inside the same repo? Then if e.g. we need to enable Printers in an older compiler it should be enough to locally checkout to this branch, change the code and run sbt publishLocal. I'm currently trying to use coursier to get the specified version of the compiler so this should work almost seamlessly. The only trick needed would be that we would either use suffixes like _c3.0.2-SNAPSHOT or make vulpix handle that under the hood somehow.

[nicolasstucki]

In that scenario would it be possible to edit the code of the "released" version of the compiler to allow debugging?

[prolativ]

checkout to a branch -> adjust tasty version in TastyFormat.scala -> publishLocal
I think this should work even if the compiler version contains -SNAPSHOT

[smarter]

This all still looks really complicated, almost like we're reinventing a build tool in our test framework when we already have sbt scripted tests.

[smarter]

The goal of our tests is to check what will actually happen when users use these flags, if we start hacking our own compiler code, we're straying away from this. I agree that 3.1.2-XX-SNAPSHOT should output 3.1 tasty since we haven't changed the tasty format.

[nicolasstucki]

The goal of our tests is to check what will actually happen when users use these flags, if we start hacking our own compiler code, we're straying away from this.

We only need this while debugging the code. It is never going to be on master. Therefore we won't be straying away.

[nicolasstucki]

This all still looks really complicated, almost like we're reinventing a build tool in our test framework when we already have sbt scripted tests.

@smarter could you try to port the tests in https://github.com/lampepfl/dotty/pull/14302/files into scripted tests? Not sure how to do it properly. Then we could compare the 2 approaches.

[prolativ]

I think sbt scripted tests would suffer from the same problems we have here. To bring things up together into one solution:

• After the TASTy format gets stabilized for a new minor release we should not bump TASTy version for SNAPSHOT versions of the compiler. I.e. when we decide our next release is going to be 3.2.0-RC1 we change the project version to 3.2.0-RC1-bin-SNAPSHOT and TASTy version to 28.2-1. Until the release of 3.2.0 stable we might bump the experimental part of the TASTy version. Before that point the version of generated TASTy files will be experimental 28.2-X when we compile with -Yscala-release 3.2 or without this flag. That would mean we can't use _c3.2.0 in your tests yet but it makes sense as 3.2.0 hasn't been released. During the release of scala 3.2.0 we set TASTy version to 28.2-0 and all later versions of the compiler within 3.2.x series (including SNAPSHOT releases) will keep emitting TASTy 28.2-0 as the format has been fixed and it cannot be changed. Now we can use _c3.2.0 in tests while having _r3.2 files as dependencies without any additional hacks. We should also set the TASTy version to 28.1-0 on our current master branch.
• If you need to enable Printers in an older compiler for the purpose of debugging you should manually checkout to an older branch, modify the compiler and run publishLocal. Preferably publishing should be done after changing the compiler version to something like 3.0.2-bin-test-SNAPSHOT just to avoid messing up your local ivy cache for stable versions. Then you would need to temporarily rename your test file so that it contains the _c3.0.2-bin-test-SNAPSHOT suffix.

[nicolasstucki]

An alternative that might be simpler is to allow stable compiler versions to compile as if they were unstable. We could add a -Yas-experimental that would make the compiler behave as a snapshot or nightly: accepting other nightly dependencies but generating TASTy marked as experimental.

The advantage of this approach is that we would never be able to publish a TASTy marked as stable from a non-stable version of the compiler. Keeping this strong guarantee seems paramount for the correct behavior published stable artifacts.

[prolativ]

IMO we have two separate problems which we are unnecessarily trying to solve with a single mechanism:

1. Which version of the compiler should be able to write and read which version of TASTy?
2. How to prevent TASTy files generated by snapshot/experimental compilers from leaking into production/non-experimental codebases?

I think the -Yas-experimental solution alone wouldn't solve our current problem with tests if we don't change the versioning scheme of TASTy.
Let's have a look at the example from the beginning of this thread:

TASTy signature has wrong version.
 expected: {majorVersion: 28, minorVersion: 1}
 found   : {majorVersion: 28, minorVersion: 2 [unstable release: 1]}

This TASTy file was produced by an unstable release.
To read this TASTy file, your tooling must be at the same version.
The TASTy file was produced by Scala 3.1.2-RC1-bin-SNAPSHOT-nonbootstrapped-git-c3eaadd.

Here we're compiling some code with -scala-output-version 3.1 flag using a locally built experimental compiler and then we're checking if the non-experimental published compiler 3.1.0 can consume the produced TASTy. Even if -Yas-experimental allowed for consumption of experimental TASTy files by non-experimental compilers there would be a mismatch in minor TASTy version anyway

 expected: {majorVersion: 28, minorVersion: 1 [unstable release: 1]}
 found   : {majorVersion: 28, minorVersion: 2 [unstable release: 1]}

What's more, -Yas-experimental as a boolean flag would be ambiguous because experimental version can be greater than 1.

I would then suggest the following solutions to the problems I mentioned at the beginning of this comment:

1. Replace the current versioning scheme of TASTy with the one that I described earlier, e.g. (with -SN meaning -SNAPSHOT or -NIGHTLY; TASTy versions with e suffix are experimental):

scala version	3.2.0-SN	3.2.0-RC1	3.2.0	3.2.1-SN	3.2.1-RC1	3.2.1	3.2.2-SN	3.2.2-RC1	3.2.2	3.3.0-SN
TASTy version (current)	28.2e	28.2e	28.2	28.3e	28.2	28.2	28.3e	28.2	28.2	28.3e
TASTy version (new)	28.2e	28.2e	28.2	28.2	28.2	28.2	28.2	28.2	28.2	28.3e

2. a) Add a check while reading TASTy files to ensure that VersionString is not an experimental/unstable version when the compiler itself is stable.
3. b) Add -Yas-experimental flag (we might yet think if this is the best name), which:

• allows compilers in stable versions to read TASTy files in a stable format produced by an experimental compiler
• sets VersionString of TASTy files produced by a stable compiler so that these files look as if they were produced by an experimental compiler version without actually changing the TASTy version (major, minor and experimental fields).

[prolativ]

After an internal discussion it has been agreed that:

• We should switch to the new versioning scheme show above - so experimental version part in TASTy file header will indicate only if the format of the file is experimental or stable, not if the file was produced by an experimental compiler or not
• The TASTy format will get extended (from scala 3.2.0 on) to include a field indicating if the file itself is experimental
• We should add a new compiler flag allowing a stable compiler to behave like an unstable one - allowing it to consume experimental TASTy files (assuming there's no mismatch in the format's version), which would however propagate the experimental marker to the compiler's outputs

Things to be discussed (preferably in this thread):

• How should we define the field added to TASTy format: should this be just a boolean or do we need to store some more information?
• What should be the name of the new compiler flag? Should it take some parameters?
• Under what circumstances should TASTy files be marked as experimental? Should unstable compilers always produce experimental TASTy or only if the compiled code uses (or might potentially use) an experimental feature? In the latter case what about experimental features which are purely syntactical (e.g. fewer braces)?

[smarter]

This looks like a lot of changes/complications, is all of this really needed just to let us test the -scala-output-version flag? If we go back earlier in the discussion you mentioned:

Initially 3.1.2-RC1-SNAPSHOT with -Yscala-release 3.1 indeed produced stable 28.1-0 TASTy, similarly as it produced 28.0-0 with -Yscala-release 3.0 but this caused some other problems in tests and was against the intuition that Yscala-release 3.x should work as noop for any 3.x.y* compiler

To me this still makes sense: -Yscala-release 3.1 would mean that even an experimental compiler can act as a release compiler. Is this not good enough for other reasons?

[bishabosha]

should this be closed?

[nicolasstucki]

Yes, but we should create another issue to track the confusion between the tasty experimental version and the tasty generated by an experimental compiler.

[ckipp01]

With the comment in #14633 (comment) and the similar issue in #16204, is this something that can be closed?

[prolativ]

I think so, as the problem as stated in the issue title is not a concern anymore