From 1ad34d5b4da00040553c5f5ab25474d87c8c44ed Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Thu, 3 Jul 2025 12:33:03 +0000 Subject: [PATCH 1/2] Add an explainer to the DoubleDefinition error --- .../dotty/tools/dotc/reporting/messages.scala | 48 +++++++++++++++++-- tests/neg/doubleDefinition.check | 36 ++++++++++++++ tests/neg/exports.check | 8 ++++ tests/neg/i14966a.check | 2 + tests/neg/i19809.check | 2 + tests/neg/i23402.check | 44 +++++++++++++++-- tests/neg/i23402.scala | 2 + tests/neg/i23402b.check | 2 + tests/neg/i23402c.check | 2 + tests/neg/i23402d.check | 12 +++++ tests/neg/i23402d.scala | 5 ++ tests/neg/into-override.check | 6 +++ tests/neg/override-erasure-clash.check | 2 + tests/neg/type-params.check | 2 + 14 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 tests/neg/i23402d.check create mode 100644 tests/neg/i23402d.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index f69dfea0007a..4974884f8286 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2339,14 +2339,16 @@ class SymbolIsNotAValue(symbol: Symbol)(using Context) extends TypeMsg(SymbolIsN class DoubleDefinition(decl: Symbol, previousDecl: Symbol, base: Symbol)(using Context) extends NamingMsg(DoubleDefinitionID) { + import Signature.MatchDegree.* + + private def erasedType: Type = + if ctx.erasedTypes then decl.info + else TypeErasure.transformInfo(decl, decl.info) + def msg(using Context) = { def nameAnd = if (decl.name != previousDecl.name) " name and" else "" - def erasedType: Type = - if ctx.erasedTypes then decl.info - else TypeErasure.transformInfo(decl, decl.info) def details(using Context): String = if (decl.isRealMethod && previousDecl.isRealMethod) { - import Signature.MatchDegree.* // compare the signatures when both symbols represent methods decl.signature.matchDegree(previousDecl.signature) match { @@ -2397,7 +2399,43 @@ extends NamingMsg(DoubleDefinitionID) { |""" } + details } - def explain(using Context) = "" + def explain(using Context) = + decl.signature.matchDegree(previousDecl.signature) match + case FullMatch => + i""" + |As part of the Scala compilation pipeline every type is reduced to its erased + |(runtime) form. In this phase, among other transformations, generic parameters + |disappear and separate parameter-list boundaries are flattened. + | + |For example, both `f[T](x: T)(y: String): Unit` and `f(x: Any, z: String): Unit` + |erase to the same runtime signature `f(x: Object, y: String): Unit`. Note that + |parameter names are irrelevant. + | + |In your code the two declarations + | + | ${previousDecl.showDcl} + | ${decl.showDcl} + | + |erase to the identical signature + | + | ${erasedType} + | + |so the compiler cannot keep both: the generated bytecode symbols would collide. + | + |To fix this error, you need to disambiguate the two definitions. You can either: + | + |1. Rename one of the definitions, or + |2. Keep the same names in source but give one definition a distinct + | bytecode-level name via `@targetName` for example: + | + | @targetName("${decl.name.show}_2") + | ${decl.showDcl} + | + |Choose the `@targetName` argument carefully: it is the name that will be used + |when calling the method externally, so it should be unique and descriptive. + """ + case _ => "" + } class ImportedTwice(sel: Name)(using Context) extends SyntaxMsg(ImportedTwiceID) { diff --git a/tests/neg/doubleDefinition.check b/tests/neg/doubleDefinition.check index db98effbabdd..3c164fcf878b 100644 --- a/tests/neg/doubleDefinition.check +++ b/tests/neg/doubleDefinition.check @@ -8,6 +8,8 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:21:5 ---------------------------------------------------------- 21 | def foo(x: List[A]): Function2[B, B, B] = ??? // error | ^ @@ -21,24 +23,32 @@ | Conflicting definitions: | val foo: Int in class Test4 at line 25 and | def foo: Int in class Test4 at line 26 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:31:5 ---------------------------------------------------------- 31 | val foo = 1 // error | ^ | Conflicting definitions: | def foo: Int in class Test4b at line 30 and | val foo: Int in class Test4b at line 31 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:36:5 ---------------------------------------------------------- 36 | var foo = 1 // error | ^ | Conflicting definitions: | def foo: Int in class Test4c at line 35 and | var foo: Int in class Test4c at line 36 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:41:5 ---------------------------------------------------------- 41 | def foo = 2 // error | ^ | Conflicting definitions: | var foo: Int in class Test4d at line 40 and | def foo: Int in class Test4d at line 41 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:55:5 ---------------------------------------------------------- 55 | def foo(x: List[B]): Function1[B, B] = ??? // error: same jvm signature | ^ @@ -49,6 +59,8 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:62:5 ---------------------------------------------------------- 62 | def foo(x: List[A]): Function2[B, B, B] = ??? // error | ^ @@ -62,69 +74,93 @@ | Conflicting definitions: | val foo: Int in class Test8 at line 66 and | def foo: Int in class Test8 at line 67 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:72:5 ---------------------------------------------------------- 72 | val foo = 1 // error | ^ | Conflicting definitions: | def foo: Int in class Test8b at line 71 and | val foo: Int in class Test8b at line 72 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:77:5 ---------------------------------------------------------- 77 | var foo = 1 // error | ^ | Conflicting definitions: | def foo: Int in class Test8c at line 76 and | var foo: Int in class Test8c at line 77 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:82:5 ---------------------------------------------------------- 82 | def foo = 2 // error | ^ | Conflicting definitions: | var foo: Int in class Test8d at line 81 and | def foo: Int in class Test8d at line 82 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:88:5 ---------------------------------------------------------- 88 | def foo: String // error | ^ | Conflicting definitions: | val foo: Int in class Test9 at line 87 and | def foo: String in class Test9 at line 88 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:92:5 ---------------------------------------------------------- 92 | def foo: Int // error | ^ | Conflicting definitions: | val foo: Int in class Test10 at line 91 and | def foo: Int in class Test10 at line 92 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:96:5 ---------------------------------------------------------- 96 | def foo: String // error | ^ | Conflicting definitions: | val foo: Int in class Test11 at line 95 and | def foo: String in class Test11 at line 96 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:100:5 --------------------------------------------------------- 100 | def foo: Int // error | ^ | Conflicting definitions: | val foo: Int in class Test12 at line 99 and | def foo: Int in class Test12 at line 100 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:104:5 --------------------------------------------------------- 104 | def foo: String // error | ^ | Conflicting definitions: | var foo: Int in class Test13 at line 103 and | def foo: String in class Test13 at line 104 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:108:5 --------------------------------------------------------- 108 | def foo: Int // error | ^ | Conflicting definitions: | var foo: Int in class Test14 at line 107 and | def foo: Int in class Test14 at line 108 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:112:5 --------------------------------------------------------- 112 | def foo: String // error | ^ | Conflicting definitions: | var foo: Int in class Test15 at line 111 and | def foo: String in class Test15 at line 112 + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/doubleDefinition.scala:116:5 --------------------------------------------------------- 116 | def foo: Int // error | ^ | Conflicting definitions: | var foo: Int in class Test16 at line 115 and | def foo: Int in class Test16 at line 116 + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/exports.check b/tests/neg/exports.check index 4016d8729043..bb4d7ab59421 100644 --- a/tests/neg/exports.check +++ b/tests/neg/exports.check @@ -22,6 +22,8 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/exports.scala:24:20 ------------------------------------------------------------------ 24 | export scanUnit._ // error: double definition | ^ @@ -32,6 +34,8 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/exports.scala:26:21 ------------------------------------------------------------------ 26 | export printUnit.status // error: double definition | ^ @@ -42,6 +46,8 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` -- Error: tests/neg/exports.scala:35:24 -------------------------------------------------------------------------------- 35 | export this.{concat => ++} // error: no eligible member | ^^^^^^^^^^^^ @@ -58,6 +64,8 @@ | Conflicting definitions: | val bar: Bar in class Baz at line 45 and | final def bar: (Baz.this.bar.bar : => (Baz.this.bar.baz.bar : Bar)) in class Baz at line 46 + | + | longer explanation available when compiling with `-explain` -- [E083] Type Error: tests/neg/exports.scala:57:11 -------------------------------------------------------------------- 57 | export printer.* // error: not stable | ^^^^^^^ diff --git a/tests/neg/i14966a.check b/tests/neg/i14966a.check index 933d16e593aa..fd941509807a 100644 --- a/tests/neg/i14966a.check +++ b/tests/neg/i14966a.check @@ -8,3 +8,5 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i19809.check b/tests/neg/i19809.check index bac93138c0ea..fc51067e339c 100644 --- a/tests/neg/i19809.check +++ b/tests/neg/i19809.check @@ -8,3 +8,5 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i23402.check b/tests/neg/i23402.check index 7310c7738b38..4a98af863348 100644 --- a/tests/neg/i23402.check +++ b/tests/neg/i23402.check @@ -1,10 +1,46 @@ --- [E120] Naming Error: tests/neg/i23402.scala:4:5 --------------------------------------------------------------------- -4 | def apply(p1: String)(p2: Int): A = A(p1, p2) // error +-- [E120] Naming Error: tests/neg/i23402.scala:6:5 --------------------------------------------------------------------- +6 | def apply(p1: String)(p2: Int): A = A(p1, p2) // error | ^ | Conflicting definitions: - | def apply(p1: String, p2: Int): A in object A at line 3 and - | def apply(p1: String)(p2: Int): A in object A at line 4 + | def apply(p1: String, p2: Int): A in object A at line 5 and + | def apply(p1: String)(p2: Int): A in object A at line 6 | have the same type (p1: String, p2: Int): A after erasure. | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + |--------------------------------------------------------------------------------------------------------------------- + | Explanation (enabled by `-explain`) + |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + | + | As part of the Scala compilation pipeline every type is reduced to its erased + | (runtime) form. In this phase, among other transformations, generic parameters + | disappear and separate parameter-list boundaries are flattened. + | + | For example, both `f[T](x: T)(y: String): Unit` and `f(x: Any, z: String): Unit` + | erase to the same runtime signature `f(x: Object, y: String): Unit`. Note that + | parameter names are irrelevant. + | + | In your code the two declarations + | + | def apply(p1: String, p2: Int): A + | def apply(p1: String)(p2: Int): A + | + | erase to the identical signature + | + | (p1: String, p2: Int): A + | + | so the compiler cannot keep both: the generated bytecode symbols would collide. + | + | To fix this error, you need to disambiguate the two definitions. You can either: + | + | 1. Rename one of the definitions, or + | 2. Keep the same names in source but give one definition a distinct + | bytecode-level name via `@targetName` for example: + | + | @targetName("apply_2") + | def apply(p1: String)(p2: Int): A + | + | Choose the `@targetName` argument carefully: it is the name that will be used + | when calling the method externally, so it should be unique and descriptive. + | + --------------------------------------------------------------------------------------------------------------------- diff --git a/tests/neg/i23402.scala b/tests/neg/i23402.scala index 8849db2646dd..9f6fcdf032aa 100644 --- a/tests/neg/i23402.scala +++ b/tests/neg/i23402.scala @@ -1,3 +1,5 @@ +//> using options -explain + class A(p1: String, p2: Int) object A { def apply(p1: String, p2: Int): A = A(p1, p2) diff --git a/tests/neg/i23402b.check b/tests/neg/i23402b.check index 1fce53d00d26..5036cf20097f 100644 --- a/tests/neg/i23402b.check +++ b/tests/neg/i23402b.check @@ -8,3 +8,5 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i23402c.check b/tests/neg/i23402c.check index 5a87250ffb9e..be73dd9c361d 100644 --- a/tests/neg/i23402c.check +++ b/tests/neg/i23402c.check @@ -8,3 +8,5 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i23402d.check b/tests/neg/i23402d.check new file mode 100644 index 000000000000..38806f9f122e --- /dev/null +++ b/tests/neg/i23402d.check @@ -0,0 +1,12 @@ +-- [E120] Naming Error: tests/neg/i23402d.scala:5:4 -------------------------------------------------------------------- +5 |def f(x: Any): Unit = ??? // error + | ^ + | Conflicting definitions: + | def f[T](x: T): Unit in the top-level definitions in package at line 4 and + | def f(x: Any): Unit in the top-level definitions in package at line 5 + | have the same type (x: Object): Unit after erasure. + | + | Consider adding a @targetName annotation to one of the conflicting definitions + | for disambiguation. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i23402d.scala b/tests/neg/i23402d.scala new file mode 100644 index 000000000000..674515fefd14 --- /dev/null +++ b/tests/neg/i23402d.scala @@ -0,0 +1,5 @@ +// This test checks that the example given in the `-explain` of the +// `DoubleDefinition` message is correct. + +def f[T](x: T): Unit = ??? +def f(x: Any): Unit = ??? // error diff --git a/tests/neg/into-override.check b/tests/neg/into-override.check index 7ecbc7b4fa2a..c497986a9800 100644 --- a/tests/neg/into-override.check +++ b/tests/neg/into-override.check @@ -8,6 +8,8 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/into-override.scala:19:6 ------------------------------------------------------------- 19 |class D[X] extends B[X], C[X] // error | ^ @@ -18,6 +20,8 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` -- [E120] Naming Error: tests/neg/into-override.scala:21:6 ------------------------------------------------------------- 21 |trait E[X] extends C[X]: // error | ^ @@ -28,3 +32,5 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/override-erasure-clash.check b/tests/neg/override-erasure-clash.check index 2295eed3f125..136eb0b4885f 100644 --- a/tests/neg/override-erasure-clash.check +++ b/tests/neg/override-erasure-clash.check @@ -5,3 +5,5 @@ | def f(): Int in class A at line 3 and | def g(): Int in class B at line 5 | have the same name and type (): Int after erasure. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/type-params.check b/tests/neg/type-params.check index c812358c9603..cebc311588c4 100644 --- a/tests/neg/type-params.check +++ b/tests/neg/type-params.check @@ -59,6 +59,8 @@ | | Consider adding a @targetName annotation to one of the conflicting definitions | for disambiguation. + | + | longer explanation available when compiling with `-explain` -- Error: tests/neg/type-params.scala:4:7 ------------------------------------------------------------------------------ 4 | "".==[Int] // error | ^^^^^^^^^^ From 3c892d3948f961b733230b2fcef1e456c4988917 Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Thu, 3 Jul 2025 12:39:33 +0000 Subject: [PATCH 2/2] Add regression test for #23350 --- tests/neg/i23350.check | 46 ++++++++++++++++++++++++++++++++++++++++++ tests/neg/i23350.scala | 10 +++++++++ 2 files changed, 56 insertions(+) create mode 100644 tests/neg/i23350.check create mode 100644 tests/neg/i23350.scala diff --git a/tests/neg/i23350.check b/tests/neg/i23350.check new file mode 100644 index 000000000000..d9ae6a99cdca --- /dev/null +++ b/tests/neg/i23350.check @@ -0,0 +1,46 @@ +-- [E120] Naming Error: tests/neg/i23350.scala:8:7 --------------------------------------------------------------------- +8 |object D extends A: // error + | ^ + | Name clash between defined and inherited member: + | def apply(p: A.this.Props): Unit in class A at line 5 and + | def apply(a: UndefOr2[String]): Unit in object D at line 10 + | have the same type (a: Object): Unit after erasure. + | + | Consider adding a @targetName annotation to one of the conflicting definitions + | for disambiguation. + |--------------------------------------------------------------------------------------------------------------------- + | Explanation (enabled by `-explain`) + |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + | + | As part of the Scala compilation pipeline every type is reduced to its erased + | (runtime) form. In this phase, among other transformations, generic parameters + | disappear and separate parameter-list boundaries are flattened. + | + | For example, both `f[T](x: T)(y: String): Unit` and `f(x: Any, z: String): Unit` + | erase to the same runtime signature `f(x: Object, y: String): Unit`. Note that + | parameter names are irrelevant. + | + | In your code the two declarations + | + | def apply(p: A.this.Props): Unit + | def apply(a: UndefOr2[String]): Unit + | + | erase to the identical signature + | + | (a: Object): Unit + | + | so the compiler cannot keep both: the generated bytecode symbols would collide. + | + | To fix this error, you need to disambiguate the two definitions. You can either: + | + | 1. Rename one of the definitions, or + | 2. Keep the same names in source but give one definition a distinct + | bytecode-level name via `@targetName` for example: + | + | @targetName("apply_2") + | def apply(a: UndefOr2[String]): Unit + | + | Choose the `@targetName` argument carefully: it is the name that will be used + | when calling the method externally, so it should be unique and descriptive. + | + --------------------------------------------------------------------------------------------------------------------- diff --git a/tests/neg/i23350.scala b/tests/neg/i23350.scala new file mode 100644 index 000000000000..fa85805f8ae3 --- /dev/null +++ b/tests/neg/i23350.scala @@ -0,0 +1,10 @@ +//> using options -explain + +abstract class A: + type Props + def apply(p: Props) = () + +type UndefOr2[A] = A | Unit +object D extends A: // error + case class Props() + def apply(a: UndefOr2[String]) = ()