A few thoughts on 2.12

[Simon Ochsenreither]

While reviewing the code base for Scala 2.12, I found a few things I'd like to bring up/discuss:

• Naked @deprecated. We really should just outlaw deprecated annotations without message/version. Nothing good comes from it, because it seems the warning already emitted is too easy to ignore. This is pretty annoying because it means we will be stuck with those elements for another few years. Example: https://github.com/scala/scala/blob/2.11.x/src/library/scala/collection/Iterator.scala#L169
• We are using exceptions for control flow in TraversableLike for core collection methods like isEmpty, forall, exists, takeWhile, copyToArray, etc. Additionally, we are creating a lot of superfluous classes here:
    def head: A = {
      var result: () => A = () => throw new NoSuchElementException
      breakable {
        for (x <- this) {
          result = () => x
          break
        }
      }
      result()
    }
  Maybe I'm missing something, but I think we could get rid of most of this stuff.
• Shall we go ahead an convert scala.Cloneable and scala.Serializable to simple type aliases for java.lang.Cloneable and java.io.Serializable? I remember that the current state has caused confusion and a few issues.
• The signature of the withDefault* methods is an issue. Basically, it just creates a wrapper around the original Map, which isn't an issue if the underlying Map doesn't provide a larger API. Especially with concurrent.TrieMap, this is not great, because if you want to have defaults, you can't access TrieMap's API anymore (which was likely the case why one wanted to use TrieMap in the first place).
• Also, it seems like TrieMap is exposing a lot of methods which are neither documented nor look like they should be public at all, like @volatile var root = r, def CAS_ROOT(ov: AnyRef, nv: AnyRef), def RDCSS_READ_ROOT(abort: Boolean = false).
• We should really look into getting rid of those "default" implementations in scala.collection/scala.collection.generic which can't be right for both mutable and immutable collections.
• We should start thinking about a better name for scala.collection.immutable.Stream, especially in the light of all the other existing and upcoming things named Stream, which do something completely different. Just a few weeks ago, a group I was working with fell into a huge performance trap (using Stream[Char] for parsing) due to the misleading naming.
• While working on Range, it seems like partest's MemoryTest uses some of the deprecated methods of Range, although I can't see it in the source. Either I'm not looking at the source, or other stuff (like optimizations, inlining, ...) is going on there:
  java.lang.NoSuchMethodError: scala.collection.immutable.Range.terminalElement()I
      at scala.tools.partest.MemoryTest.main(MemoryTest.scala:25)
• Scala 2.12 seems to use a pretty old STARR, 2.11.1 what's the plan here?
• Instead of keeping scala-actors shipping as a deprecated module, I'd prefer to get rid of it for 2.12, so all the stuff in the library which is more or less waiting for the end of scala-actors can finally be removed/deprecated.
• Imho, it would make sense to have some option for scaladoc to fail if a public method/class lacks documentation.
  

Opinions?

Thanks,

Simon

[Simon Ochsenreither]

• Can we come up with a solution for ClassTags and friends? Even if reflection as a whole is still experimental/might be obsoleted by scala.meta, I think it should be possible to at least declare the small part of *Tags as "supported", so that we can stop dragging *Manifests around.
  

[Som Snytt]

Thanks, I didn't even know about MemoryTest, although I'd spent some time recently looking at testing memory retention, including reflection.

I needed new eyeglasses anyway, but this time I'm definitely not getting the rose-colored fashion eyewear that comes with blinders made popular in a music video (where Lady Gaga was wearing them ironically, duh).

Wasn't "Naked deprecated" also a Lady Gaga song?

On Sat, Nov 29, 2014 at 9:52 PM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

• Can we come up with a solution for ClassTags and friends? Even if reflection as a whole is still experimental/might be obsoleted by scala.meta, I think it should be possible to at least declare the small part of *Tags as "supported", so that we can stop dragging *Manifests around.
  

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-interna...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Hanns Holger Rutz]

On 11/30/2014 06:42 AM, Simon Ochsenreither wrote:
> We are using exceptions for control flow in TraversableLike for core
> collection methods like isEmpty, forall, exists, takeWhile, copyToArray,
> etc. Additionally, we are creating a lot of superfluous classes here:
> def head: A = {
> var result: () => A = () => throw new NoSuchElementException
> breakable {
> for (x <- this) {
> result = () => x
> break
> }
> }
> result()
> }
> Maybe I'm missing something, but I think we could get rid of most of
> this stuff.

Wow that _is_ ugly (and as you say, probably inefficient due to the
object creations). So `TraversableLike` can only use `foreach`...

I know non-local-returns are, too, but perhaps a bit better:

def head: A = {
for (x <- this) return x
throw new NoSuchElementException
}

?

[Hanns Holger Rutz]

On 11/30/2014 06:42 AM, Simon Ochsenreither wrote:

> Imho, it would make sense to have some option for scaladoc to fail if a
> public method/class lacks documentation.

as an option, that would be great

[Rex Kerr]

On Sat, Nov 29, 2014 at 9:42 PM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

• We are using exceptions for control flow in TraversableLike for core collection methods like isEmpty, forall, exists, takeWhile, copyToArray, etc. Additionally, we are creating a lot of superfluous classes here:
    def head: A = {
      var result: () => A = () => throw new NoSuchElementException
      breakable {
        for (x <- this) {
          result = () => x
          break
        }
      }
      result()
    }
  Maybe I'm missing something, but I think we could get rid of most of this stuff.

There is no choice but to use exceptions for control flow, as all you have to work with is foreach, but there's no reason it can't be a standard return.  I also have noticed this and thought about fixing it all, but it seemed like the worst offenders were mostly overridden in subclasses, so I haven't yet.

Maybe the thing to do is file a ticket.  If you assign it to me, I'll make sure it all gets fixed (at least everything in TraversableLike).

 

• The signature of the withDefault* methods is an issue. Basically, it just creates a wrapper around the original Map, which isn't an issue if the underlying Map doesn't provide a larger API. Especially with concurrent.TrieMap, this is not great, because if you want to have defaults, you can't access TrieMap's API anymore (which was likely the case why one wanted to use TrieMap in the first place).

Indeed.  If we're trying to stay source-compatible in 2.12, we should leave it alone for now but for 2.13 I'd definitely like to clean that up.  Along with all the other places where you lose the collection you presumably chose for good reason.  (Anything with ordering tends to have problems.)

 

• Also, it seems like TrieMap is exposing a lot of methods which are neither documented nor look like they should be public at all, like @volatile var root = r, def CAS_ROOT(ov: AnyRef, nv: AnyRef), def RDCSS_READ_ROOT(abort: Boolean = false).

Let's get those deprecated as fast as we can.  Definitely should file a ticket on this one--I can't see how one can consider this anything but a bug.

 

• We should really look into getting rid of those "default" implementations in scala.collection/scala.collection.generic which can't be right for both mutable and immutable collections.

Did you have specific ones in mind?

 

• We should start thinking about a better name for scala.collection.immutable.Stream, especially in the light of all the other existing and upcoming things named Stream, which do something completely different. Just a few weeks ago, a group I was working with fell into a huge performance trap (using Stream[Char] for parsing) due to the misleading naming.

Ugh.  This is a tough one because Scala has had Stream for a long time.  But with Akka streams and Java Stream (which mean two different things), and various other "stream" ideas, the name is overused.  Do you have any ideas for a better name?  LazyList is accurate but rather inelegant.

 

• Imho, it would make sense to have some option for scaladoc to fail if a public method/class lacks documentation.
  

Sounds like a good idea for me as an option, but I think it would end up being too noisy in practice for most cases (e.g. incompletely documented inner objects used as enum values--what else could Red possibly mean in object Colors { sealed trait Color; object Red extends Color; ... }?  Because of worries about noise, I'm not sure how high a priority this should be.

Thanks for the overview--a lot of good points / questions!

  --Rex

[Rex Kerr]

On Sat, Nov 29, 2014 at 9:42 PM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

• We are using exceptions for control flow in TraversableLike for core collection methods like isEmpty, forall, exists, takeWhile, copyToArray, etc. Additionally, we are creating a lot of superfluous classes here:
    def head: A = {
      var result: () => A = () => throw new NoSuchElementException
      breakable {
        for (x <- this) {
          result = () => x
          break
        }
      }
      result()
    }
  Maybe I'm missing something, but I think we could get rid of most of this stuff.

There is no choice but to use exceptions for control flow, as all you have to work with is foreach, but there's no reason it can't be a standard return.  I also have noticed this and thought about fixing it all, but it seemed like the worst offenders were mostly overridden in subclasses, so I haven't yet.

Maybe the thing to do is file a ticket.  If you assign it to me, I'll make sure it all gets fixed (at least everything in TraversableLike).

 

• The signature of the withDefault* methods is an issue. Basically, it just creates a wrapper around the original Map, which isn't an issue if the underlying Map doesn't provide a larger API. Especially with concurrent.TrieMap, this is not great, because if you want to have defaults, you can't access TrieMap's API anymore (which was likely the case why one wanted to use TrieMap in the first place).

Indeed.  If we're trying to stay source-compatible in 2.12, we should leave it alone for now but for 2.13 I'd definitely like to clean that up.  Along with all the other places where you lose the collection you presumably chose for good reason.  (Anything with ordering tends to have problems.)

 

• Also, it seems like TrieMap is exposing a lot of methods which are neither documented nor look like they should be public at all, like @volatile var root = r, def CAS_ROOT(ov: AnyRef, nv: AnyRef), def RDCSS_READ_ROOT(abort: Boolean = false).

Let's get those deprecated as fast as we can.  Definitely should file a ticket on this one--I can't see how one can consider this anything but a bug.

 

• We should really look into getting rid of those "default" implementations in scala.collection/scala.collection.generic which can't be right for both mutable and immutable collections.

Did you have specific ones in mind?

 

• We should start thinking about a better name for scala.collection.immutable.Stream, especially in the light of all the other existing and upcoming things named Stream, which do something completely different. Just a few weeks ago, a group I was working with fell into a huge performance trap (using Stream[Char] for parsing) due to the misleading naming.

Ugh.  This is a tough one because Scala has had Stream for a long time.  But with Akka streams and Java Stream (which mean two different things), and various other "stream" ideas, the name is overused.  Do you have any ideas for a better name?  LazyList is accurate but rather inelegant.  "Creek" is a synonym for "Stream", but I doubt that would help anyone's comprehension (though we might induce people to name things "paddle" while traversing their Creek).

 

• Imho, it would make sense to have some option for scaladoc to fail if a public method/class lacks documentation.
  

[Rex Kerr]

(Sorry about the double-post.  Someone needs to teach gmail the difference between "at most once" and "at least once" when it comes to mail delivery on a flaky connection.)

[Simon Ochsenreither]

Ugh.  This is a tough one because Scala has had Stream for a long time.  But with Akka streams and Java Stream (which mean two different things), and various other "stream" ideas, the name is overused.  Do you have any ideas for a better name?  LazyList is accurate but rather inelegant.  "Creek" is a synonym for "Stream", but I doubt that would help anyone's comprehension (though we might induce people to name things "paddle" while traversing their Creek).

List of things which I think are important details necessary to understand Stream (sorted by importance):

1. Memoizing
2. List (in the sense of a Cons-like datastructure)
3. Lazy evaluation
4. Generator-like (in the sense that one can supply a function to compute the next element)

I'd probably go with something like MemoizingList or MemoizingGenerator (not really preferred because we haven't used Generator in scala.collections yet, and I'd try to avoid introducing more ad-hoc names for probably ad-hoc concepts).

[Jean-Rémi Desjardins]

I *strongly* prefer LazyList over MemoizingList. While we are at it can we just deprecate Stream and implement LazyList with a lazy head. Last time I checked, it was a generally accepted flaw with current Stream that only the tail is lazy.

--

[Simon Ochsenreither]

Also, here is a PR with the hopefully not contentious parts of the deprecations/removals for 2.12: https://github.com/scala/scala/pull/4177

[Simon Ochsenreither]

-[quick.library] warning: there were 134 deprecation warnings; re-run with -deprecation for details
+[quick.library] warning: there were 93 deprecation warnings; re-run with -deprecation for details
-[quick.reflect] warning: there were 61 deprecation warnings; re-run with -deprecation for details
+[quick.reflect] warning: there were 22 deprecation warnings; re-run with -deprecation for details
-[quick.compiler] warning: there were 175 deprecation warnings; re-run with -deprecation for details
+[quick.compiler] warning: there were 119 deprecation warnings; re-run with -deprecation for details
-[quick.repl] warning: there were 22 deprecation warnings; re-run with -deprecation for details
+[quick.repl] warning: there were 19 deprecation warnings; re-run with -deprecation for details
-[quick.partest-extras] warning: there was one deprecation warning; re-run with -deprecation for details

-Total time: 7 minutes 1 second
+Total time: 6 minutes 46 seconds

[Lukas Rytz]

On Sun, Nov 30, 2014 at 6:42 AM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

While reviewing the code base for Scala 2.12, I found a few things I'd like to bring up/discuss:

• Naked @deprecated. We really should just outlaw deprecated annotations without message/version. Nothing good comes from it, because it seems the warning already emitted is too easy to ignore. This is pretty annoying because it means we will be stuck with those elements for another few years. Example: https://github.com/scala/scala/blob/2.11.x/src/library/scala/collection/Iterator.scala#L169

It should work to

• remove the defaults from the constructor of deprecated
• add deprecated overloaded constructors to support to keep source compatibility
  

 

• We are using exceptions for control flow in TraversableLike for core collection methods like isEmpty, forall, exists, takeWhile, copyToArray, etc. Additionally, we are creating a lot of superfluous classes here:
    def head: A = {
      var result: () => A = () => throw new NoSuchElementException
      breakable {
        for (x <- this) {
          result = () => x
          break
        }
      }
      result()
    }
  Maybe I'm missing something, but I think we could get rid of most of this stuff.

Agree with Rex' answer. I don't see the optimizer being able to get rid of the Function0.

 

• Shall we go ahead an convert scala.Cloneable and scala.Serializable to simple type aliases for java.lang.Cloneable and java.io.Serializable? I remember that the current state has caused confusion and a few issues.

Phew, me neither.. For 2.12, the problem is that the change is not source compatible.

 

• Scala 2.12 seems to use a pretty old STARR, 2.11.1 what's the plan here?

This will be fixed once we get 2.12 milestones rolling, which should be not too far out.

Lukas

[Simon Ochsenreither]

It should work to

• remove the defaults from the constructor of deprecated
• add deprecated overloaded constructors to support to keep source compatibility

Yes, that should work! (Although I think this would be an opportunity for a @deprecatedDefault annotation ... :-D)
 

[Adriaan Moors]

Thanks for the great suggestions, Simon (doubly so, since they came with a PR)!

Several comments in this thread would make for great abide rules (e.g., public method should be documented).

Just to re-iterate: we want 2.11 and 2.12 to be source compatible including deprecated parts, so mostly we'll have to resort to deprecation to set the scene for the actual clean ups in 2.13, which will focus on just that (cleaning up the std lib).

--

[Simon Ochsenreither]

we want 2.11 and 2.12 to be source compatible including deprecated parts

Ah, right, wasn't aware of that anymore.
Makes sense because some of the deprecations in 2.11 were only half-done anyway.

[Simon Ochsenreither]

• It would be nice to start using the new class file attributes to save parameter names. (And especially start reading them from other class files, so that the Scala compiler doesn't have to do his x$1 thing when perfectly fine parameter names are available.
• Should we drop the private[Scala] modifier from deprecatedX annotations, so that everyone can start using them?
  

[Adriaan Moors]

On Wed, Dec 10, 2014 at 10:45 AM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

• It would be nice to start using the new class file attributes to save parameter names. (And especially start reading them from other class files, so that the Scala compiler doesn't have to do his x$1 thing when perfectly fine parameter names are available.

Agreed. Are you interested in working on this?

 

• Should we drop the private[Scala] modifier from deprecatedX annotations, so that everyone can start using them?

Yes, but we also need to spend sometime updating the docs etc.

[Simon Ochsenreither]

It would be nice to start using the new class file attributes to save parameter names. (And especially start reading them from other class files, so that the Scala compiler doesn't have to do his x$1 thing when perfectly fine parameter names are available.Agreed.

Are you interested in working on this?

Yes, that was kind of the plan, I'm too deep inside the annotation/attribute/classfile jungle anyway. :-)

[iulian dragos]

I'm not aware of any new classfile attributes for this purpose, what do you mean exactly? AFAIK, Scala already saves parameter names in pickle information and also in the standard LocalVariableTable (just like any other local variables). Synthetic names like x$1 are created for Java methods, so your proposal is to load LocalVariableTable contents for Java methods?

iulian

 

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-interna...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

[Adriaan Moors]

Support for this was added in Java 8: http://openjdk.java.net/jeps/118

[iulian dragos]

On Thu, Dec 11, 2014 at 6:46 PM, Adriaan Moors <adr...@typesafe.com> wrote:

Support for this was added in Java 8: http://openjdk.java.net/jeps/118

Ah, thanks, I checked the 1.7 specification. I agree it would be good to implement it.

[Simon Ochsenreither]

Additional things:

• Print message and version of @deprecated in the compiler warning
• Use a macro to implement the s string interpolator to allow usage where a constant literal is required (to achieve parity with String#+)
• Include som-snytt's regextractor string interpolator macro, which is totally amazing. (Example: https://github.com/soc/CFClient/blob/master/src/main/scala/cfclient/MessageParser.scala)
  

[Simon Ochsenreither]

Apart from that, another few things I'd like to get done (which only need some final effort):

• Fix the last remaining blockers for @enum (fails with "assignment to non variable", because the type of the AST of enum fields is set to a Constant type...)
• Real support for annotations (Jason helped me a lot here, but I'm still looking into an issue with emulating Java's implicit T=>Array[T] conversion)
• Turn classOf[T] into a macro and integrate with ClassTags (like new Array[T])
• Reduce the name mangling, see John Rose's "Symbolic Freedom" post (this has been discussed quite some time ago, with the concern that we might want to pick an additional symbol in addition to $ to recude the pain when creating class file names, but I think this can be easily taken into account)

Additionally, it might make sense to look into SI-2034 (especially with regard to the last point) and figure out what's the state of the attached patch.

[David Barri]

I'd love to see your @enum work make it into 2.12, Simon! I remember looking at it a year ago and thinking it was excellent.

[Simon Ochsenreither]

Me too, but I'm not sure how that will work. :-) The issue is that @enum depends on macro annotations, and those macros are not even on a roadmap as far as I know. I guess with all the ScalaMeta stuff coming up, it will be delayed for another couple of years.

So it either needs to be a separate library which can depend on macro paradise, or I would need to convert the code based on the external macro API into code using the internal compiler API.

But just like so many other things, I need to fix the last remaining issues with it so it starts working for good.

[Simon Ochsenreither]

Deprecate Stream: https://issues.scala-lang.org/browse/SI-9872