Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with Scala 2.13.0-M4. #3361

Merged
merged 3 commits into from
May 15, 2018
Merged

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented May 12, 2018

This PR puts the codebase in a state where we can build the compiler plugins, libraries and test suite with Scala 2.12.0-M4 and its new collections. The tools API and related artifacts are not yet updated.

This is the minimal set of changes necessary to be able to build and back-publish for 2.13.0-M4 when it comes out. We will not be able to publish tools artifact for that version, but that is not critical.

In the next commits, the overrides-2.13 directory will contain
overrides for 2.13.0-M4+. This commit copies existing overrides
to an overrides-2.13.0-M3 directory so that they have a higher
precedence over the overrides of the overrides-2.13 directory.

Files scala/Array.scala and scala/Enumeration.scala have been
copied without changes from the general overrides/ directory. All
the other files have been copied from the overrides-2.13/ directory.
@sjrd sjrd changed the title CI ONLY Compatibility with Scala 2.13.0-M4. DO NOT MERGE Compatibility with Scala 2.13.0-M4. May 14, 2018
@sjrd sjrd requested a review from gzm0 May 14, 2018 09:53
@sjrd
Copy link
Member Author

sjrd commented May 14, 2018

This PR is ready for review. But it must not be merged yet because I will have to adapt it a little bit after they merge scala/scala#6620 upstream.

This PR is enough to build and publish for 2.13.0-M4. So once it's merged, we can do an "emergency" release of 0.6.23 so that we're ready to back-publish it for 2.13.0-M4 when that one is released (any day now, according to the scala/scala team).

@sjrd sjrd force-pushed the compat-with-2.13.0-M4 branch from adad485 to 0012d82 Compare May 14, 2018 09:59
@sjrd sjrd mentioned this pull request May 14, 2018
11 tasks
b.result()
}

// TODO overload collect, flatMap and concat
Copy link
Member Author

@sjrd sjrd May 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julienrf How important is this TODO? What are we missing if we do not do it now? And how hard would it be to address?

Copy link
Contributor

@julienrf julienrf May 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry I forgot about that TODO. If we don’t overload collect, flatMap and concat they can only return a mutable.Map (since we don’t rely anymore on CanBuildFrom to determine the result type). Above you can see that I’ve added an overload of map that returns a WrappedDictionary[B] when the transformation function returns a tuple (String, B).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks. We should probably address that, then, because it might be a source breaking change for some clients.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -37,7 +37,7 @@ REVERSI_OPT_GZ_SIZE=$(stat '-c%s' "$REVERSI_OPT.gz")

case $FULLVER in
2.10.2)
REVERSI_PREOPT_EXPECTEDSIZE=532000
REVERSI_PREOPT_EXPECTEDSIZE=533000
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The small increase in code size comes from the indirection through runtime.toScalaVarArgs. Even though it is inlined, the argument must be extracted in a temp var by the optimizer to preserve evaluation order (or so it thinks) which slightly increase code size.

@sjrd sjrd force-pushed the compat-with-2.13.0-M4 branch from 0012d82 to b156691 Compare May 14, 2018 13:09
@sjrd
Copy link
Member Author

sjrd commented May 14, 2018

Updated after scala/scala#6620 was merged upstream.
But actually we're also depending on when/if/how much of scala/scala#6611 is merged. So this is still a don't merge.

@sjrd sjrd force-pushed the compat-with-2.13.0-M4 branch from b156691 to 6d7d0a9 Compare May 14, 2018 15:56
@sjrd sjrd changed the title DO NOT MERGE Compatibility with Scala 2.13.0-M4. Compatibility with Scala 2.13.0-M4. May 14, 2018
@sjrd
Copy link
Member Author

sjrd commented May 14, 2018

OK, now this PR and its friend #3344 are complete wrt. the upcoming 2.13.0-M4. At least, to the best of my knowledge, and assuming they don't break anything anymore.

@sjrd
Copy link
Member Author

sjrd commented May 14, 2018

Actually ... not ready. They broke it again: https://github.com/scala/scala/pull/6572/files

@sjrd sjrd changed the title Compatibility with Scala 2.13.0-M4. DO NOT MERGE Compatibility with Scala 2.13.0-M4. May 14, 2018
julienrf added 2 commits May 14, 2018 19:36
They were taken from the commit
a0026305125dcc814097b1fbb798fe925ca9f81f in scala/scala.
This commit puts the codebase in a state where we can build the
compiler plugins, libraries and test suite with Scala 2.13.0-M4
and its new collections. The tools API and related artifacts are
not yet updated.

This is the minimal set of changes necessary to be able to build
and back-publish for 2.13.0-M4 when it comes out. We will not be
able to publish tools artifact for that version, but that is not
critical.
@sjrd sjrd force-pushed the compat-with-2.13.0-M4 branch from 6d7d0a9 to 5ac5168 Compare May 14, 2018 18:44
@sjrd sjrd changed the title DO NOT MERGE Compatibility with Scala 2.13.0-M4. Compatibility with Scala 2.13.0-M4. May 14, 2018
@sjrd
Copy link
Member Author

sjrd commented May 14, 2018

Good to go again.

Note that I made a PR upstream at scala/scala#6644, but since we override Array.scala anyway, we can go ahead whether or not it is merged upstream.

@sjrd
Copy link
Member Author

sjrd commented May 15, 2018

Well, Scala 2.13.0-M4 has reached Maven Central, so emergency just turned into deportation. I'm going to go ahead and merge this and publish 0.6.23. At least it works. If tweaks should be done, they can still be done later.

@sjrd sjrd merged commit a25474d into scala-js:0.6.x May 15, 2018
@sjrd sjrd deleted the compat-with-2.13.0-M4 branch May 15, 2018 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants