search

scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.81k stars 1.05k forks source link

iterableOnceExtensionMethods is being inserted unnecessarily, causing deprecation warnings #14244

Open nafg opened 2 years ago

nafg commented 2 years ago

(accidentally filed first at https://github.com/scala/bug/issues/12521)

Compiler version

3.0.2 and 3.1.0 (it works in 2.13.7)

Minimized code

(SBT / REPL session with -Xprint:typer provided below)

import scala.scalajs.js
def d = js.Dictionary.empty[Int]                                                                                                                                                                                                                     
def e = d.map(_.toString)
def f = js.Any.wrapDictionary(d).map(_.toString)

Output (REPL)

def d: scala.scalajs.js.Dictionary[Int]

-- Deprecation Warning:
1 |def e = d.map(_.toString)
  |        ^^^^^
  |method map in class IterableOnceExtensionMethods is deprecated since 2.13.0: Use .iterator.map instead or consider requiring an Iterable
def e: IterableOnce[String]

def f: scala.collection.mutable.Iterable[String]
Edited SBT / REPL session with -Xprint:typer ``` sbt:simpleFacade> set Compile/consoleQuick/scalacOptions += "-Xprint:typer" [info] Defining Compile / consoleQuick / scalacOptions [info] The new value will be used by Compile / consoleQuick [info] Reapplying settings... [info] set current project to simpleFacade (in build file:/home/naftoli/dev/github.com/nafg/scalajs-facades/) sbt:simpleFacade> ++ 3.1.0 [info] Setting Scala version to 3.1.0 on 11 projects. [info] Reapplying settings... [info] set current project to simpleFacade (in build file:/home/naftoli/dev/github.com/nafg/scalajs-facades/) sbt:simpleFacade> consoleQuick ``` ```scala Welcome to Scala 3.1.0 (17.0.1, Java OpenJDK 64-Bit Server VM). Type in expressions for evaluation. Or try :help. scala> import scala.scalajs.js -- Info: [[syntax trees at end of typer]] // rs$line$1 package repl { final lazy module val rs$line$1: repl.rs$line$1 = new repl.rs$line$1() final module class rs$line$1() extends Object() { this: repl.rs$line$1.type => import scala.scalajs.js } } scala> def d = js.Dictionary.empty[Int] -- Info: [[syntax trees at end of typer]] // rs$line$2 package repl { final lazy module val rs$line$2: repl.rs$line$2 = new repl.rs$line$2() final module class rs$line$2() extends Object() { this: repl.rs$line$2.type => def d: scala.scalajs.js.Dictionary[Int] = scalajs.js.Dictionary.empty[Int] } } def d: scala.scalajs.js.Dictionary[Int] scala> def e = d.map(_.toString) -- Info: [[syntax trees at end of typer]] // rs$line$3 package repl { final lazy module val rs$line$3: repl.rs$line$3 = new repl.rs$line$3() final module class rs$line$3() extends Object() { this: repl.rs$line$3.type => def e: IterableOnce[String] = scala.collection.IterableOnce.iterableOnceExtensionMethods[(String, Int)]( scala.scalajs.js.Any.wrapDictionary[Int](d) ).map[String]( { def $anonfun(_$1: (String, Int)): String = _$1.toString() closure($anonfun) } ) } } -- Deprecation Warning: 1 |def e = d.map(_.toString) | ^^^^^ |method map in class IterableOnceExtensionMethods is deprecated since 2.13.0: Use .iterator.map instead or consider requiring an Iterable def e: IterableOnce[String] scala> def f = js.Any.wrapDictionary(d).map(_.toString) -- Info: [[syntax trees at end of typer]] // rs$line$4 package repl { final lazy module val rs$line$4: repl.rs$line$4 = new repl.rs$line$4() final module class rs$line$4() extends Object() { this: repl.rs$line$4.type => def f: scala.collection.mutable.Iterable[String] = scalajs.js.Any.wrapDictionary[Int](d).map[String]( { def $anonfun(_$1: (String, Int)): String = _$1.toString() closure($anonfun) } ) } } def f: scala.collection.mutable.Iterable[String] ```

Expectation

There should be no warning for e, just like for f and just like e in Scala 2.12

As you can see in the REPL session, once wrapDictionary is inserted there is no need for iterableOnceExtensionMethods. Yet when the compiler inserts the former it somehow inserts the latter too. This results in the warnings like you can see when defining e.

Also, if instead of .map(_.toString) I pass certain other functions I get different results. For instance x => x and _.swap do not produce a warning or involve iterableOnceExtensionMethods. I think the pattern is that if it's producing a Map (returning a Tuple2) it is okay and if it returns something else it warns.

SethTisue commented 2 years ago

This is interesting and peculiar.

Is Scala.js essential here, is there any reproduction that doesn't involve it?

dwijnand commented 2 years ago

I tried to reproduce this, but it seems like the warning isn't there using Scala 3.1.0:

$ scala-cli compile -S 3.0.2 --js i14244.scala
Compiling project (Scala 3.0.2, Scala.JS)
[warn] ./i14244.scala:4:9: method map in class IterableOnceExtensionMethods is deprecated since 2.13.0: Use .iterator.map instead or consider requiring an Iterable
[warn] def e = d.map(_.toString)
[warn]         ^^^^^
Compiled project (Scala 3.0.2, Scala.JS)
$ scala-cli clean i14244.scala
$ scala-cli compile -S 3.0.2 --js i14244.scala
Compiling project (Scala 3.0.2, Scala.JS)
[warn] ./i14244.scala:4:9: method map in class IterableOnceExtensionMethods is deprecated since 2.13.0: Use .iterator.map instead or consider requiring an Iterable
[warn] def e = d.map(_.toString)
[warn]         ^^^^^
Compiled project (Scala 3.0.2, Scala.JS)
$ scala-cli clean i14244.scala
$ scala-cli compile -S 3.1.0 --js i14244.scala
Compiling project (Scala 3.1.0, Scala.JS)
Compiled project (Scala 3.1.0, Scala.JS)
$ scala-cli clean i14244.scala
$ scala-cli compile -S 3.1.0 --js i14244.scala
Compiling project (Scala 3.1.0, Scala.JS)
Compiled project (Scala 3.1.0, Scala.JS)

Same using the REPL (via scala-cli repl).

nafg commented 2 years ago

Weird, because as you can see if you expand the SBT session, I demonstrated it with 3.1.0

SethTisue commented 2 years ago

Yeah. I don't know what to make of that.

som-snytt commented 2 years ago

I think nafg is close to diagnosing it. I'm under the weather with the "booster flu", but I'll try to put down what I noticed, just looking into scala-js a bit.

WrappedDictionary has a special map for scala-new-collections

def map[B](f: ((String, A)) => (String, B)): js.WrappedDictionary[B]

where normally a mutable.Map will see

scala> val m = collection.mutable.Map("two"->2)
val m: scala.collection.mutable.Map[String,Int] = HashMap(two -> 2)

def map[K2, V2](f: ((String, Int)) => (K2, V2)): scala.collection.mutable.Map[K2,V2]
def map[B](f: ((String, Int)) => B): scala.collection.mutable.Iterable[B]

scala> m.map(_.toString)
val res0: scala.collection.mutable.Iterable[String] = ArrayBuffer((two,2))

scala> m.map(p => (p._1, p._2.toString))
val res1: scala.collection.mutable.Map[String,String] = HashMap(two -> 2)

I see LowPrioAnyImplicits offers to wrap a dictionary or a Map, where the dictionary conversion is more specific

implicit def wrapDictionary[A](dict: js.Dictionary[A]): js.WrappedDictionary[A] =
    new js.WrappedDictionary(dict)

and LowestPrioAnyImplicits

implicit def iterableOps[A](iterable: js.Iterable[A]): js.IterableOps[A] =
    new js.IterableOps(iterable)

The deprecations extensions take IterableOnce[A] and offer

def map[B](f: A => B): IterableOnce[B]
som-snytt commented 2 years ago

I haven't tried to minimize or reproduce, I was just looking idly at the moving parts, or rather, the non-spinning parts disassembled on the floor of the garage.

som-snytt commented 2 years ago

I wasn't able to reproduce on my starter project, it's been several years since I tried scala-js at work.

I was able to reproduce from nafg's project, following the steps closely.

The thing consoleQuick doesn't tell you is

[warn] Scala REPL doesn't work with Scala.js. You are running a JVM REPL. JavaScript things won't work.

I don't know how it may be tweaking the environment, but I assume that is the difference in compilation.

sbt:js-something> compile
[info]     def e: scala.collection.mutable.Iterable[String] =
[info]       scala.scalajs.js.Any.wrapDictionary[Int](Something.d).map[String](
[info]         {
[info]           def $anonfun(_$1: (String, Int)): String = _$1.toString()
[info]           closure($anonfun)
[info]         }
[info]       )

but 

sbt:js-something> consoleQuick
bad option '-Xlint' was ignored
Welcome to Scala 3.1.0 (17.0.1, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> :load src/main/scala/Something.scala

      def e: IterableOnce[String] =
        scala.collection.IterableOnce.iterableOnceExtensionMethods[(String, Int)
          ]
        (scala.scalajs.js.Any.wrapDictionary[Int](Something.d)).map[String](
          {
            def $anonfun(_$1: (String, Int)): String = _$1.toString()
            closure($anonfun)
          }
        )

This is a good example of why folks need an honest REPL.

nafg commented 2 years ago

The original issue was not in the repl. I only went to the repl because if you add -Xprint:typer to scalacOptions besides that it thinks I want to recompile EVERYTHING from scratch, you get the trees from everything and it's hard to find the relevant part. And that allowed me to try two Scala 3 versions too without recompiling.

On Thu, Jan 13, 2022, 6:15 PM som-snytt @.***> wrote:

I wasn't able to reproduce on my starter project, it's been several years since I tried scala-js at work.

I was able to reproduce from nafg's project, following the steps closely.

The thing consoleQuick doesn't tell you is

[warn] Scala REPL doesn't work with Scala.js. You are running a JVM REPL. JavaScript things won't work.

I don't know how it may be tweaking the environment, but I assume that is the difference in compilation.

sbt:js-something> compile [info] def e: scala.collection.mutable.Iterable[String] = [info] scala.scalajs.js.Any.wrapDictionaryInt.map[String]( [info] { [info] def $anonfun($1: (String, Int)): String = $1.toString() [info] closure($anonfun) [info] } [info] )

but

sbt:js-something> consoleQuick bad option '-Xlint' was ignored Welcome to Scala 3.1.0 (17.0.1, Java OpenJDK 64-Bit Server VM). Type in expressions for evaluation. Or try :help.

scala> :load src/main/scala/Something.scala

  def e: IterableOnce[String] =
    scala.collection.IterableOnce.iterableOnceExtensionMethods[(String, Int)
      ]
    (scala.scalajs.js.Any.wrapDictionary[Int](Something.d)).map[String](
      {
        def $anonfun(_$1: (String, Int)): String = _$1.toString()
        closure($anonfun)
      }
    )

This is a good example of why folks need an honest REPL.

— Reply to this email directly, view it on GitHub https://github.com/lampepfl/dotty/issues/14244#issuecomment-1012604852, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAYAUGLKK75CBNA3JXHHSDUV5MI5ANCNFSM5LUXJMGA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

som-snytt commented 2 years ago

I can't speak to the original issue, but I showed that it happens in 3.1.0 in JVM REPL but not in normal compilation.

I get two warnings under 3.0.2.

[warn] -- Deprecation Warning: .../src/main/scala/Something.scala:7:12
[warn] 7 |  def e = d.map(_.toString)
[warn]   |          ^^^^^
[warn]   |method map in class IterableOnceExtensionMethods is deprecated since 2.13.0: Use .iterator.map instead or consider requiring an Iterable
[warn] -- Deprecation Warning: .../src/main/scala/Something.scala:8:35
[warn] 8 |  def f = js.Any.wrapDictionary(d).map(_.toString)
[warn]   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warn]   |method map in class IterableOnceExtensionMethods is deprecated since 2.13.0: Use .iterator.map instead or consider requiring an Iterable
[warn] two warnings found

I haven't started using scala-cli yet.

nafg commented 2 years ago

Theory: maybe in 3.0.2 it's both in files and in the REPL, and in 3.1.0 it's only an issue in the REPL. Because my codebase was on 3.0.2 and I don't think I tried recompiling on 3.1.0 (that's why I did consoleQuick). That would resolve the contradictory observations in this thread.

dwijnand commented 2 years ago

I like the theory. It doesn't explain why I didn't see it in the scala-cli REPL, but there could be (silly) differences there too, I guess.