Open matwojcik opened 9 months ago
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.
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)
@i10416 Excellent analysis!
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.
Compiler version
3.3.1
Minimized code
Code from the gist:
or
Output
Expectation
I would expect that either:
fn1
andfn2
calls fail with missing argumentfn1
andfn2
calls warn withunused 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
then the result is:
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.