Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
griggt opened this issue Mar 6, 2022 · 4 comments · Fixed by #14651
Closed

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

griggt opened this issue Mar 6, 2022 · 4 comments · Fixed by #14651
Assignees
Labels
area:desugar Desugaring happens after parsing but before typing, see desugar.scala itype:bug
Milestone

Comments

@griggt
Copy link
Contributor

griggt commented Mar 6, 2022

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 #14294 (comment)

@griggt griggt added itype:bug area:desugar Desugaring happens after parsing but before typing, see desugar.scala labels Mar 6, 2022
@griggt griggt self-assigned this Mar 6, 2022
griggt added a commit to griggt/dotty that referenced this issue Mar 6, 2022
griggt added a commit to griggt/dotty that referenced this issue Mar 7, 2022
Scratch that, it was causing the desugaring to include a withFilter
which I believe is undesirable here.

This reverts commit 9c26189.
griggt added a commit to griggt/dotty that referenced this issue Mar 7, 2022
@griggt griggt removed their assignment Mar 7, 2022
@dwijnand
Copy link
Member

dwijnand commented Mar 7, 2022

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
Copy link
Contributor

odersky commented Mar 7, 2022

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
Copy link
Member

dwijnand commented Mar 7, 2022

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

@dwijnand
Copy link
Member

dwijnand commented Mar 7, 2022

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) => ...).

odersky added a commit to dotty-staging/dotty that referenced this issue Mar 9, 2022
If a function argument is a synthetic term of the form

  x$1 => x$1 match case (a_1, ..., a_n) => e

and the expected type is an n-ary function type, rewrite the argument to

  (a_1, ..., a_n) => e

Fixes scala#14626.

The example in scala#14626 now compiles without an implicit tupling conversion.
Such a conversion was inserted before in 2.13, 3.0 and 3.1.
griggt added a commit to griggt/dotty that referenced this issue Mar 17, 2022
Martin's fix removes the need for this workaround

This reverts commit 36e0c29.
griggt added a commit to griggt/dotty that referenced this issue Apr 4, 2022
griggt added a commit to griggt/dotty that referenced this issue Apr 4, 2022
Scratch that, it was causing the desugaring to include a withFilter
which I believe is undesirable here.

This reverts commit 9c26189.
griggt added a commit to griggt/dotty that referenced this issue Apr 4, 2022
griggt added a commit to griggt/dotty that referenced this issue Apr 4, 2022
Martin's fix removes the need for this workaround

This reverts commit 36e0c29.
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:desugar Desugaring happens after parsing but before typing, see desugar.scala itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants