Skip to content

Commit 69e97e9

Browse files
authored
Invent given pattern name in for comprehension (#23121)
Fixes #23119 A given pattern in a for comprehension results in a fresh val in the body of the mapping function, but it should have the same (arbitrary) name as in the rest of the expansion. This commit gives the given its usual given name (in `makeIdPat`) so that the unused check can check it. Currently, without `-preview`, showing that `$1$` is called `given_Int` by typer: ``` ➜ scala-cli repl --server=false -S 3.7.2-RC1 -Vprint:typer,refchecks -Wunused:all Welcome to Scala 3.7.2-RC1 (17.0.15, Java OpenJDK 64-Bit Server VM). Type in expressions for evaluation. Or try :help. scala> for given Int <- 1 to 2; j: Int = summon[Int] yield j 1 warning found [[syntax trees at end of typer]] // rs$line$1 package <empty> { final lazy module val rs$line$1: rs$line$1 = new rs$line$1() final module class rs$line$1() extends Object() { this: rs$line$1.type => val res0: IndexedSeq[Int] = intWrapper(1).to(2).map[(Int, Int)]((x$1: Int) => x$1:Int @unchecked match { case given $1$ @ _:Int => val j: Int = $1$ Tuple2.apply[Int, Int]($1$, j) } ).map[Int]((x$1: (Int, Int)) => x$1:(x$1 : (Int, Int)) @unchecked match { case Tuple2.unapply[Int, Int](given given_Int @ _:Int, j @ _:Int) => j:Int } ) } } ``` Without `-preview`, this commit: ``` scala> for given Int <- 1 to 2; j: Int = summon[Int] yield j [[syntax trees at end of typer]] // rs$line$1 package <empty> { final lazy module val rs$line$1: rs$line$1 = new rs$line$1() final module class rs$line$1() extends Object() { this: rs$line$1.type => val res0: IndexedSeq[Int] = intWrapper(1).to(2).map[(Int, Int)]((x$1: Int) => x$1:Int @unchecked match { case given given_Int @ _:Int => val j: Int = given_Int Tuple2.apply[Int, Int](given_Int, j) } ).map[Int]((x$1: (Int, Int)) => x$1:(x$1 : (Int, Int)) @unchecked match { case Tuple2.unapply[Int, Int](given given_Int @ _:Int, j @ _:Int) => j:Int } ) } } ``` With `-preview`, the tupling map is eliminated early: ``` scala> for given Int <- 1 to 2; j: Int = summon[Int] yield j [[syntax trees at end of typer]] // rs$line$1 package <empty> { final lazy module val rs$line$1: rs$line$1 = new rs$line$1() final module class rs$line$1() extends Object() { this: rs$line$1.type => val res0: IndexedSeq[Int] = intWrapper(1).to(2).map[Int]((x$1: Int) => x$1:Int @unchecked match { case given given_Int @ _:Int => val j: Int = given_Int j:Int } ) } } ```
2 parents 7cbadac + 87e434a commit 69e97e9

File tree

5 files changed

+97
-9
lines changed

5 files changed

+97
-9
lines changed

compiler/src/dotty/tools/dotc/ast/Desugar.scala

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,7 +1347,7 @@ object desugar {
13471347
)).withSpan(tree.span)
13481348
end makePolyFunctionType
13491349

1350-
/** Invent a name for an anonympus given of type or template `impl`. */
1350+
/** Invent a name for an anonymous given of type or template `impl`. */
13511351
def inventGivenName(impl: Tree)(using Context): SimpleName =
13521352
val str = impl match
13531353
case impl: Template =>
@@ -2136,18 +2136,19 @@ object desugar {
21362136
* that refers to the bound variable for the pattern. Wildcard Binds are
21372137
* also replaced by Binds with fresh names.
21382138
*/
2139-
def makeIdPat(pat: Tree): (Tree, Ident) = pat match {
2140-
case bind @ Bind(name, pat1) =>
2141-
if name == nme.WILDCARD then
2142-
val name = UniqueName.fresh()
2143-
(cpy.Bind(pat)(name, pat1).withMods(bind.mods), Ident(name))
2144-
else (pat, Ident(name))
2139+
def makeIdPat(pat: Tree): (Tree, Ident) = pat match
2140+
case pat @ Bind(nme.WILDCARD, body) =>
2141+
val name =
2142+
body match
2143+
case Typed(Ident(nme.WILDCARD), tpt) if pat.mods.is(Given) => inventGivenName(tpt)
2144+
case _ => UniqueName.fresh()
2145+
(cpy.Bind(pat)(name, body).withMods(pat.mods), Ident(name))
2146+
case Bind(name, _) => (pat, Ident(name))
21452147
case id: Ident if isVarPattern(id) && id.name != nme.WILDCARD => (id, id)
21462148
case Typed(id: Ident, _) if isVarPattern(id) && id.name != nme.WILDCARD => (pat, id)
21472149
case _ =>
21482150
val name = UniqueName.fresh()
21492151
(Bind(name, pat), Ident(name))
2150-
}
21512152

21522153
/** Make a pattern filter:
21532154
* rhs.withFilter { case pat => true case _ => false }

compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2125,8 +2125,27 @@ extends NamingMsg(AlreadyDefinedID):
21252125
i" in ${conflicting.associatedFile}"
21262126
else if conflicting.owner == owner then ""
21272127
else i" in ${conflicting.owner}"
2128+
def print(tpe: Type): String =
2129+
def addParams(tpe: Type): List[String] = tpe match
2130+
case tpe: MethodType =>
2131+
val s = if tpe.isContextualMethod then i"(${tpe.paramInfos}%, %) =>" else ""
2132+
s :: addParams(tpe.resType)
2133+
case tpe: PolyType =>
2134+
i"[${tpe.paramNames}%, %] =>" :: addParams(tpe.resType)
2135+
case tpe =>
2136+
i"$tpe" :: Nil
2137+
addParams(tpe).mkString(" ")
21282138
def note =
2129-
if owner.is(Method) || conflicting.is(Method) then
2139+
if conflicting.is(Given) && name.startsWith("given_") then
2140+
i"""|
2141+
|
2142+
|Provide an explicit, unique name to given definitions,
2143+
|since the names assigned to anonymous givens may clash. For example:
2144+
|
2145+
| given myGiven: ${print(atPhase(typerPhase)(conflicting.info))} // define an instance
2146+
| given myGiven @ ${print(atPhase(typerPhase)(conflicting.info))} // as a pattern variable
2147+
|"""
2148+
else if owner.is(Method) || conflicting.is(Method) then
21302149
"\n\nNote that overloaded methods must all be defined in the same group of toplevel definitions"
21312150
else ""
21322151
if conflicting.isTerm != name.isTermName then

tests/neg/i23119.check

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
-- [E161] Naming Error: tests/neg/i23119.scala:8:4 ---------------------------------------------------------------------
2+
8 | given Option[List[Int]] = Some(List(x)) // error
3+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4+
| given_Option_List is already defined as given instance given_Option_List
5+
|
6+
| Provide an explicit, unique name to given definitions,
7+
| since the names assigned to anonymous givens may clash. For example:
8+
|
9+
| given myGiven: Option[List[String]] // define an instance
10+
| given myGiven @ Option[List[String]] // as a pattern variable
11+
-- [E161] Naming Error: tests/neg/i23119.scala:18:8 --------------------------------------------------------------------
12+
18 | given [A] => List[A] = ??? // error
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
| given_List_A is already defined as given instance given_List_A
15+
|
16+
| Provide an explicit, unique name to given definitions,
17+
| since the names assigned to anonymous givens may clash. For example:
18+
|
19+
| given myGiven: [A] => List[A] // define an instance
20+
| given myGiven @ [A] => List[A] // as a pattern variable

tests/neg/i23119.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//> using options -explain
2+
3+
@main def test = println:
4+
for x <- 1 to 2
5+
// works with explicit name
6+
//ols @ given Option[List[String]] = Some(List(x.toString))
7+
given Option[List[String]] = Some(List(x.toString))
8+
given Option[List[Int]] = Some(List(x)) // error
9+
yield summon[Option[List[String]]].map(ss => ss.corresponds(given_Option_List.get)((a, b) => a == b.toString))
10+
11+
// The naming clash is noticed when defining local values for "packaging":
12+
// given_Option_List is already defined as given instance given_Option_List
13+
// Previously the naming clash was noticed when extracting values in the map or do function:
14+
// duplicate pattern variable: given_Option_List
15+
16+
def also =
17+
given [A] => List[A] = ???
18+
given [A] => List[A] = ??? // error
19+
()

tests/pos/i23119.scala

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//> using options -Wunused:patvars -Werror
2+
3+
def make: IndexedSeq[FalsePositive] =
4+
for {
5+
i <- 1 to 2
6+
given Int = i
7+
fp = FalsePositive()
8+
} yield fp
9+
10+
def broken =
11+
for
12+
i <- List(42)
13+
(x, y) = "hello" -> "world"
14+
yield
15+
s"$x, $y" * i
16+
17+
def alt: IndexedSeq[FalsePositive] =
18+
given String = "hi"
19+
for
20+
given Int <- 1 to 2
21+
j: Int = summon[Int] // simple assign because irrefutable
22+
_ = j + 1
23+
k :: Nil = j :: Nil : @unchecked // pattern in one var
24+
fp = FalsePositive(using k)
25+
yield fp
26+
27+
class FalsePositive(using Int):
28+
def usage(): Unit =
29+
println(summon[Int])

0 commit comments

Comments
 (0)