Skip to content

Commit

Permalink
Tighten closure extractor in TreeInfo (#21621)
Browse files Browse the repository at this point in the history
The previous extractor for closures matches also arbitrary blocks that
ended in a (possible deeply nested) closure. This caused wrong use sets
in #21620. The new definition is stricter. There is also a new
blockEndingInclosure extractor that keeps the old behavior.

Fixes #21620
  • Loading branch information
odersky authored Sep 22, 2024
2 parents 4293a45 + 614314a commit 22ed2fb
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 6 deletions.
24 changes: 19 additions & 5 deletions compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -814,17 +814,31 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
case _ => false
}

/** An extractor for closures, either contained in a block or standalone.
/** An extractor for closures, possibly typed, and possibly including the
* definition of the anonymous def.
*/
object closure {
def unapply(tree: Tree): Option[(List[Tree], Tree, Tree)] = tree match {
case Block(_, expr) => unapply(expr)
case Closure(env, meth, tpt) => Some(env, meth, tpt)
case Typed(expr, _) => unapply(expr)
def unapply(tree: Tree)(using Context): Option[(List[Tree], Tree, Tree)] = tree match {
case Block((meth : DefDef) :: Nil, closure: Closure) if meth.symbol == closure.meth.symbol =>
unapply(closure)
case Block(Nil, expr) =>
unapply(expr)
case Closure(env, meth, tpt) =>
Some(env, meth, tpt)
case Typed(expr, _) =>
unapply(expr)
case _ => None
}
}

/** An extractor for a closure or a block ending in one. This was
* previously `closure` before that one was tightened.
*/
object blockEndingInClosure:
def unapply(tree: Tree)(using Context): Option[(List[Tree], Tree, Tree)] = tree match
case Block(_, expr) => unapply(expr)
case _ => closure.unapply(tree)

/** An extractor for def of a closure contained the block of the closure. */
object closureDef {
def unapply(tree: Tree)(using Context): Option[DefDef] = tree match {
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Migrations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ trait Migrations:
val nestedCtx = ctx.fresh.setNewTyperState()
val res = typed(qual, pt1)(using nestedCtx)
res match {
case closure(_, _, _) =>
case blockEndingInClosure(_, _, _) =>
case _ =>
val recovered = typed(qual)(using ctx.fresh.setExploreTyperState())
val msg = OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen, tree)
Expand Down
13 changes: 13 additions & 0 deletions tests/neg-custom-args/captures/i21620.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
-- [E129] Potential Issue Warning: tests/neg-custom-args/captures/i21620.scala:5:6 -------------------------------------
5 | x
| ^
| A pure expression does nothing in statement position
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/i21620.scala:9:31 ----------------------------------------
9 | val _: () -> () ->{x} Unit = f // error
| ^
| Found: () ->{f} () ->{x} Unit
| Required: () -> () ->{x} Unit
|
| longer explanation available when compiling with `-explain`
10 changes: 10 additions & 0 deletions tests/neg-custom-args/captures/i21620.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class C
def test(x: C^) =
val f = () =>
def foo() =
x
()
println(s"hey: $x")
() => foo()
val _: () -> () ->{x} Unit = f // error
()
11 changes: 11 additions & 0 deletions tests/pos-custom-args/captures/i21620.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class C
def test(x: C^) =
def foo() =
x
()
val f = () =>
// println() // uncomenting would give an error, but with
// a different way of handling curried functions should be OK
() => foo()
val _: () -> () ->{x} Unit = f
()

0 comments on commit 22ed2fb

Please sign in to comment.