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

Support Scala 2.12 Mixins #2685

Merged
merged 10 commits into from
Jun 14, 2017
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jun 5, 2017

This PR is a first step to Scala 2.12 support from Dotty. It makes it possible for Dotty to extend traits compiled with Scala 2.12.

@odersky odersky requested a review from DarkDimius June 5, 2017 09:47
@smarter
Copy link
Member

smarter commented Jun 5, 2017

not found: ./src/dotty/tools/dotc/transform/LinkScala2ImplClasses.scala

In compiler/test/dotc/tests.scala there is a reference to LinkScala2ImplClasses.scala that needs to be renamed.

@@ -399,9 +399,15 @@ object Flags {
/** A module variable (Scala 2.x only) */
final val Scala2ModuleVar = termFlag(57, "<modulevar>")

/** A Scala 2.12 trait that has been augmented with static members */
final val Scala_2_12_Augmented = typeFlag(57, "<scala12augmented")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know the convention about type flag names, but this is the only one with <, but without >.
Should it be <scala12augmented> instead?

@@ -734,7 +734,7 @@ object Denotations {
this
case _ =>
if (coveredInterval.containsPhaseId(ctx.phaseId)) {
if (ctx.debug) ctx.traceInvalid(this)
if (ctx.debug || true) ctx.traceInvalid(this)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line looks strange.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catches, thanks!

Speed up construction and testing of base class sets.
Use a linear search with automatic bubbling up of
hits.
Adapt to either 2.11 or 2.12 scheme.
Don't mention ImplClasses in the name since they do not exist in the scheme for 2.12.
We really should use a blacklist instead. It's super-fragile to hardcode large directory listings like that.
@@ -70,26 +70,29 @@ object Mode {
/** We are currently unpickling Scala2 info */
val Scala2Unpickling = newMode(13, "Scala2Unpickling")

/** We are currently unpickling from Java 8 or higher */
val Java8Unpickling = newMode(14, "Java8Unpickling")
Copy link
Member

Choose a reason for hiding this comment

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

it would be nice to leave a note that it is used as heuristic to find scala 2.12

@DarkDimius
Copy link
Member

Otherwise LGTM.
As far as I understand, those tests don't actually test anything, as we are only using scala 2.11 in our CI

@DarkDimius DarkDimius merged commit 91cf397 into scala:master Jun 14, 2017
@allanrenucci allanrenucci deleted the add-scala12-mixins-v2 branch December 14, 2017 16:57
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.

None yet

4 participants