Prepare for 2.13 library modularization [ci: last-only] #5677
Conversation
9f0a931 to
b58b8fd
Compare
|
Deprecations
|
|
For clarity, "to their own modules" means: moved out of the repo, switch groupId to org.scala-lang.modules and new versions, right? Like XML and parser-combinators? Or is it more conservative than that? |
|
Yes, for most of these, I'd want them to evolve faster than the core of the
library, or, if that's not possible, they should make way for community
modules of higher quality (such as better-files).
…On Tue, Feb 7, 2017 at 09:33 Dale Wijnand ***@***.***> wrote:
For clarity, "to their own modules" means: moved out of the repo, switch
groupId to org.scala-lang.modules and new versions, right? Like XML and
parser-combinators? Or is it more conservative than that?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#5677 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFjywEDff9tazkV38Ij2A0VwXUNo0cKks5raKr7gaJpZM4L47ZD>
.
|
|
Do you expect the same treatment to be applied to the collections library, either for 2.13 or for some hypothetical later release? |
|
No. The collections are core to the language. We will keep reducing the
coupling between the spec/compiler and the collections, but they will
always be part of the core. Doing otherwise would fragment the language
because then you would have two implementations of, say, List and Iterable.
…On Wed, Feb 8, 2017 at 06:56 Alec Zorab ***@***.***> wrote:
Do you expect the same treatment to be applied to the collections library,
either for 2.13 or for some hypothetical later release?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#5677 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFjy5tE-USoLsuOOIZXY-IWMy5hdWTRks5radeBgaJpZM4L47ZD>
.
|
|
My recollection (which is likely wrong, I looked at this a long time ago)
was that the only "collection" types referred to specifically in the spec
itself were `Option` (for `unapply`) and `Seq` (`unapplySeq`, possibly
others? ) . Since the addition of name based pattern matching, it seems
like references to `Option` could be dropped, which would leave just `Seq`.
My extremely rough searching (on a phone through a pdf of the 2.9 spec)
found no references to `Iterable` nor `List` in the spec outside of
examples.
It would be a shame to forgo the potential increase in modularity, given
that the collections library is being reworked. Indeed, if `Seq` lost its
place of privilege in the spec then it would be possible to offer both the
legacy collections library and the reworked one as individual, separate
modules which would no doubt substantially ease the lives of people
migrating to 2.13
|
|
I agree that the coupling should be reduced, as I tried to acknowledge in my message. However, the more important point is that, in my opinion, it is crucial for Scala adoption that there be only one Scala collections API. Library interop crucially relies on agreeing on many of the core collection types, even if the compiler and spec could be reworked not to rely on our specific implementations (using a name-based approach as with for comprehensions). I'm happy to see extensions built on top of the collections, but wholesale replacement is not feasible for a widely used library (again, in my opinion). |
|
can we discuss this elsewhere, e.g. on http://contributors.scala-lang.org? it isn't directly relevant to this pull request, which is more limited in scope. |
|
I'd suggest that exactly that did happen for actors, but I'll certainly concede that multiple collection libraries might result in some level of inconvenience for people needing to go between libraries depending on different libs. As Seth suggests, I'll reserve further comment for another venue |
|
(Testing out scala/scabot#68, but something seems off -- it did work at #5662!) |
|
review by @SethTisue |
| @@ -27,7 +27,7 @@ trait ScalaSettings extends AbsScalaSettings | |||
| * defaults to the value of CLASSPATH env var if it is set, as in Java, | |||
| * or else to `"."` for the current user directory. | |||
| */ | |||
| protected def defaultClasspath = sys.env.getOrElse("CLASSPATH", ".") | |||
| protected def defaultClasspath = Option(System.getenv("CLASSPATH")).getOrElse(".") | |||
There was a problem hiding this comment.
There is scala.util.Properties.{envOrElse, envOrNone} for that :)
There was a problem hiding this comment.
As Sébastien said, this is one of the ties I'd like to cut.
There was a problem hiding this comment.
I understand the goal of this PR is to cut dependencies, so this comment is out of scope: I'm in favor of shipping this kind of convenience (envOrElse) with Scala (in a form that makes it easily available, I understand we want to provide an opt-out).
| @@ -1,7 +1,7 @@ | |||
| package scala.reflect.macros | |||
| package compiler | |||
|
|
|||
| import scala.compat.Platform.EOL | |||
| import java.lang.System.{lineSeparator => EOL} | |||
There was a problem hiding this comment.
There already was Properties.lineSeparator too, though it doesn't really add anything at all.
There was a problem hiding this comment.
As Sébastien said, this is one of the ties I'd like to cut.
| val budgetProp = scala.sys.Prop[String]("scalac.patmat.analysisBudget") | ||
| if (budgetProp.isSet) { | ||
| reportWarning(s"Please remove -D${budgetProp.key}, it is ignored.") | ||
| if (System.getProperty("scalac.patmat.analysisBudget") != null) { |
There was a problem hiding this comment.
That's where all the Properties.{propOrXXX, propIsSet, ...} methods are useful. I hadn't even realized it was all duplicated in sys.props until I saw it being used in the REPL.
There was a problem hiding this comment.
The scala.sys.Prop is a bit over-designed for my tastes.
There was a problem hiding this comment.
I would just re-read the source whenever I needed to use it.
|
@gourlaysama From the PR description, IIUC, the idea is also not to rely on |
|
Ah, never mind, I hadn't seen that I would argue for moving |
|
The stuff read from Besides, even though system properties overall do work in Scala.js, the only reason it is supported is because of JUnit. They shouldn't otherwise be used, IMO. |
Agreed. This is very much the goal here: the core should be easily portable across platforms. (Of course, as in scala.js and scala-native, parts of the Java stdlib will have to be reimplemented on those platforms.) |
Eh? It isn't. I've responded at some length over at https://contributors.scala-lang.org/t/modularization-of-scala-2-13-standard-library/635 |
What about these:
|
I think it makes sense to remove these.
Honestly, I've used this so much, and I've seen so many people relying on it, that I'm a little bit reluctant on moving this away. I think it should stay. |
|
One thing I never understood was the history of About I think we really need to keep those core things in the core library. Are we going to have an "util" module? Sounds a bit like a dumping ground :) But some util things are quite useful, and we want to provide the "batteries included" experience (with easy to get modules).
|
|
Keeping However, I'm not sure if we should leave these in the core even if Scalac depends on them:
This functionality seems auxiliary. It's not clear if we should keep them because:
That being said, I think that a high-level discussion on the contents of the core would be beneficial. It's obvious to me that collections are necessary, but utilities that are not as visible as collections should be picked with care and a clear intention. Should we raise this discussion in the next SIP meeting? We can probably identify common ground between the two compilers and find the most urgent needs that could benefit all Scala developers. I don't know if this makes sense, so I think it would be useful to bring it up to debate. /cc @odersky @adriaanm @SethTisue @sjrd @xeno-by @heathermiller @dragos @jsuereth. |
| @@ -45,6 +45,7 @@ package object sys { | |||
| * | |||
| * @return the result of `java.lang.Runtime.getRuntime()` | |||
| */ | |||
| @deprecated("use Runtime.getRuntime", "2.12.2") | |||
There was a problem hiding this comment.
as discussed (half?) offline, i think this pr should not deprecate things that will move out to separate modules. it would be better to define those modules first, carve them out, and then evolve the codebase there, hopefully with new contributors and energy :)
I won't copy this comment to other deprecations introduced in this PR, they are easy to find.
| @@ -27,7 +27,7 @@ trait ScalaSettings extends AbsScalaSettings | |||
| * defaults to the value of CLASSPATH env var if it is set, as in Java, | |||
| * or else to `"."` for the current user directory. | |||
| */ | |||
| protected def defaultClasspath = sys.env.getOrElse("CLASSPATH", ".") | |||
| protected def defaultClasspath = Option(System.getenv("CLASSPATH")).getOrElse(".") | |||
There was a problem hiding this comment.
I understand the goal of this PR is to cut dependencies, so this comment is out of scope: I'm in favor of shipping this kind of convenience (envOrElse) with Scala (in a form that makes it easily available, I understand we want to provide an opt-out).
|
now in #5830 |
|
@nafg pointed out on the contributors forum that if |
|
A lot of ADTs follow the convention that the subtypes are put inside the
companion, so technically one option is to do Try.Success / Try.Failure...
…On Mon, Dec 18, 2017 at 7:23 PM Seth Tisue ***@***.***> wrote:
@nafg <https://github.com/nafg> pointed out on Discourse that if Try
moves up to scala.Try, Success and Failure would move with them, which
seems questionable
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5677 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGAUAA2YFMIT8xd_qxQ56GQODj6WAA2ks5tBwH9gaJpZM4L47ZD>
.
|
|
Current Should we apply the some rules to these ADTs (put their subtypes inside the companion), or just treat some special? |
|
I'm not sure the fate of regex was fixed? It would nice if support were in a form like xml, where the std lib doesn't depend on an impl, but usages of I hope somebody actually takes a big pair of scissors and cuts Adriaan's tie when this is all complete. |
|
we've run out of time in the 2.13 cycle — let's pick this up again for 2.14 |
In 2.13, the following packages will move out of scala-library
(to their own module):
This commit cuts some ties to prepare for this move.
To compensate for the associated deprecations in your own code:
compat.Platform.EOL--> addimport java.lang.System.{lineSeparator => EOL}compat.Platform.currentTime--> addimport java.lang.System.{currentTimeMillis => currentTime}compat.Platform.arraycopy--> addimport java.lang.System.arraycopysys.error-->throw new RuntimeException(or subclass)sys.exit-->System.exitsys.props-->System.getProperties().asScala(withimport scala.collection.JavaConverters._)sys.env-->System.getenv().asScalasys.runtime-->Runtime.getRuntimesys.addShutdownHook-->Runtime.getRuntime.addShutdownHook(new Thread(() => ...))Console.BOLD-->AnsiColor.BOLD(withimport scala.io.AnsiColor)Console.readLine-->Console.in.readLine()concurrent.Lock-->java.util.concurrent.locks.ReentrantLock(lock/unlock)TODO
method this(java.lang.String)Unit in class scala.sys.ShutdownHookThreadjava.security.AccessControlException: access denied ("java.util.PropertyPermission" "scala.control.noTraceSuppression" "read")