Skip to content

Commit 074e555

Browse files
committed
Support extension methods imported from different objects
Add a special case to name resolution so that when expanding an extension method from `e.m` to `m(e)` and `m` is imported by several imports on the same level, we try to typecheck under every such import and pick the successful alternative if it exists and is unambiguous. Fixes #16920
1 parent 8020c77 commit 074e555

File tree

10 files changed

+325
-43
lines changed

10 files changed

+325
-43
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
190190
case InlineGivenShouldNotBeFunctionID // errorNumber 174
191191
case ValueDiscardingID // errorNumber 175
192192
case UnusedNonUnitValueID // errorNumber 176
193+
case AmbiguousExtensionMethodID // errorNumber 177
193194

194195
def errorNumber = ordinal - 1
195196

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,6 +1403,15 @@ extends ReferenceMsg(AmbiguousOverloadID), NoDisambiguation {
14031403
|"""
14041404
}
14051405

1406+
class AmbiguousExtensionMethod(tree: untpd.Tree, expansion1: tpd.Tree, expansion2: tpd.Tree)(using Context)
1407+
extends ReferenceMsg(AmbiguousExtensionMethodID), NoDisambiguation:
1408+
def msg(using Context) =
1409+
i"""Ambiguous extension methods:
1410+
|both $expansion1
1411+
|and $expansion2
1412+
|are possible expansions of $tree"""
1413+
def explain(using Context) = ""
1414+
14061415
class ReassignmentToVal(name: Name)(using Context)
14071416
extends TypeMsg(ReassignmentToValID) {
14081417
def msg(using Context) = i"""Reassignment to val $name"""

compiler/src/dotty/tools/dotc/typer/Typer.scala

Lines changed: 81 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,13 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
158158
* @param required flags the result's symbol must have
159159
* @param excluded flags the result's symbol must not have
160160
* @param pos indicates position to use for error reporting
161+
* @param altImports a ListBuffer in which alternative imported references are
162+
* collected in case `findRef` is called from an expansion of
163+
* an extension method, i.e. when `e.m` is expanded to `m(e)` and
164+
* a reference for `m` is searched. `null` in all other situations.
161165
*/
162-
def findRef(name: Name, pt: Type, required: FlagSet, excluded: FlagSet, pos: SrcPos)(using Context): Type = {
166+
def findRef(name: Name, pt: Type, required: FlagSet, excluded: FlagSet, pos: SrcPos,
167+
altImports: mutable.ListBuffer[TermRef] | Null = null)(using Context): Type = {
163168
val refctx = ctx
164169
val noImports = ctx.mode.is(Mode.InPackageClauseName)
165170
def suppressErrors = excluded.is(ConstructorProxy)
@@ -230,15 +235,52 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
230235
fail(AmbiguousReference(name, newPrec, prevPrec, prevCtx))
231236
previous
232237

233-
/** Recurse in outer context. If final result is same as `previous`, check that it
234-
* is new or shadowed. This order of checking is necessary since an
235-
* outer package-level definition might trump two conflicting inner
236-
* imports, so no error should be issued in that case. See i7876.scala.
238+
/** Assemble and check alternatives to an imported reference. This implies:
239+
* - If we expand an extension method (i.e. altImports != null),
240+
* search imports on the same level for other possible resolutions of `name`.
241+
* The result and altImports together then contain all possible imported
242+
* references of the highest possible precedence, where `NamedImport` beats
243+
* `WildImport`.
244+
* - Find a posssibly shadowing reference in an outer context.
245+
* If the result is the same as `previous`, check that it is new or
246+
* shadowed. This order of checking is necessary since an outer package-level
247+
* definition might trump two conflicting inner imports, so no error should be
248+
* issued in that case. See i7876.scala.
249+
* @param previous the previously found reference (which is an import)
250+
* @param prevPrec the precedence of the reference (either NamedImport or WildImport)
251+
* @param prevCtx the context in which the reference was found
252+
* @param using_Context the outer context of `precCtx`
237253
*/
238-
def recurAndCheckNewOrShadowed(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type =
239-
val found = findRefRecur(previous, prevPrec, prevCtx)
240-
if found eq previous then checkNewOrShadowed(found, prevPrec)(using prevCtx)
241-
else found
254+
def checkImportAlternatives(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type =
255+
256+
def addAltImport(altImp: TermRef) =
257+
if !TypeComparer.isSameRef(previous, altImp)
258+
&& !altImports.uncheckedNN.exists(TypeComparer.isSameRef(_, altImp))
259+
then
260+
altImports.uncheckedNN += altImp
261+
262+
if altImports != null && ctx.isImportContext then
263+
val curImport = ctx.importInfo.uncheckedNN
264+
namedImportRef(curImport) match
265+
case altImp: TermRef =>
266+
if prevPrec == WildImport then
267+
// Discard all previously found references and continue with `altImp`
268+
altImports.clear()
269+
checkImportAlternatives(altImp, NamedImport, ctx)(using ctx.outer)
270+
else
271+
addAltImport(altImp)
272+
checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
273+
case _ =>
274+
if prevPrec == WildImport then
275+
wildImportRef(curImport) match
276+
case altImp: TermRef => addAltImport(altImp)
277+
case _ =>
278+
checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
279+
else
280+
val found = findRefRecur(previous, prevPrec, prevCtx)
281+
if found eq previous then checkNewOrShadowed(found, prevPrec)(using prevCtx)
282+
else found
283+
end checkImportAlternatives
242284

243285
def selection(imp: ImportInfo, name: Name, checkBounds: Boolean): Type =
244286
imp.importSym.info match
@@ -328,7 +370,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
328370
if (ctx.scope eq EmptyScope) previous
329371
else {
330372
var result: Type = NoType
331-
332373
val curOwner = ctx.owner
333374

334375
/** Is curOwner a package object that should be skipped?
@@ -449,11 +490,11 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
449490
else if (isPossibleImport(NamedImport) && (curImport nen outer.importInfo)) {
450491
val namedImp = namedImportRef(curImport.uncheckedNN)
451492
if (namedImp.exists)
452-
recurAndCheckNewOrShadowed(namedImp, NamedImport, ctx)(using outer)
493+
checkImportAlternatives(namedImp, NamedImport, ctx)(using outer)
453494
else if (isPossibleImport(WildImport) && !curImport.nn.importSym.isCompleting) {
454495
val wildImp = wildImportRef(curImport.uncheckedNN)
455496
if (wildImp.exists)
456-
recurAndCheckNewOrShadowed(wildImp, WildImport, ctx)(using outer)
497+
checkImportAlternatives(wildImp, WildImport, ctx)(using outer)
457498
else {
458499
updateUnimported()
459500
loop(ctx)(using outer)
@@ -3359,11 +3400,37 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
33593400
def selectionProto = SelectionProto(tree.name, mbrProto, compat, privateOK = inSelect)
33603401

33613402
def tryExtension(using Context): Tree =
3362-
findRef(tree.name, WildcardType, ExtensionMethod, EmptyFlags, qual.srcPos) match
3403+
val altImports = new mutable.ListBuffer[TermRef]()
3404+
findRef(tree.name, WildcardType, ExtensionMethod, EmptyFlags, qual.srcPos, altImports) match
33633405
case ref: TermRef =>
3364-
extMethodApply(untpd.TypedSplice(tpd.ref(ref).withSpan(tree.nameSpan)), qual, pt)
3406+
def tryExtMethod(ref: TermRef)(using Context) =
3407+
extMethodApply(untpd.TypedSplice(tpd.ref(ref).withSpan(tree.nameSpan)), qual, pt)
3408+
if altImports.isEmpty then
3409+
tryExtMethod(ref)
3410+
else
3411+
// Try all possible imports and collect successes and failures
3412+
val successes, failures = new mutable.ListBuffer[(Tree, TyperState)]
3413+
for alt <- ref :: altImports.toList do
3414+
val nestedCtx = ctx.fresh.setNewTyperState()
3415+
val app = tryExtMethod(alt)(using nestedCtx)
3416+
(if nestedCtx.reporter.hasErrors then failures else successes)
3417+
+= ((app, nestedCtx.typerState))
3418+
typr.println(i"multiple extensioin methods, success: ${successes.toList}, failure: ${failures.toList}")
3419+
3420+
def pick(alt: (Tree, TyperState)): Tree =
3421+
val (app, ts) = alt
3422+
ts.commit()
3423+
app
3424+
3425+
successes.toList match
3426+
case Nil => pick(failures.head)
3427+
case success :: Nil => pick(success)
3428+
case (expansion1, _) :: (expansion2, _) :: _ =>
3429+
report.error(AmbiguousExtensionMethod(tree, expansion1, expansion2), tree.srcPos)
3430+
expansion1
33653431
case _ =>
33663432
EmptyTree
3433+
end tryExtension
33673434

33683435
def nestedFailure(ex: TypeError) =
33693436
rememberSearchFailure(qual,

docs/_docs/reference/contextual/extension-methods.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,16 @@ The precise rules for resolving a selection to an extension method are as follow
244244
Assume a selection `e.m[Ts]` where `m` is not a member of `e`, where the type arguments `[Ts]` are optional, and where `T` is the expected type.
245245
The following two rewritings are tried in order:
246246

247-
1. The selection is rewritten to `m[Ts](e)`.
247+
1. The selection is rewritten to `m[Ts](e)` and typechecked, using the following
248+
slight modification of the name resolution rules:
249+
250+
- If `m` is imported by several imports which are all on the nesting level,
251+
try each import as an extension method instead of failing with an ambiguity.
252+
If only one import leads to an expansion that typechecks without errors, pick
253+
that expansion. If there are several such imports, but only one import which is
254+
not a wildcard import, pick the expansion from that import. Otherwise, report
255+
an ambiguous reference error.
256+
248257
2. If the first rewriting does not typecheck with expected type `T`,
249258
and there is an extension method `m` in some eligible object `o`, the selection is rewritten to `o.m[Ts](e)`. An object `o` is _eligible_ if
250259

tests/neg/i13558.check

Lines changed: 0 additions & 26 deletions
This file was deleted.

tests/neg/i16920.check

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
-- [E008] Not Found Error: tests/neg/i16920.scala:18:11 ----------------------------------------------------------------
2+
18 | "five".wow // error
3+
| ^^^^^^^^^^
4+
| value wow is not a member of String.
5+
| An extension method was tried, but could not be fully constructed:
6+
|
7+
| Two.wow("five")
8+
|
9+
| failed with:
10+
|
11+
| Found: ("five" : String)
12+
| Required: Int
13+
-- [E008] Not Found Error: tests/neg/i16920.scala:26:6 -----------------------------------------------------------------
14+
26 | 5.wow // error
15+
| ^^^^^
16+
| value wow is not a member of Int.
17+
| An extension method was tried, but could not be fully constructed:
18+
|
19+
| AlsoFails.wow(5)
20+
|
21+
| failed with:
22+
|
23+
| Found: (5 : Int)
24+
| Required: Boolean
25+
-- [E008] Not Found Error: tests/neg/i16920.scala:27:11 ----------------------------------------------------------------
26+
27 | "five".wow // error
27+
| ^^^^^^^^^^
28+
| value wow is not a member of String.
29+
| An extension method was tried, but could not be fully constructed:
30+
|
31+
| AlsoFails.wow("five")
32+
|
33+
| failed with:
34+
|
35+
| Found: ("five" : String)
36+
| Required: Boolean
37+
-- [E008] Not Found Error: tests/neg/i16920.scala:34:6 -----------------------------------------------------------------
38+
34 | 5.wow // error
39+
| ^^^^^
40+
| value wow is not a member of Int.
41+
| An extension method was tried, but could not be fully constructed:
42+
|
43+
| Three.wow(5)
44+
|
45+
| failed with:
46+
|
47+
| Ambiguous extension methods:
48+
| both Three.wow(5)
49+
| and Two.wow(5)
50+
| are possible expansions of 5.wow
51+
-- [E008] Not Found Error: tests/neg/i16920.scala:42:11 ----------------------------------------------------------------
52+
42 | "five".wow // error
53+
| ^^^^^^^^^^
54+
| value wow is not a member of String.
55+
| An extension method was tried, but could not be fully constructed:
56+
|
57+
| Two.wow("five")
58+
|
59+
| failed with:
60+
|
61+
| Found: ("five" : String)
62+
| Required: Int
63+
-- [E008] Not Found Error: tests/neg/i16920.scala:49:11 ----------------------------------------------------------------
64+
49 | "five".wow // error
65+
| ^^^^^^^^^^
66+
| value wow is not a member of String.
67+
| An extension method was tried, but could not be fully constructed:
68+
|
69+
| Two.wow("five")
70+
|
71+
| failed with:
72+
|
73+
| Found: ("five" : String)
74+
| Required: Int
75+
-- [E008] Not Found Error: tests/neg/i16920.scala:56:6 -----------------------------------------------------------------
76+
56 | 5.wow // error
77+
| ^^^^^
78+
| value wow is not a member of Int.
79+
| An extension method was tried, but could not be fully constructed:
80+
|
81+
| Three.wow(5)
82+
|
83+
| failed with:
84+
|
85+
| Ambiguous extension methods:
86+
| both Three.wow(5)
87+
| and Two.wow(5)
88+
| are possible expansions of 5.wow

tests/neg/i16920.scala

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
object One:
2+
extension (s: String)
3+
def wow: Unit = println(s)
4+
5+
object Two:
6+
extension (i: Int)
7+
def wow: Unit = println(i)
8+
9+
object Three:
10+
extension (i: Int)
11+
def wow: Unit = println(i)
12+
13+
object Fails:
14+
import One._
15+
def test: Unit =
16+
import Two._
17+
5.wow
18+
"five".wow // error
19+
20+
object AlsoFails:
21+
extension (s: Boolean)
22+
def wow = println(s)
23+
import One._
24+
import Two._
25+
def test: Unit =
26+
5.wow // error
27+
"five".wow // error
28+
29+
object Fails2:
30+
import One._
31+
import Two._
32+
import Three._
33+
def test: Unit =
34+
5.wow // error
35+
"five".wow // ok
36+
37+
object Fails3:
38+
import One._
39+
import Two.wow
40+
def test: Unit =
41+
5.wow // ok
42+
"five".wow // error
43+
44+
object Fails4:
45+
import Two.wow
46+
import One._
47+
def test: Unit =
48+
5.wow // ok
49+
"five".wow // error
50+
51+
object Fails5:
52+
import One.wow
53+
import Two.wow
54+
import Three.wow
55+
def test: Unit =
56+
5.wow // error
57+
"five".wow // ok

tests/new/test.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
object Test:
22
def test = 0
3+

tests/neg/i13558.scala renamed to tests/pos/i13558.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ object Main {
2020
import ExtensionB._
2121
import ExtensionA._
2222
val a = A()
23-
println(a.id) // error
23+
println(a.id) // now ok
2424
}
2525
def main2(args: Array[String]): Unit = {
2626
import ExtensionA._
2727
import ExtensionB._
2828
val a = A()
29-
println(a.id) // error
29+
println(a.id) // now ok
3030
}
3131
}

0 commit comments

Comments
 (0)