scala / scala3

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

"Wrong number of parameters" error in `for` expression under `-source: future` #14626

Closed griggt closed 2 years ago

griggt commented 2 years ago

Compiler version

3.1.2-RC1

Minimized code

Welcome to Scala 3.1.2-RC1 (1.8.0_292, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> :settings -source:future
scala> for (a, b) <- List("a","b","c").lazyZip(List(1,2,3)) do println(s"$a$b")

Output

-- [E086] Syntax Error: --------------------------------------------------------
1 |for (a, b) <- List("a","b","c").lazyZip(List(1,2,3)) do println(s"$a$b")
  |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |    Wrong number of parameters, expected: 2
1 error found

Expectation

a1
b2
c3

Noticed at https://github.com/lampepfl/dotty/pull/14294#issuecomment-1059905903

dwijnand commented 2 years ago

So LazyZip2 has a foreach method that takes a Tuple2: def foreach[U](f: (El1, El2) => U): Unit. But also has a conversion in implicit scope:

object LazyZip2 {
  implicit def lazyZip2ToIterable[El1, El2](zipped2: LazyZip2[El1, El2, _]): View[(El1, El2)] = zipped2.toIterable
}

So before -source:future it was using that to get elements of type Tuple2, which it can withFilter and foreach.

Reading the spec (SLS §6.19 For Comprehensions and For Loops) it says:

A generator p <- e produces bindings from an expression e which is matched in some way against pattern p.

Which it seems to stop doing under -source:future, by not applying the conversion. And then

A for loop for (p <- e) e′ is translated to e.foreach { case p => e′ }.

Which, again, isn't going to work with LazyZip2's foreach. Looking at Scala 2, it also converts when using lazyZip in a for comprehension.

@odersky Are for-comprehension in Scala 3 meant to desugar to using a Function2 foreach if available, or is it a bug that the implicit conversion isn't still used?

odersky commented 2 years ago

The difference comes from the fact that under source:future the for expression is treated as an irrefutable pattern match, so no withFilter is inserted. There's no special provision for for comprehensions to use Function2.

The type of the lazyZip is

scala.collection.LazyZip2[String, Int, ?1.CAP]

LazyZip2 does not have a withFilter method (I was tricked initially since ScalaDoc claims it has one, but that's actually added by the conversion, looking at the source shows there's no withFilter). So if the generator is filtering the conversion gets applied and the result then works for withFilter and foreach. But if the generator is non-filtering (i.e. no case) the withFilter is skipped and foreach is applied directly. Only, that foreach has the wrong arity. It's binary where a unary foreach was expected.

dwijnand commented 2 years ago

From the meeting: Martin is trying to fix the arity in desugaring, which means that we won't create tuples!

dwijnand commented 2 years ago

With the change in #14294, which makes this fail to compile without -source:future, the workaround is to switch from lazyZip to zip, because you were creating tuples anyways, or switch away from for-comprehensions (eg xs.lazyZip(ys).foreach((x, y) => ...).