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

Switch to the 2.13 standard library #7019

Merged
merged 88 commits into from
Aug 25, 2019

Conversation

smarter
Copy link
Member

@smarter smarter commented Aug 8, 2019

Still to do:

As soon as this PR is merged we should publish a new nightly and use it as the reference compiler, so that we don't spend too much time with a build that uses both the 2.12 and 2.13 stdlib (it works, but it can be confusing and cumbersome).

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

  1. Separate subject from body with a blank line
  2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
  3. Limit the subject line to 72 characters
  4. Capitalize the subject line
  5. Do not end the subject line with a period
  6. Use the imperative mood in the subject line ("Add" instead of "Added")
  7. Wrap the body at 80 characters
  8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

@smarter
Copy link
Member Author

smarter commented Aug 8, 2019

@bishabosha I'll look into the failing test cases, meanwhile, feel free to help with any of the other items in the todo list above.

@smarter
Copy link
Member Author

smarter commented Aug 8, 2019

On the CI the community build fails to build every project with:

module not found: ch.epfl.lamp#sbt-dotty;0.3.4-SNAPSHOT

But I can't reproduce that locally. Weird, but this doesn't prevent us from working on it locally at least.

@smarter
Copy link
Member Author

smarter commented Aug 8, 2019

@milessabin There's (at least) one compilation error for shapeless on 2.13:

[error] -- [E007] Type Mismatch Error: /home/smarter/opt/dotty/community-build/community-projects/shapeless/core/src/main/scala/shapeless/shapeless.scala:64:51
[error] 64 |      WrappedArray.make[String](summonValuesAsArray[mirror.MirroredElemLabels])
[error]    |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |                                Found:    Array[Any]
[error]    |                                Required: Array[String]
[error] one error found

If you have the time, it'd be great if you could take a look at it.

@bishabosha
Copy link
Member

now I see why there is 😱 for deprecation

@milessabin
Copy link
Contributor

@smarter you should be good after #7020.

@smarter
Copy link
Member Author

smarter commented Aug 9, 2019

@milessabin There's still an error

[error] -- [E007] Type Mismatch Error: /home/smarter/opt/dotty/community-build/community-projects/shapeless/core/src/main/scala/shapeless/shapeless.scala:65:31
[error] 65 |      WrappedArray.make[String](summonValuesAsArray[mirror.MirroredElemLabels, String])
[error]    |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |      Found:    scala.collection.mutable.ArraySeq[String]
[error]    |      Required: Seq[String]

This can be solved by adding a .toSeq to get an immutable Seq, but there might be a nicer way. Note that you don't necessarily need to keep compiling against both the 2.12 and 2.13 stdlib, I'm happy with a 2.13-only git branch.

@milessabin
Copy link
Contributor

@smarter I'll add the .toSeq for now and switch over to 2.13-only once this PR is merged.

@milessabin
Copy link
Contributor

Done in #7021.

Copy link
Member Author

@smarter smarter left a comment

Choose a reason for hiding this comment

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

@bishabosha Would be good to add some test cases for the hashcode changes.

@smarter
Copy link
Member Author

smarter commented Aug 9, 2019

Would be good to add some test cases for the hashcode changes.

Actually we already have a related test that should be updated: https://github.com/lampepfl/dotty/blob/master/tests/run/caseClassHash.scala https://github.com/lampepfl/dotty/blob/master/tests/run/caseClassHash.check

@bishabosha
Copy link
Member

with the new update ## and MurmurHash produce the same value

@milessabin
Copy link
Contributor

@bishabosha what's with the shapeless bump? That's already merged on master ... wouldn't it be easier to rebase?

@smarter
Copy link
Member Author

smarter commented Aug 14, 2019

@milessabin Rebased with your patch in.

@smarter
Copy link
Member Author

smarter commented Aug 14, 2019

@liufengyun Unfortunately, https://github.com/liufengyun/bench/tree/master/tests/scala-library does not compile anymore now that the compiler runs using the 2.13 stdlib. I tried to hack it to make it compile, but it seems pretty hard, so I think we'll have to remove it from the benchmarks, and replace it by the new stdlib.

The value of type Unit is `()`, using `Unit` usually also works because
it refers to the companion object of the class Unit, and because of
automatic `()` insertion when the expected type is `Unit`, but it's
still wrong. 2.13 marked it as `@compileTimeOnly` which is how I noticed.
This is easier to reason about, in particular because it avoids having
to think about what kind of classloader sbt decided to inflict upon us.
This fixes running the community-build test after switching to the 2.13
stdlib in the bootstrapped build.
This matches the behavior of Scala 2, this is necessary when using the
2.13 stdlib where scala.Serializable is a type alias of
java.lang.Serializable, otherwise the compiler will disallow extending
both Serializable and AnyVal.
This required some surgery in Build.scala: when compiling the
non-bootstrapped compiler, we need the 2.12 stdlib on the JVM and
compiler classpaths, because the current reference compiler cannot be
run using the 2.13 stdlib, on the other hand when _running_ the
non-bootstrapped compiler we need to the 2.13 stdlib on the compiler
classpath to produce a bootstrapped compiler that links against 2.13.

Upgrade the Scala 2 PickleFormat version to 5.2 (which is in fact
identical to 5.0).

Remove no longer needed -Ynew-collections flag and update the
related test files.
Replace getExternalDep and findLib by findArtifact, which is safer than
getLib was (it doesn't use `contains` to find the name which might yield
false positives).
smarter and others added 11 commits August 23, 2019 16:09
productElementName is no longer generated in Desugar after the previous
commit.
It doesn't output an error anymore for some reason, but that's fine: we
only care about it not crashing the compiler, which is what fuzzy tests
check.
This fixes a pickling/unpickling difference in sip23-valueof.scala and
makes sense in general.
Needs to be replaced by something that doesn't use the parallel
collections, I attempted to do so in
b9ba16e but the result caused
deadlocks, don't have time to investigate right now.
I accidentally replaced `"` by `'` in the generated .dotty-ide.json file
in 2ae438d which broke everything.
We now need to dealias deeply to get tests/run/Course-2002-13.scala and
tests/run/enrich-gentraversable.scala to pass.
`membersBasedOnFlags` internally uses `memberNames` which returns a Set,
so the order of its elements is unstable. We were using it when
generating static forwarder methods which lead to some idempotency tests
failing (no idea why this didn't manifest itself before). Fixed by
adding a new `sortedMembersBasedOnFlags` which sorts members by names
and signatures (this required adding a somewhat arbitrary lexicographic
ordering to Signature).
@smarter smarter force-pushed the upgrade/scalac-2.13 branch 2 times, most recently from dfbb87a to 1ba7e75 Compare August 25, 2019 12:00
bishabosha and others added 10 commits August 25, 2019 15:42
So far, Jar was defined as an Iterable implementation that overrides
`foreach` and then define `override def iterator = this.toList.iterator`
That was OK with the 2.12 stdlib, but stackoverflows with the 2.13 one
due to `toList` now being implemented in terms of `iterator`.

Any way, it turns out that we don't really need Jar to be an Iterable,
so the simplest fix was to just remove these methods, and add back a
`toList` for the one usesite that needed it (not very efficient, but not
any worse than before).
Not needed with 2.13 where CanBuildFrom is mostly gone.
Originally fixed in f8a746e, lost in
b9ba16e when tests were updated based
on their version in scala/scala.
To pick up the improvements since the previous one.
And the revert the CI changes from the previous commit which are no
longer needed.
@bishabosha
Copy link
Member

🔥🔥🔥

@smarter smarter merged commit 38e87b0 into scala:master Aug 25, 2019
@smarter smarter deleted the upgrade/scalac-2.13 branch August 25, 2019 15:16
@anatoliykmetyuk anatoliykmetyuk added this to the 0.18 Tech Preview milestone Aug 28, 2019
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.

Mix the productPrefix.hashCode into the hash code of case classes, like Scala 2.13.0-RC1
7 participants