Undeterministic eta expansion for a method with default arguments

[matwojcik]

Compiler version

3.3.1

Minimized code

Code from the gist:

  def fn1(x: Int, y: Int = 5)(z: Int) = Some(x+y+z)
  def fn2(x: Int)(y: Int) = Some(x+y)

  def program = 
    fn1(4) // why this compiles
    fn2(2) // when this does not
    5

scala-cli compile Unused.scala -Wnonunit-statement

or

scala-cli compile https://gist.github.com/matwojcik/85c0527502871b0a672441728601afc9 -Wnonunit-statement

Output

[error] missing argument list for value of type Int => Some[Int]
[error]     fn2(2) // when this does not

Expectation

I would expect that either:

• both fn1 and fn2 calls fail with missing argument
• both fn1 and fn2 calls warn with unused value of type Int => Some[Int] (add : Unit to discard silently)

If you adapt this code to scala 2.13 (add object - see https://gist.github.com/matwojcik/55e9e27c9581f136dcd3266d1b4a48ab) and run

scala-cli compile --scala 2.13 -Xsource:3 [Unused.scala](https://gist.github.com/matwojcik/55e9e27c9581f136dcd3266d1b4a48ab) -Wnonunit-statement

then the result is:

[warn] ./Unused213.scala:8:5
[warn] unused value of type Int => Some[Int] (add `: Unit` to discard silently)
[warn]     fn2(2) // when this does not
[warn]     ^^^^^^
[warn] ./Unused213.scala:7:5
[warn] unused value of type Int => Some[Int] (add `: Unit` to discard silently)
[warn]     fn1(4) // why this compiles
[warn]     ^^^^^^

So something that I expected in the first place - warning about non used value. Scala3 though is apparently doing eta expansion not in the deterministic way here - when the function is having a default parameters.

[som-snytt]

The expansion looks normal -Vprint, so maybe it's a spurious error during type check? that is corrected by eta expansion.

The other note is if you comment out the erroneous line, -Wnonunit-statement does not notice the unused closure in Scala 3. I don't know if there is a ticket for that, but it sounds familiar.

[i10416]

I think the difference in your example is intentional.

  def fn1(x: Int, y: Int = 5)(z: Int) = Some(x+y+z)
  def fn2(x: Int)(y: Int) = Some(x+y)

  def program = 
    fn1(4) // why this compiles
    fn2(2) // when this does not
    5

Default value for by-value parameter may have side-effect on eta-expansion, so it is reasonable not to warn " A pure expression does nothing in statement position" and not to raise error. Perhaps, we should warn NonUnitUnusedValue.

On the other hand, method without default argument and method with default arguments for by-name parameters are pure. It does not perform side-effect on eta-expansion. Pure synthetic lambda in statement position is prohibited according to https://github.com/lampepfl/dotty/pull/11769. [^1]

[^1]: It says "Disallow lambdas in statement position", but it prohibits only pure lambdas in statement position.

It seems expected behavior to raise error rather than warning.

The difference is, for example, in the following code block, fn1(1) mutates arr, but f2(2) won't.

import scala.collection.mutable.ArrayBuffer
import scala.util.Random

val arr = ArrayBuffer[Int]()

def fn0(x: Int)(y: String) = Some(s"$x,$y")
def fn1(x: Int, y: Unit = arr.addOne(Random.nextInt()))(z:Double) = Some(s"$x,$y,$z")
def fn2(x: Int, y: => Unit = arr.addOne(Random.nextInt()))(z:Double) = Some(s"$x,$y,$z")

// This is pure and falls into the missing argument arm
// here https://github.com/lampepfl/dotty/blob/1716bcd9dbefbef88def848c09768a698b6b9ed9/tests/pos-with-compiler-cc/dotc/typer/Typer.scala#L4203
// Thus, it shouldn't compile.
fn0(0)
// This is impure(perform side-effect to mutate `arr`). Therefore this passes through `checkStatementPurity`.
fn1(1)
// This is pure and falls into the missing argument arm 
// https://github.com/lampepfl/dotty/blob/1716bcd9dbefbef88def848c09768a698b6b9ed9/tests/pos-with-compiler-cc/dotc/typer/Typer.scala#L4203
// Thus, it shouldn't compile.
fn2(2)

[odersky]

@i10416 Excellent analysis!

[som-snytt]

Per my previous comment, I would expect -Wnonunit-statement to warn about the nowarn cases. Oh I guess that's what the PR intends to remedy.

class C {
  def i = "42".toInt
  def f(x: Int, y: Int = 42)(z: Int) = x+y+z
  def test: Int = {
    f(27)       // nowarn bc side-effect; but also no nonunit in statement pos
    f(27, i)    // nowarn bc side-effect; but also no nonunit in statement pos
    f(27, 42)   // missing args bc synthetic
    f(27)(_)    // warn pure in statement pos
    f(27, i)(_) // warn pure in statement pos
    5
  }
}

The experience on the previous "disallow synthetic lambdas" PR, that test frameworks like weird code, happened again for Wnonunit-statement where ScalaTest assert breaks the check in user code.