Skip to content

Commit 62387f9

Browse files
authored
Merge pull request #485 from scala/backport-lts-3.3-23470
Backport "Add an explainer to the DoubleDefinition error" to 3.3 LTS
2 parents 3b8e521 + c657425 commit 62387f9

File tree

14 files changed

+212
-9
lines changed

14 files changed

+212
-9
lines changed

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

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2301,14 +2301,16 @@ class JavaSymbolIsNotAValue(symbol: Symbol)(using Context) extends TypeMsg(JavaS
23012301

23022302
class DoubleDefinition(decl: Symbol, previousDecl: Symbol, base: Symbol)(using Context)
23032303
extends NamingMsg(DoubleDefinitionID) {
2304+
import Signature.MatchDegree.*
2305+
2306+
private def erasedType: Type =
2307+
if ctx.erasedTypes then decl.info
2308+
else TypeErasure.transformInfo(decl, decl.info)
2309+
23042310
def msg(using Context) = {
23052311
def nameAnd = if (decl.name != previousDecl.name) " name and" else ""
2306-
def erasedType: Type =
2307-
if ctx.erasedTypes then decl.info
2308-
else TypeErasure.transformInfo(decl, decl.info)
23092312
def details(using Context): String =
23102313
if (decl.isRealMethod && previousDecl.isRealMethod) {
2311-
import Signature.MatchDegree.*
23122314

23132315
// compare the signatures when both symbols represent methods
23142316
decl.signature.matchDegree(previousDecl.signature) match {
@@ -2359,7 +2361,43 @@ extends NamingMsg(DoubleDefinitionID) {
23592361
|"""
23602362
} + details
23612363
}
2362-
def explain(using Context) = ""
2364+
def explain(using Context) =
2365+
decl.signature.matchDegree(previousDecl.signature) match
2366+
case FullMatch =>
2367+
i"""
2368+
|As part of the Scala compilation pipeline every type is reduced to its erased
2369+
|(runtime) form. In this phase, among other transformations, generic parameters
2370+
|disappear and separate parameter-list boundaries are flattened.
2371+
|
2372+
|For example, both `f[T](x: T)(y: String): Unit` and `f(x: Any, z: String): Unit`
2373+
|erase to the same runtime signature `f(x: Object, y: String): Unit`. Note that
2374+
|parameter names are irrelevant.
2375+
|
2376+
|In your code the two declarations
2377+
|
2378+
| ${previousDecl.showDcl}
2379+
| ${decl.showDcl}
2380+
|
2381+
|erase to the identical signature
2382+
|
2383+
| ${erasedType}
2384+
|
2385+
|so the compiler cannot keep both: the generated bytecode symbols would collide.
2386+
|
2387+
|To fix this error, you need to disambiguate the two definitions. You can either:
2388+
|
2389+
|1. Rename one of the definitions, or
2390+
|2. Keep the same names in source but give one definition a distinct
2391+
| bytecode-level name via `@targetName` for example:
2392+
|
2393+
| @targetName("${decl.name.show}_2")
2394+
| ${decl.showDcl}
2395+
|
2396+
|Choose the `@targetName` argument carefully: it is the name that will be used
2397+
|when calling the method externally, so it should be unique and descriptive.
2398+
"""
2399+
case _ => ""
2400+
23632401
}
23642402

23652403
class ImportRenamedTwice(ident: untpd.Ident)(using Context) extends SyntaxMsg(ImportRenamedTwiceID) {

tests/neg/doubleDefinition.check

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
|
99
| Consider adding a @targetName annotation to one of the conflicting definitions
1010
| for disambiguation.
11+
|
12+
| longer explanation available when compiling with `-explain`
1113
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:21:5 ----------------------------------------------------------
1214
21 | def foo(x: List[A]): Function2[B, B, B] = ??? // error
1315
| ^
@@ -21,24 +23,32 @@
2123
| Conflicting definitions:
2224
| val foo: Int in class Test4 at line 25 and
2325
| def foo: Int in class Test4 at line 26
26+
|
27+
| longer explanation available when compiling with `-explain`
2428
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:31:5 ----------------------------------------------------------
2529
31 | val foo = 1 // error
2630
| ^
2731
| Conflicting definitions:
2832
| def foo: Int in class Test4b at line 30 and
2933
| val foo: Int in class Test4b at line 31
34+
|
35+
| longer explanation available when compiling with `-explain`
3036
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:36:5 ----------------------------------------------------------
3137
36 | var foo = 1 // error
3238
| ^
3339
| Conflicting definitions:
3440
| def foo: Int in class Test4c at line 35 and
3541
| var foo: Int in class Test4c at line 36
42+
|
43+
| longer explanation available when compiling with `-explain`
3644
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:41:5 ----------------------------------------------------------
3745
41 | def foo = 2 // error
3846
| ^
3947
| Conflicting definitions:
4048
| var foo: Int in class Test4d at line 40 and
4149
| def foo: Int in class Test4d at line 41
50+
|
51+
| longer explanation available when compiling with `-explain`
4252
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:55:5 ----------------------------------------------------------
4353
55 | def foo(x: List[B]): Function1[B, B] = ??? // error: same jvm signature
4454
| ^
@@ -49,6 +59,8 @@
4959
|
5060
| Consider adding a @targetName annotation to one of the conflicting definitions
5161
| for disambiguation.
62+
|
63+
| longer explanation available when compiling with `-explain`
5264
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:62:5 ----------------------------------------------------------
5365
62 | def foo(x: List[A]): Function2[B, B, B] = ??? // error
5466
| ^
@@ -62,69 +74,93 @@
6274
| Conflicting definitions:
6375
| val foo: Int in class Test8 at line 66 and
6476
| def foo: Int in class Test8 at line 67
77+
|
78+
| longer explanation available when compiling with `-explain`
6579
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:72:5 ----------------------------------------------------------
6680
72 | val foo = 1 // error
6781
| ^
6882
| Conflicting definitions:
6983
| def foo: Int in class Test8b at line 71 and
7084
| val foo: Int in class Test8b at line 72
85+
|
86+
| longer explanation available when compiling with `-explain`
7187
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:77:5 ----------------------------------------------------------
7288
77 | var foo = 1 // error
7389
| ^
7490
| Conflicting definitions:
7591
| def foo: Int in class Test8c at line 76 and
7692
| var foo: Int in class Test8c at line 77
93+
|
94+
| longer explanation available when compiling with `-explain`
7795
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:82:5 ----------------------------------------------------------
7896
82 | def foo = 2 // error
7997
| ^
8098
| Conflicting definitions:
8199
| var foo: Int in class Test8d at line 81 and
82100
| def foo: Int in class Test8d at line 82
101+
|
102+
| longer explanation available when compiling with `-explain`
83103
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:88:5 ----------------------------------------------------------
84104
88 | def foo: String // error
85105
| ^
86106
| Conflicting definitions:
87107
| val foo: Int in class Test9 at line 87 and
88108
| def foo: String in class Test9 at line 88
109+
|
110+
| longer explanation available when compiling with `-explain`
89111
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:92:5 ----------------------------------------------------------
90112
92 | def foo: Int // error
91113
| ^
92114
| Conflicting definitions:
93115
| val foo: Int in class Test10 at line 91 and
94116
| def foo: Int in class Test10 at line 92
117+
|
118+
| longer explanation available when compiling with `-explain`
95119
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:96:5 ----------------------------------------------------------
96120
96 | def foo: String // error
97121
| ^
98122
| Conflicting definitions:
99123
| val foo: Int in class Test11 at line 95 and
100124
| def foo: String in class Test11 at line 96
125+
|
126+
| longer explanation available when compiling with `-explain`
101127
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:100:5 ---------------------------------------------------------
102128
100 | def foo: Int // error
103129
| ^
104130
| Conflicting definitions:
105131
| val foo: Int in class Test12 at line 99 and
106132
| def foo: Int in class Test12 at line 100
133+
|
134+
| longer explanation available when compiling with `-explain`
107135
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:104:5 ---------------------------------------------------------
108136
104 | def foo: String // error
109137
| ^
110138
| Conflicting definitions:
111139
| var foo: Int in class Test13 at line 103 and
112140
| def foo: String in class Test13 at line 104
141+
|
142+
| longer explanation available when compiling with `-explain`
113143
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:108:5 ---------------------------------------------------------
114144
108 | def foo: Int // error
115145
| ^
116146
| Conflicting definitions:
117147
| var foo: Int in class Test14 at line 107 and
118148
| def foo: Int in class Test14 at line 108
149+
|
150+
| longer explanation available when compiling with `-explain`
119151
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:112:5 ---------------------------------------------------------
120152
112 | def foo: String // error
121153
| ^
122154
| Conflicting definitions:
123155
| var foo: Int in class Test15 at line 111 and
124156
| def foo: String in class Test15 at line 112
157+
|
158+
| longer explanation available when compiling with `-explain`
125159
-- [E120] Naming Error: tests/neg/doubleDefinition.scala:116:5 ---------------------------------------------------------
126160
116 | def foo: Int // error
127161
| ^
128162
| Conflicting definitions:
129163
| var foo: Int in class Test16 at line 115 and
130164
| def foo: Int in class Test16 at line 116
165+
|
166+
| longer explanation available when compiling with `-explain`

tests/neg/exports.check

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
|
2323
| Consider adding a @targetName annotation to one of the conflicting definitions
2424
| for disambiguation.
25+
|
26+
| longer explanation available when compiling with `-explain`
2527
-- [E120] Naming Error: tests/neg/exports.scala:24:20 ------------------------------------------------------------------
2628
24 | export scanUnit._ // error: double definition
2729
| ^
@@ -32,6 +34,8 @@
3234
|
3335
| Consider adding a @targetName annotation to one of the conflicting definitions
3436
| for disambiguation.
37+
|
38+
| longer explanation available when compiling with `-explain`
3539
-- [E120] Naming Error: tests/neg/exports.scala:26:21 ------------------------------------------------------------------
3640
26 | export printUnit.status // error: double definition
3741
| ^
@@ -42,6 +46,8 @@
4246
|
4347
| Consider adding a @targetName annotation to one of the conflicting definitions
4448
| for disambiguation.
49+
|
50+
| longer explanation available when compiling with `-explain`
4551
-- Error: tests/neg/exports.scala:35:24 --------------------------------------------------------------------------------
4652
35 | export this.{concat => ++} // error: no eligible member
4753
| ^^^^^^^^^^^^
@@ -58,6 +64,8 @@
5864
| Conflicting definitions:
5965
| val bar: Bar in class Baz at line 45 and
6066
| final def bar: (Baz.this.bar.bar : => (Baz.this.bar.baz.bar : Bar)) in class Baz at line 46
67+
|
68+
| longer explanation available when compiling with `-explain`
6169
-- [E083] Type Error: tests/neg/exports.scala:57:11 --------------------------------------------------------------------
6270
57 | export printer.* // error: not stable
6371
| ^^^^^^^

tests/neg/i14966a.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,5 @@
88
|
99
| Consider adding a @targetName annotation to one of the conflicting definitions
1010
| for disambiguation.
11+
|
12+
| longer explanation available when compiling with `-explain`

tests/neg/i23350.check

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
-- [E120] Naming Error: tests/neg/i23350.scala:8:7 ---------------------------------------------------------------------
2+
8 |object D extends A: // error
3+
| ^
4+
| Name clash between defined and inherited member:
5+
| def apply(p: A.this.Props): Unit in class A at line 5 and
6+
| def apply(a: UndefOr2[String]): Unit in object D at line 10
7+
| have the same type (a: Object): Unit after erasure.
8+
|
9+
| Consider adding a @targetName annotation to one of the conflicting definitions
10+
| for disambiguation.
11+
|---------------------------------------------------------------------------------------------------------------------
12+
| Explanation (enabled by `-explain`)
13+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
14+
|
15+
| As part of the Scala compilation pipeline every type is reduced to its erased
16+
| (runtime) form. In this phase, among other transformations, generic parameters
17+
| disappear and separate parameter-list boundaries are flattened.
18+
|
19+
| For example, both `f[T](x: T)(y: String): Unit` and `f(x: Any, z: String): Unit`
20+
| erase to the same runtime signature `f(x: Object, y: String): Unit`. Note that
21+
| parameter names are irrelevant.
22+
|
23+
| In your code the two declarations
24+
|
25+
| def apply(p: A.this.Props): Unit
26+
| def apply(a: UndefOr2[String]): Unit
27+
|
28+
| erase to the identical signature
29+
|
30+
| (a: Object): Unit
31+
|
32+
| so the compiler cannot keep both: the generated bytecode symbols would collide.
33+
|
34+
| To fix this error, you need to disambiguate the two definitions. You can either:
35+
|
36+
| 1. Rename one of the definitions, or
37+
| 2. Keep the same names in source but give one definition a distinct
38+
| bytecode-level name via `@targetName` for example:
39+
|
40+
| @targetName("apply_2")
41+
| def apply(a: UndefOr2[String]): Unit
42+
|
43+
| Choose the `@targetName` argument carefully: it is the name that will be used
44+
| when calling the method externally, so it should be unique and descriptive.
45+
|
46+
---------------------------------------------------------------------------------------------------------------------

tests/neg/i23350.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//> using options -explain
2+
3+
abstract class A:
4+
type Props
5+
def apply(p: Props) = ()
6+
7+
type UndefOr2[A] = A | Unit
8+
object D extends A: // error
9+
case class Props()
10+
def apply(a: UndefOr2[String]) = ()

tests/neg/i23402.check

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,46 @@
1-
-- [E120] Naming Error: tests/neg/i23402.scala:4:5 ---------------------------------------------------------------------
2-
4 | def apply(p1: String)(p2: Int): A = A(p1, p2) // error
1+
-- [E120] Naming Error: tests/neg/i23402.scala:6:5 ---------------------------------------------------------------------
2+
6 | def apply(p1: String)(p2: Int): A = A(p1, p2) // error
33
| ^
44
| Conflicting definitions:
5-
| def apply(p1: String, p2: Int): A in object A at line 3 and
6-
| def apply(p1: String)(p2: Int): A in object A at line 4
5+
| def apply(p1: String, p2: Int): A in object A at line 5 and
6+
| def apply(p1: String)(p2: Int): A in object A at line 6
77
| have the same type (p1: String, p2: Int): A after erasure.
88
|
99
| Consider adding a @targetName annotation to one of the conflicting definitions
1010
| for disambiguation.
11+
|---------------------------------------------------------------------------------------------------------------------
12+
| Explanation (enabled by `-explain`)
13+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
14+
|
15+
| As part of the Scala compilation pipeline every type is reduced to its erased
16+
| (runtime) form. In this phase, among other transformations, generic parameters
17+
| disappear and separate parameter-list boundaries are flattened.
18+
|
19+
| For example, both `f[T](x: T)(y: String): Unit` and `f(x: Any, z: String): Unit`
20+
| erase to the same runtime signature `f(x: Object, y: String): Unit`. Note that
21+
| parameter names are irrelevant.
22+
|
23+
| In your code the two declarations
24+
|
25+
| def apply(p1: String, p2: Int): A
26+
| def apply(p1: String)(p2: Int): A
27+
|
28+
| erase to the identical signature
29+
|
30+
| (p1: String, p2: Int): A
31+
|
32+
| so the compiler cannot keep both: the generated bytecode symbols would collide.
33+
|
34+
| To fix this error, you need to disambiguate the two definitions. You can either:
35+
|
36+
| 1. Rename one of the definitions, or
37+
| 2. Keep the same names in source but give one definition a distinct
38+
| bytecode-level name via `@targetName` for example:
39+
|
40+
| @targetName("apply_2")
41+
| def apply(p1: String)(p2: Int): A
42+
|
43+
| Choose the `@targetName` argument carefully: it is the name that will be used
44+
| when calling the method externally, so it should be unique and descriptive.
45+
|
46+
---------------------------------------------------------------------------------------------------------------------

tests/neg/i23402.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
//> using options -explain
2+
13
class A(p1: String, p2: Int)
24
object A {
35
def apply(p1: String, p2: Int): A = A(p1, p2)

tests/neg/i23402b.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,5 @@
88
|
99
| Consider adding a @targetName annotation to one of the conflicting definitions
1010
| for disambiguation.
11+
|
12+
| longer explanation available when compiling with `-explain`

tests/neg/i23402c.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,5 @@
88
|
99
| Consider adding a @targetName annotation to one of the conflicting definitions
1010
| for disambiguation.
11+
|
12+
| longer explanation available when compiling with `-explain`

0 commit comments

Comments
 (0)