Skip to content

Commit

Permalink
Tighten closure extractor in TreeInfo
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 scala#21620. The new
definition is stricter. There is also a new blockEndingInclosure extractor that
keeps the old behavior.

Fixes scala#21620
  • Loading branch information
odersky committed Sep 20, 2024
1 parent 65a53b5 commit b410f30
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 ananonymous 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 b410f30

Please sign in to comment.