Skip to content

fix(#19266): better warning for impure synthetic lambda in stmt position #19540

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

fix(#19266): better warning for impure synthetic lambda in stmt position #19540

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c942f1f
draft:fix(#19266): add test cases
i10416 Jan 26, 2024
a953135
draft: better error for impure lambda in stmt position
i10416 Jan 26, 2024
8684632
warn impure synthetic lambda with in stmt position
i10416 Jan 26, 2024
ea306da
fix: tweak reporting
i10416 Jan 26, 2024
7330791
fix: false-positive closureDef match
i10416 Jan 27, 2024
4b3e82b
Merge branch 'master' into fix/19266-inconsistency-in--method-with-pa…
i10416 Feb 9, 2024
62c569a
Merge branch 'master' into fix/19266-inconsistency-in--method-with-pa…
i10416 Feb 24, 2024
19e8865
tidy: apply code review
i10416 Feb 24, 2024
f969d1a
Merge branch 'master' into fix/19266-inconsistency-in--method-with-pa…
i10416 Mar 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions tests/pos/i19266.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
object i19266:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note:

  def fn1(x: Int, y: String = "y")(z: Double) = Some(s"$x$y$z")
  def fn2(p: Int)(q: String) = Some(s"$p$q")

  def checkCompile =
    // This is NOT regarded as `isPureExpr`, but I think should be to warn `nonunit-statement`.
    // See https://github.com/lampepfl/dotty/issues/19266#issuecomment-1856248380 
    fn1(1)
    // This is regarded as `isPureExpr` and therefore it results in error.
    fn2(2)

Copy link
Contributor Author

@i10416 i10416 Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Block(
    List(
        ValDef(
            y$1,
            TypeTree[AnnotatedType(TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class scala)),object Predef),type String),ConcreteAnnotation(Apply(Select(New(TypeTree[TypeRef(ThisType(TypeRef(NoPrefix,module class unchecked)),class uncheckedVariance)]),<init>),List())))],
            Select(This(Ident(i19266$)),fn1$default$2)
        )
    ),
    Block(
        List(
            DefDef(
                $anonfun,
                List(List(ValDef(z,TypeTree[TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object scala),class Double)],EmptyTree))),
                TypeTree[AppliedType(TypeRef(ThisType(TypeRef(NoPrefix,module class scala)),class Some),List(TypeRef(ThisType(TypeRef(NoPrefix,module class lang)),class String)))],
                Apply(Apply(Ident(fn1),List(Literal(Constant(1)), Ident(y$1))),List(Ident(z)))
            )
        ),
        Closure(List(),Ident($anonfun),EmptyTree)
    )
)


Block(
    List(
        DefDef(
            $anonfun,
            List(List(ValDef(q,TypeTree[TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class scala)),object Predef),type String)],EmptyTree))),
            TypeTree[AppliedType(TypeRef(ThisType(TypeRef(NoPrefix,module class scala)),class Some),List(TypeRef(ThisType(TypeRef(NoPrefix,module class lang)),class String)))],
            Apply(Apply(Ident(fn2),List(Literal(Constant(2)))),List(Ident(q)))
        )
    ),
    Closure(List(),Ident($anonfun),EmptyTree)
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f1(1) is regarded as Block with ValDef statement and Block expr whereas f2(2) is regarded as Block with DefDef statement and Closure.

def fn1(x: Int, y: String = "y")(z: Double) = Some(s"$x$y$z")
def fn2(x: Int)(y: String) = Some(s"$x$y")

def checkCompile =
fn1(1)
Copy link
Contributor Author

@i10416 i10416 Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scala 2.13.12 with -Xsource:3 compiles with warnings; " a pure expression does nothing in statement position".

Scala 2.13.12 without -Xsource:3 raises errors for fn1(1), fn2(2) and fn2(3).

Unapplied methods are only converted to functions when a function type is expected.
You can make this conversion explicit by writing fn2 _ or fn2(_)(_) instead of fn2.

Copy link
Contributor Author

@i10416 i10416 Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the following to unapplied methods used as value

What characteristic allows us to assume unapplied methods are actually used as value?

https://github.com/lampepfl/dotty/blob/1716bcd9dbefbef88def848c09768a698b6b9ed9/compiler/src/dotty/tools/dotc/typer/Typer.scala#L3645

   *  (4) Do the following to unapplied methods used as values:
   *  (4.1) If the method has only implicit parameters pass implicit arguments
   *  (4.2) otherwise, if `pt` is a function type and method is not a constructor,
   *        convert to function by eta-expansion,
   *  (4.3) otherwise, if the method is nullary with a result type compatible to `pt`
   *        and it is not a constructor, apply it to ()
   *  otherwise issue an error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/lampepfl/dotty/blob/main/docs/_spec/06-expressions.md

Method Conversions

The following four implicit conversions can be applied to methods which are not applied to some argument list.

Evaluation

A parameterless method ´m´ of type => ´T´ is always converted to type ´T´ by evaluating the expression to which ´m´ is bound.

Implicit Application

If the method takes only implicit parameters, implicit arguments are passed following the rules here.

Eta Expansion

Otherwise, if the method is not a constructor, and the expected type ´\mathit{pt}´ is a function type, or, for methods of non-zero arity, a type sam-convertible to a function type, ´(\mathit{Ts}') \Rightarrow T'´, eta-expansion is performed on the expression ´e´.

(The exception for zero-arity methods is to avoid surprises due to unexpected sam conversion.)

Empty Application

Otherwise, if ´e´ has method type ´()T´, it is implicitly applied to the empty argument list, yielding ´e()´.

Copy link
Contributor Author

@i10416 i10416 Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the following example m can be partially applied to the first two parameters. Assigning m to f1 will automatically eta-expand

def m(x: Boolean, y: String)(z: Int): List[Int]
val f1 = m
val f2 = m(true, "abc")

(The emphasis is added by me)
What should happen if m is not assigned to value?

https://dotty.epfl.ch/docs/reference/changed-features/eta-expansion-spec.html#automatic-eta-expansion-and-partial-application-1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for myself.
https://github.com/lampepfl/dotty/blob/1716bcd9dbefbef88def848c09768a698b6b9ed9/compiler/src/dotty/tools/dotc/typer/Typer.scala#L4411

// This should compile with warning
fn2(2)
val _ = fn2(3)
8 changes: 8 additions & 0 deletions tests/warn/i19266.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
object i19266:
def fn1(x: Int, y: String = "y")(z: Double) = Some(s"$x$y$z")
def fn2(x: Int)(y: String) = Some(s"$x$y")

def checkWarning =
fn1(1) // warn
fn2(2) // warn
val a = fn2(3) // warn