- Notifications
You must be signed in to change notification settings - Fork3.1k
Make the CharSequence wrappers in Predef non-implicit, for JDK 15#9292
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
In JDK 15 CharSequence has an isEmpty method with a defaultimplementation, which clashes with our Array[Char]#isEmpty,IndexedSeq[Char]#isEmpty, as well as our StringBuilder and reflect'sName.
smarter commentedOct 29, 2020
isEmpty is defined directly on Seq, so I don't think that an implicit conversion also defining isEmpty leads to a clash. |
dwijnand commentedOct 29, 2020
I added it mostly to get rid of the other way CharSequence, which we don't control, can break us. |
SethTisue commentedOct 29, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
do we want to deprecate these and suggest people write in the |
smarter commentedOct 29, 2020
Bit of a mixed message withhttp://dotty.epfl.ch/docs/reference/other-new-features/creator-applications.html :)
Yep that's what I suggested in the previous PR too. |
dwijnand commentedOct 30, 2020
Just I agree to deprecating |
SethTisue commentedOct 30, 2020
community build results for 2.13.4-bin-0230f94-SNAPSHOT are in. my conclusion: this stuff is rarely used. it is okay to go ahead with this. details follow:
scala-parser-combinators needed changes to 3 call sites in 3 different source files:scala/scala-parser-combinators#306 — it is using scalatest was just using I had to fix those (and, separately, scalatest-3-0) in order to get a usable run. The remaining failures are: the twirl failure is actually about that the exhaustivity changes and not about this. so that leaves scalafix and scoverage in scalafix, four call sites in a single source file is affected; in scoverage, two call sites in a single source file this all seems well within the range of acceptability to me |
smarter commentedNov 5, 2020
One thing that I don't think has been discussed: should this be backported to 2.12? Or perhaps the original idea of hacking implicit search would be more appropriate there? Or Java 15+ should just be declared unsupported? (Might be problematic given that Java 17 is not that far away and will be an LTS release). |
dwijnand commentedNov 5, 2020
Yeah, we'll want to backport this to 2.12, though it's not as critical as 2.13.4 for it to be in 2.12.13, IMO. PR's welcome. |
lrytz commentedNov 5, 2020
I agree.
A good replacement would be
That wasn't done in the end, it seems? |
2.13.4 brings JDK 15 support withscala/scala#9292
2.13.4 brings JDK 15 support withscala/scala#9292
2.13.4 brings JDK 15 support withscala/scala#9292- tests/run/t8153.scala was removed like it was removed in Scala 2 withscala/scala@8f6e522- tests/neg/missing-implicit-2.check was updated because the implicitNotFound message was changed in 2.13.4- tests/run/t6260-delambdafy.scala had to be changed to behave the same under JDK <= 15 and JDK 15.
2.13.4 brings JDK 15 support withscala/scala#9292Also upgrade Scala.js to 1.3.1 so that the testcases we import from itcompile with JDK 15.- tests/run/t8153.scala was removed like it was removed in Scala 2 withscala/scala@8f6e522- tests/neg/missing-implicit-2.check was updated because the implicitNotFound message was changed in 2.13.4- tests/run/t6260-delambdafy.scala had to be changed to behave the same under JDK <= 15 and JDK 15.
This requires a single source code change to handlescala/scala#9292.
2.13.4 brings JDK 15 support withscala/scala#9292- tests/run/t8153.scala was removed like it was removed in Scala 2 withscala/scala@8f6e522- tests/neg/missing-implicit-2.check was updated because the implicitNotFound message was changed in 2.13.4- tests/run/t6260-delambdafy.scala had to be changed to behave the same under JDK <= 15 and JDK 15.
2.13.4 brings JDK 15 support withscala/scala#9292- tests/run/t8153.scala was removed like it was removed in Scala 2 withscala/scala@8f6e522- tests/neg/missing-implicit-2.check was updated because the implicitNotFound message was changed in 2.13.4- tests/run/t6260-delambdafy.scala had to be changed to behave the same under JDK <= 15 and JDK 15.
2.13.4 brings JDK 15 support withscala/scala#9292- tests/run/t8153.scala was removed like it was removed in Scala 2 withscala/scala@8f6e522- tests/neg/missing-implicit-2.check was updated because the implicitNotFound message was changed in 2.13.4- tests/run/t6260-delambdafy.scala had to be changed to behave the same under JDK <= 15 and JDK 15.
2.13.4 brings JDK 15 support withscala/scala#9292- tests/run/t8153.scala was removed like it was removed in Scala 2 withscala/scala@8f6e522- tests/neg/missing-implicit-2.check was updated because the implicitNotFound message was changed in 2.13.4- tests/run/t6260-delambdafy.scala had to be changed to behave the same under JDK <= 15 and JDK 15.
2.13.4 brings JDK 15 support withscala/scala#9292- tests/run/t8153.scala was removed like it was removed in Scala 2 withscala/scala@8f6e522- tests/neg/missing-implicit-2.check was updated because the implicitNotFound message was changed in 2.13.4- tests/run/t6260-delambdafy.scala had to be changed to behave the same under JDK <= 15 and JDK 15.
retronym commentedDec 10, 2020
Another way the source incompatibility can be experienced, for the record. |
Problem=======Upgrade source tohttps://github.com/scala/scala/releases/tag/v2.12.13 fromhttps://github.com/scala/scala/releases/tag/v2.12.12. This upgrade includes a highlighted feature of configurable warnings and errors (https://www.scala-lang.org/2021/01/12/configuring-and-suppressing-warnings.html /scala/scala#9248).Other noticable changes our code based will experience* Exhaustivity warnings for patterns involving tuples* Better type checking for `CharSequence` argumentsscala/scala#9292* Simplified Reportersscala/scala#8338* Promotion of `-deprecation` to `-Xlint:deprecation`scala/scala#7714* Improves performance of building `immutable.{TreeMap,TreeSet}` by using mutation within the builderscala/scala#8794Full list of changes can be found athttps://github.com/scala/scala/pulls?q=is%3Amerged+milestone%3A2.12.13+Solution========* Bump `BUILD` file `SCALA_REV` && `SCALAC_REV_FOR_DEPS`.* Depdnencies which are not built for scala 2.12.13 needed to be bumped `SEMANTICDB_PLUGIN_REV` && `SEMANTICDB_REV` && `SCALAFIX_REV`.* Include `-S-Xlint:-deprecation` to `pants.ini` preventing build failures on deprecated annotations (existing behavior)* Bump `cache_key_gen_version` in `pants.ini` for newly built artifacts on different scala version.* Removed scalafix plugin (`scalac-profiling`) which is not built for 2.12.13. `scalac-profiling` code looks abandoned by ~3 years.* Updated all failing tests that could have depended or expected ordered sequences when the sequence was generated from non ordered collections.Notes=====It seems a few tests and golden files in source have depended or tested against a stable sort order when sorted order is not guaranteed. This has been seen in a few places such as output json objects (map key fields), code that uses `groupBy` for rekeying results to the user and transforming into sequences, strings built from sequences or maps that are not guaranteed to be ordered.The following PR are related to this change in expectationsscala/scala#8794scala/scala#8948scala/scala#9218scala/scala#9376scala/scala#9091scala/scala#9216We took the liberty updating tests with what the current map order would be or updating the test in a way that wouldn't depend on order. Since we may not fully understand all the context of the tests we are hoping this either signals to the owners that there might be some issue with assumed order or signal that upgrading might break implementations due to bug in scala 2.12.13. {D611202} will improve the files changed for workflow builder outside of this change.Please feel to reach out directly and discuss the changes here or bring up issues with this upgrade. Slack [[https://app.slack.com/client/T86S8GHEG/C01NZAFRLFK|#scala-upgrade-2-12-13]]JIRA Issues: SCALA-25Differential Revision:https://phabricator.twitter.biz/D607826
…-CharSequenceMake the CharSequence wrappers in Predef non-implicit, for JDK 15
… JDK 15In JDK 15 CharSequence has an isEmpty method with a defaultimplementation, which clashes with our Array[Char]#isEmpty,IndexedSeq[Char]#isEmpty, as well as our StringBuilder and reflect'sName.Backport ofscala/scala#9292 from 2.13.x to 2.12.x
as per discussion onscala/scala#9292
…-CharSequenceMake the CharSequence wrappers in Predef non-implicit, for JDK 15
… JDK 15In JDK 15 CharSequence has an isEmpty method with a defaultimplementation, which clashes with our Array[Char]#isEmpty,IndexedSeq[Char]#isEmpty, as well as our StringBuilder and reflect'sName.Backport ofscala/scala#9292 from 2.13.x to 2.12.x
as per discussion onscala/scala#9292
Uh oh!
There was an error while loading.Please reload this page.
This change is binary compatible, but not source compatible. Users will need to update callsites to explicitly convert to
CharSequence, since the conversions are no longer implicit.ArrayCharSequenceis no longer needed and will be deprecated in 2.12.13 and 2.13.5. Instead, usejava.nio.CharBuffer.wrapto convert anArray[Char]to aCharSequence.SeqCharSequenceremains useful, but now must be explicitly constructed.Why was this necessary? In JDK 15 CharSequence has an isEmpty method with a default implementation, which clashes with our Array[Char]#isEmpty, IndexedSeq[Char]#isEmpty, as well as our StringBuilder and reflect's Name.
Fixesscala/bug#12172