Compile Scala library with Dotty and test its TASTy #9925

nicolasstucki · 2020-10-01T10:09:59Z

No description provided.

dotty-scala-library-from-tasty-tests/test/LibraryTest.scala

smarter · 2020-10-05T15:16:56Z

project/Build.scala

@@ -817,6 +817,74 @@ object Build {
      javaOptions := (javaOptions in `dotty-compiler-bootstrapped`).value
    )

+  lazy val `dotty-scala-library-from-tasty-tests` = project.in(file("dotty-scala-library-from-tasty-tests")).


I don't understand what this project is for.

It is there to test that we can recompile the jar from dotty-scala-library from a bootstrapped compiler with the normal bootstrapped library. We needed an extra project to get the dependencies on the bootstrapped compiler right.

smarter · 2020-10-05T15:17:25Z

project/Build.scala

+    )
+
+
+  // Scala library compiled by dotty using the latest published sources of the library


I suggest using the stdlib sources from our community-build instead, that way if we need to change something we can change it in the community build submodule.

Those sources are not aligned with the library that we are using to run dotty apps. Their documentation may also slightly defer.

We should keep them aligned if possible.

If you change things because you need to change something, by definition they're not aligned anymore.

Sure but we can make PRs to keep them aligned if needed. I think it's just confusing to operate on multiple versions of scala-library at the same time, I don't see the advantages.

Under that logic, if we don't want to operate on multiple versions of the scala-library, we should remove the stdlib213 from the community-build as it is not aligned with the one we use to run dotty. But the one in the community build allows us to future proof the compilation of the library, therefore it is good to have this second variant of the scala-library.

smarter · 2020-10-05T15:17:51Z

project/Build.scala

+    )
+
+  // Tests that we can compile and run an app using the scala-library compiled by dotty
+  lazy val `dotty-scala-library-sandbox` = project.in(file("sandbox/dotty-scala-library")).


can we use dotty-scala-library/test instead of adding an extra project?

The library dependency setup is different on those two projects. That would not be testing what we want to test.

Different how?

I don't remember why we did not set up the tests of that project to this. @sjrd was there a specific reason? Or just for simplicity?

smarter · 2020-10-05T15:18:38Z

project/Build.scala

+    dependsOn(`dotty-scala-library`).
+    settings(
+      managedClasspath in Compile ~= {
+        _.filterNot(file => file.data.getName == "scala-library-2.13.3.jar")


If you want to remove the stdlib from the dependencies of a scala project, you can use autoScalaLibrary := false (this will also remove dotty-library)

We don't want to remove the dotty-library.

Ok, but then don't hardcore 2.13.3, use stdlibVersion

smarter · 2020-10-05T15:22:18Z

dotty-scala-library/src/scala/StringContext.scala

@@ -0,0 +1,488 @@
+/*


Why is this file needed? I tthought we already handled macro ??? to allow compiling the scala 2 stdlb as is.

We can compile them, but we assume that if they are going to be consumed from Scala 3 we need a scala 3 macro implementation as well. Here that second implementation is missing and when we load the class we ignore the Scala 2 macros. This implies that when loaded, StringContext does not have s, f and raw methods.

The usual solution is to add the second macro, but because those are intrinsic macros we can get away by just making them non-macros and let the compiler do its usual transformation of these methods.

Can't we interpret macro ??? like ??? when it doesn't come from scala 2 so that we can keep using the same source?

Not under the normal dual macros rules. We would have to add an extra compilation flag for this special case which seems to be overkill in this single case.

which seems to be overkill in this single case.

I disagree, I think that a special case is better than having a copy of a file we need to worry about keeping up to date. Furthermore, it would be nice if in scala/scala we didn't just compile the stdlib with dotty but we also ran the stdlib tests, that is only possible if the compiler can handle the normal StringContext correctly if I understand this correctly.

I removed the file and special-cased these macros in TreeUnpickler.

nicolasstucki · 2020-10-16T12:45:50Z

@smarter I merged the sandbox into scala3-scala2-library (former dotty-scala-library). I kept scala3-scala2-library-tasty-tasty-tests (former dotty-scala-library-tasty-tests).
I tried to put the tests in BootstrappedOnlyCompilationTests but they were timing out all the time on the CI, these tests are a bit too heavy to run in parallel with other tests.

smarter · 2020-10-16T13:06:10Z

scala3-scala2-library isn't a great name, maybe stdlib-bootstrapped?

smarter · 2020-10-16T13:07:55Z

project/Build.scala

+      },
+    )
+
+  lazy val `scala3-scala2-library-tasty-tests` = project.in(file("scala3-scala2-library-tasty-tests")).


This should have some documentation explaining what it tests and why that needs to be done in a separate project.

nicolasstucki · 2020-10-19T11:39:08Z

@smarter now it is ready for re-review. All tests pass again.

smarter · 2020-10-20T12:25:20Z

project/Build.scala

@@ -805,6 +805,74 @@ object Build {
      javaOptions := (javaOptions in `scala3-compiler-bootstrapped`).value
    )

+  /** Scala library compiled by dotty using the latest published sources of the library */
+  lazy val `scala3-scala2-library` = project.in(file("scala3-scala2-library")).


I'm not a fan of that name, I suggest stdlib-booststrapped or stdlib-recompiled maybe.

smarter · 2020-10-20T12:28:10Z

scala3-scala2-library-tasty-tests/test/Scala3Scala2LibraryTASYyTest.scala

+    compileFromTasty(loadBlacklisted.union(compileBlacklisted))
+
+  @Ignore
+  @Test def testWhiteListFromTasty: Unit =


What do we need a whitelist for?

It was a simple way to test a subset of files. Now that I have minimized everything I can remove it.

On second thought, I prefer to keep in case a change breaks the compilation of the library. This would help to minimize the failing combination of files faster.

That's fine but then please add a comment on top of the test explaining that

scala3-scala2-library-tasty-tests/test/Scala3Scala2LibraryTASYyTest.scala

smarter · 2020-10-20T12:30:42Z

scala3-scala2-library-tasty-tests/test/Scala3Scala2LibraryTASYyTest.scala

+  /** List of classes that cannot be recompilied from TASTy */
+  def compileBlacklist = List[String](
+    // OK?
+    // failed: java.lang.AssertionError: assertion failed: class Boolean


We should have an issue for thos and the comment should point to that issue

nicolasstucki · 2020-10-20T13:15:50Z

scala3 prefix is not needed but it makes naming consistent

smarter · 2020-10-20T13:17:02Z

I think ideally we would only use the scala3 prefix for the artifacts we distribute (library, compiler, ...), for internal things it doesn't seem useful.

nicolasstucki · 2020-10-20T14:32:19Z

We already have a few projects with scala3 that we don’t distribute: scala3-tastydoc-input, scala3-bench-run, scala3-sbt-bridge-tests, scala3-tastydoc, scala3-library, scala3-compiler, scala3-bench
Should we rename those?

smarter · 2020-10-20T15:08:26Z

Should we rename those?

We could yes (in fact, we could also rename scala3-compiler to compiler, etc, we just need the artifact name to start with scala3-, but the name of the sbt project can be whatever we want, so we could make them easier to type)

nicolasstucki · 2020-10-20T15:18:51Z

Let's make a single PR afterward to renames them. It will be simpler to track.

nicolasstucki · 2020-10-20T15:25:32Z

I renamed this one to stdlib-bootstrapped.

smarter · 2020-10-20T15:31:03Z

project/Build.scala

@@ -805,6 +805,74 @@ object Build {
      javaOptions := (javaOptions in `scala3-compiler-bootstrapped`).value
    )

+  /** Scala library compiled by dotty using the latest published sources of the library */
+  lazy val `stdlib-bootstrappedject.in(file("stdlib-stdlib-bootstrapped


smarter · 2020-10-20T15:31:55Z

project/Build.scala

+   *   - inspector: test that we can load the contents of the jar using the tasty inspector
+   *   - from-tasty: test that we can recompile the contents of the jar using `dotc -from-tasty`
+   */
+  lazy val `stdlib-bootstrappedtests` = project.in(file("stdlib-stdlib-bootstrapped).


I wouldn't call that project just stdlib-bootstrappedtests, because it's not clear how it's different from stdlib-bootstrapped/test, -tasty-test or -sanity-test would be better I think

smarter · 2020-10-20T15:33:20Z

project/Build.scala

+    dependsOn(dottyCompiler(Bootstrapped) % "provided; compile->runtime; test->test").
+    dependsOn(`scala3-tasty-inspector` % "test->test").
+    settings(commonBootstrappedSettings).
+    settings(


You could add a setting:

Suggested change

settings(

settings(

moduleName := "scala-library"

this way the jar will be called scala-library_3.0.0-M1.jar which makes sense.

nicolasstucki force-pushed the scala-library-from-tasty-tests branch from 24852e8 to 6c0f4de Compare October 1, 2020 10:14

nicolasstucki mentioned this pull request Oct 1, 2020

Intrinsify StringContext.f #9929

Merged

nicolasstucki force-pushed the scala-library-from-tasty-tests branch 17 times, most recently from 0ee5051 to 59e7a4f Compare October 5, 2020 12:08

nicolasstucki marked this pull request as ready for review October 5, 2020 12:48

nicolasstucki requested a review from smarter October 5, 2020 12:48

nicolasstucki assigned smarter Oct 5, 2020

smarter reviewed Oct 5, 2020

View reviewed changes

dotty-scala-library-from-tasty-tests/test/LibraryTest.scala Outdated Show resolved Hide resolved

nicolasstucki force-pushed the scala-library-from-tasty-tests branch from 006eb1f to 69dda1c Compare October 5, 2020 14:01

smarter requested changes Oct 5, 2020

View reviewed changes

nicolasstucki requested a review from smarter October 5, 2020 16:04

nicolasstucki mentioned this pull request Oct 12, 2020

Handle StringContext intrinsic macro definitions compiled with Dotty #9986

Merged

smarter mentioned this pull request Oct 12, 2020

Segregate scala2-interop-tests and add to CI #9990

Closed

nicolasstucki force-pushed the scala-library-from-tasty-tests branch from 810becc to 77d0544 Compare October 13, 2020 13:28

nicolasstucki mentioned this pull request Oct 13, 2020

Fix index out of bounds #9995

Merged

nicolasstucki force-pushed the scala-library-from-tasty-tests branch 2 times, most recently from 8ec5eb2 to dccd60b Compare October 16, 2020 07:48

smarter reviewed Oct 16, 2020

View reviewed changes

nicolasstucki force-pushed the scala-library-from-tasty-tests branch 2 times, most recently from 69a5e1b to 3647e95 Compare October 19, 2020 07:37

nicolasstucki requested a review from smarter October 19, 2020 11:38

smarter requested changes Oct 20, 2020

View reviewed changes

nicolasstucki force-pushed the scala-library-from-tasty-tests branch from 2a422ae to 8567ffc Compare October 20, 2020 14:56

nicolasstucki force-pushed the scala-library-from-tasty-tests branch from ed2e882 to aee734d Compare October 20, 2020 15:20

Fix typo: 👻strap -> bootstrap

ce67fd1

nicolasstucki force-pushed the scala-library-from-tasty-tests branch from aee734d to b4dec3c Compare October 20, 2020 15:24

smarter reviewed Oct 20, 2020

View reviewed changes

nicolasstucki force-pushed the scala-library-from-tasty-tests branch from b4dec3c to 3a6f1f7 Compare October 20, 2020 15:51

Compile Scala library with Dotty and test its TASTy

b02abfc

nicolasstucki force-pushed the scala-library-from-tasty-tests branch from 3a6f1f7 to b02abfc Compare October 20, 2020 15:53

smarter approved these changes Oct 21, 2020

View reviewed changes

nicolasstucki merged commit 57dd672 into scala:master Oct 21, 2020

nicolasstucki deleted the scala-library-from-tasty-tests branch October 21, 2020 15:02

nicolasstucki mentioned this pull request Oct 21, 2020

Fix #9994: Handle subtyping between two ThisType #10008

Merged

		)


		// Scala library compiled by dotty using the latest published sources of the library

Compile Scala library with Dotty and test its TASTy #9925

Compile Scala library with Dotty and test its TASTy #9925

Uh oh!

Conversation

nicolasstucki commented Oct 1, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarter Oct 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolasstucki Oct 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolasstucki commented Oct 16, 2020

Uh oh!

smarter commented Oct 16, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolasstucki commented Oct 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolasstucki commented Oct 20, 2020

Uh oh!

smarter commented Oct 20, 2020

Uh oh!

smarter Oct 5, 2020 •

edited

Loading

nicolasstucki Oct 6, 2020 •

edited

Loading