From ebb84900d9ffd454221bfe3f798ec2e285fc5c94 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 7 May 2025 09:16:52 -0700 Subject: [PATCH 1/4] Invent given pattern name in for comprehension --- .../src/dotty/tools/dotc/ast/Desugar.scala | 15 +++++----- tests/pos/i23119.scala | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 tests/pos/i23119.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 5112dccd2934..22b45e781a29 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -1653,18 +1653,19 @@ object desugar { * that refers to the bound variable for the pattern. Wildcard Binds are * also replaced by Binds with fresh names. */ - def makeIdPat(pat: Tree): (Tree, Ident) = pat match { - case bind @ Bind(name, pat1) => - if name == nme.WILDCARD then - val name = UniqueName.fresh() - (cpy.Bind(pat)(name, pat1).withMods(bind.mods), Ident(name)) - else (pat, Ident(name)) + def makeIdPat(pat: Tree): (Tree, Ident) = pat match + case pat @ Bind(nme.WILDCARD, body) => + val name = + body match + case Typed(Ident(nme.WILDCARD), tpt) if pat.mods.is(Given) => inventGivenName(tpt) + case _ => UniqueName.fresh() + (cpy.Bind(pat)(name, body).withMods(pat.mods), Ident(name)) + case Bind(name, _) => (pat, Ident(name)) case id: Ident if isVarPattern(id) && id.name != nme.WILDCARD => (id, id) case Typed(id: Ident, _) if isVarPattern(id) && id.name != nme.WILDCARD => (pat, id) case _ => val name = UniqueName.fresh() (Bind(name, pat), Ident(name)) - } /** Make a pattern filter: * rhs.withFilter { case pat => true case _ => false } diff --git a/tests/pos/i23119.scala b/tests/pos/i23119.scala new file mode 100644 index 000000000000..cd8026005447 --- /dev/null +++ b/tests/pos/i23119.scala @@ -0,0 +1,29 @@ +//> using options -Wunused:patvars -Werror + +def make: IndexedSeq[FalsePositive] = + for { + i <- 1 to 2 + given Int = i + fp = FalsePositive() + } yield fp + +def broken = + for + i <- List(42) + (x, y) = "hello" -> "world" + yield + s"$x, $y" * i + +def alt: IndexedSeq[FalsePositive] = + given String = "hi" + for + given Int <- 1 to 2 + j: Int = summon[Int] // simple assign because irrefutable + _ = j + 1 + k :: Nil = j :: Nil : @unchecked // pattern in one var + fp = FalsePositive(using k) + yield fp + +class FalsePositive(using Int): + def usage(): Unit = + println(summon[Int]) From e60936b0c41e64904c0f27924f320dbf51ce27f9 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 12 Sep 2025 19:03:11 +0200 Subject: [PATCH 2/4] Invent given pattern name in for comprehension [Cherry-picked b20b3382bc4ea5d4de8857201fe4fd608e31630a][modified] From a41af9de7b40d336c11e26d3d2313f31355c9c8b Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 29 Aug 2025 07:58:42 -0700 Subject: [PATCH 3/4] Status quo clashing of given names in for desugar [Cherry-picked 245a519ec97200d30ea7c492bf7b0935e1adeea7] --- tests/neg/i23119.check | 6 ++++++ tests/neg/i23119.scala | 13 +++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 tests/neg/i23119.check create mode 100644 tests/neg/i23119.scala diff --git a/tests/neg/i23119.check b/tests/neg/i23119.check new file mode 100644 index 000000000000..717199a610d0 --- /dev/null +++ b/tests/neg/i23119.check @@ -0,0 +1,6 @@ +-- [E161] Naming Error: tests/neg/i23119.scala:7:4 --------------------------------------------------------------------- +7 | given Option[List[Int]] = Some(List(x)) // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | given_Option_List is already defined as given instance given_Option_List + | + | Note that overloaded methods must all be defined in the same group of toplevel definitions diff --git a/tests/neg/i23119.scala b/tests/neg/i23119.scala new file mode 100644 index 000000000000..39e521313594 --- /dev/null +++ b/tests/neg/i23119.scala @@ -0,0 +1,13 @@ + +@main def test = println: + for x <- 1 to 2 + // works with explicit name + //ols @ given Option[List[String]] = Some(List(x.toString)) + given Option[List[String]] = Some(List(x.toString)) + given Option[List[Int]] = Some(List(x)) // error + yield summon[Option[List[String]]].map(ss => ss.corresponds(given_Option_List.get)((a, b) => a == b.toString)) + +// The naming clash is noticed when defining local values for "packaging": +// given_Option_List is already defined as given instance given_Option_List +// Previously the naming clash was noticed when extracting values in the map or do function: +// duplicate pattern variable: given_Option_List From 3100c7e03b083e855f7f3e4e8d271482dfb6c160 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 6 Sep 2025 09:48:19 -0700 Subject: [PATCH 4/4] Help renaming conflicting givens [Cherry-picked 87e434a82b02cd063a0ba190fac24bbd2fd21fd5] --- .../dotty/tools/dotc/reporting/messages.scala | 21 ++++++++++++++++++- tests/neg/i23119.check | 20 +++++++++++++++--- tests/neg/i23119.scala | 6 ++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 81ee47a3dc59..d7b9532e579f 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2087,8 +2087,27 @@ extends NamingMsg(AlreadyDefinedID): i" in ${conflicting.associatedFile}" else if conflicting.owner == owner then "" else i" in ${conflicting.owner}" + def print(tpe: Type): String = + def addParams(tpe: Type): List[String] = tpe match + case tpe: MethodType => + val s = if tpe.isContextualMethod then i"(${tpe.paramInfos}%, %) =>" else "" + s :: addParams(tpe.resType) + case tpe: PolyType => + i"[${tpe.paramNames}%, %] =>" :: addParams(tpe.resType) + case tpe => + i"$tpe" :: Nil + addParams(tpe).mkString(" ") def note = - if owner.is(Method) || conflicting.is(Method) then + if conflicting.is(Given) && name.startsWith("given_") then + i"""| + | + |Provide an explicit, unique name to given definitions, + |since the names assigned to anonymous givens may clash. For example: + | + | given myGiven: ${print(atPhase(typerPhase)(conflicting.info))} // define an instance + | given myGiven @ ${print(atPhase(typerPhase)(conflicting.info))} // as a pattern variable + |""" + else if owner.is(Method) || conflicting.is(Method) then "\n\nNote that overloaded methods must all be defined in the same group of toplevel definitions" else "" if conflicting.isTerm != name.isTermName then diff --git a/tests/neg/i23119.check b/tests/neg/i23119.check index 717199a610d0..34f9e3c564ae 100644 --- a/tests/neg/i23119.check +++ b/tests/neg/i23119.check @@ -1,6 +1,20 @@ --- [E161] Naming Error: tests/neg/i23119.scala:7:4 --------------------------------------------------------------------- -7 | given Option[List[Int]] = Some(List(x)) // error +-- [E161] Naming Error: tests/neg/i23119.scala:8:4 --------------------------------------------------------------------- +8 | given Option[List[Int]] = Some(List(x)) // error | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | given_Option_List is already defined as given instance given_Option_List | - | Note that overloaded methods must all be defined in the same group of toplevel definitions + | Provide an explicit, unique name to given definitions, + | since the names assigned to anonymous givens may clash. For example: + | + | given myGiven: Option[List[String]] // define an instance + | given myGiven @ Option[List[String]] // as a pattern variable +-- [E161] Naming Error: tests/neg/i23119.scala:18:8 -------------------------------------------------------------------- +18 | given [A] => List[A] = ??? // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | given_List_A is already defined as given instance given_List_A + | + | Provide an explicit, unique name to given definitions, + | since the names assigned to anonymous givens may clash. For example: + | + | given myGiven: [A] => List[A] // define an instance + | given myGiven @ [A] => List[A] // as a pattern variable diff --git a/tests/neg/i23119.scala b/tests/neg/i23119.scala index 39e521313594..0f882b66dc8d 100644 --- a/tests/neg/i23119.scala +++ b/tests/neg/i23119.scala @@ -1,3 +1,4 @@ +//> using options -explain @main def test = println: for x <- 1 to 2 @@ -11,3 +12,8 @@ // given_Option_List is already defined as given instance given_Option_List // Previously the naming clash was noticed when extracting values in the map or do function: // duplicate pattern variable: given_Option_List + +def also = + given [A] => List[A] = ??? + given [A] => List[A] = ??? // error + ()