Skip to content

Commit 3b2d931

Browse files
committed
Make implicit search ordering transitive
1 parent cad0a36 commit 3b2d931

File tree

3 files changed

+151
-142
lines changed

3 files changed

+151
-142
lines changed

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

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,50 +1350,46 @@ trait Applications extends Compatibility {
13501350
* Module classes also inherit the relationship from their companions. This means,
13511351
* if no direct derivation exists between `sym1` and `sym2` also perform the following
13521352
* tests:
1353-
* - If both sym1 and sym1 are module classes that have companion classes, compare the companions
1354-
* - If sym1 is a module class with a companion, and sym2 is a normal class or trait, compare
1355-
* the companion with sym2.
1353+
* - If both sym1 and sym1 are module classes that have companion classes,
1354+
* and sym2 does not inherit implicit members from a base class (#),
1355+
* compare the companion classes.
1356+
* - If sym1 is a module class with a companion, and sym2 is a normal class or trait,
1357+
* compare the companion with sym2.
13561358
*
1357-
* Note that this makes `compareOwner(_, _) > 0` not transitive! For instance:
1359+
* Condition (#) is necessary to make `compareOwner(_, _) > 0` a transitive relation.
1360+
* For instance:
13581361
*
13591362
* class A extends B
1360-
* object A
1363+
* object A { given a ... }
13611364
* class B
1362-
* object B extends C
1365+
* object B extends C { given b ... }
1366+
* class C { given c }
13631367
*
1364-
* Then compareOwner(A$, B$) = 1 and compareOwner(B$, C) == 1, but
1365-
* compareOwner(A$, C) == 0.
1368+
* Then without (#), and taking A$ for the module class of A,
1369+
* compareOwner(A$, B$) = 1 and compareOwner(B$, C) == 1,
1370+
* but compareOwner(A$, C) == 0.
1371+
* Violating transitivity in this way is bad, since it makes implicit search
1372+
* outcomes compilation order dependent. E.g. if we compare `b` with `c`
1373+
* first, we pick `b`. Then, if we compare `a` and `b`, we pick `a` as
1374+
* solution of the search. But if we start with comparing `a` with `c`,
1375+
* we get an ambiguity.
1376+
*
1377+
* With the added condition (#), compareOwner(A$, B$) == 0.
1378+
* This means we get an ambiguity between `a` and `b` in all cases.
13661379
*/
13671380
def compareOwner(sym1: Symbol, sym2: Symbol)(using Context): Int =
13681381
if sym1 == sym2 then 0
13691382
else if sym1.isSubClass(sym2) then 1
13701383
else if sym2.isSubClass(sym1) then -1
13711384
else if sym1.is(Module) then
1372-
compareOwner(sym1.companionClass, if sym2.is(Module) then sym2.companionClass else sym2)
1385+
val cls1 = sym1.companionClass
1386+
if sym2.is(Module) then
1387+
if sym2.thisType.implicitMembers.forall(_.symbol.owner == sym2) then // test for (#)
1388+
compareOwner(cls1, sym2.companionClass)
1389+
else 0
1390+
else compareOwner(cls1, sym2)
13731391
else 0
13741392

1375-
/** A version of `compareOwner` that is transitive, to be used in sorting
1376-
* It would be nice if we could use this as the general method for comparing
1377-
* owners, but unfortunately this does not compile all existsing code.
1378-
* An example is `enrich-gentraversable.scala`. Here we have
1379-
*
1380-
* trait BuildFrom...
1381-
* object BuildFrom extends BuildFromLowPriority1
1382-
*
1383-
* and we need to pick an implicit in BuildFrom over BuildFromLowPriority1
1384-
* the rules in `compareOwner` give us that, but the rules in `isSubOwner`
1385-
* don't.
1386-
* So we need to stick with `compareOwner` for backwards compatibility, even
1387-
* though it is arguably broken. We can still use `isSubOwner` for sorting
1388-
* since that is just a performance optimization, so if the two methods
1389-
* don't agree sometimes that's OK.
1390-
*/
1391-
def isSubOwner(sym1: Symbol, sym2: Symbol)(using Context): Boolean =
1392-
if sym1.is(Module) && sym1.companionClass.exists then
1393-
isSubOwner(sym1.companionClass, if sym2.is(Module) then sym2.companionClass else sym2)
1394-
else
1395-
sym1 != sym2 && sym1.isSubClass(sym2)
1396-
13971393
/** Compare to alternatives of an overloaded call or an implicit search.
13981394
*
13991395
* @param alt1, alt2 Non-overloaded references indicating the two choices

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1173,7 +1173,7 @@ trait Implicits { self: Typer =>
11731173
val arity2 = sym2.info.firstParamTypes.length
11741174
if (arity1 < arity2) return true
11751175
if (arity1 > arity2) return false
1176-
isSubOwner(sym1, sym2)
1176+
compareOwner(sym1, sym2) == 1
11771177
}
11781178

11791179
/** Sort list of implicit references according to `prefer`.

docs/docs/reference/changed-features/implicit-resolution.md

Lines changed: 123 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -7,116 +7,129 @@ Implicit resolution uses a new algorithm which caches implicit results
77
more aggressively for performance. There are also some changes that
88
affect implicits on the language level.
99

10-
1. Types of implicit values and result types of implicit methods
11-
must be explicitly declared. Excepted are only values in local blocks
12-
where the type may still be inferred:
13-
```scala
14-
class C {
15-
16-
val ctx: Context = ... // ok
17-
18-
/*!*/ implicit val x = ... // error: type must be given explicitly
19-
20-
/*!*/ implicit def y = ... // error: type must be given explicitly
21-
22-
val y = {
23-
implicit val ctx = this.ctx // ok
24-
...
25-
}
26-
```
27-
2. Nesting is now taken into account for selecting an implicit.
28-
Consider for instance the following scenario:
29-
```scala
30-
def f(implicit i: C) = {
31-
def g(implicit j: C) = {
32-
implicitly[C]
33-
}
10+
**1.** Types of implicit values and result types of implicit methods
11+
must be explicitly declared. Excepted are only values in local blocks
12+
where the type may still be inferred:
13+
```scala
14+
class C {
15+
16+
val ctx: Context = ... // ok
17+
18+
/*!*/ implicit val x = ... // error: type must be given explicitly
19+
20+
/*!*/ implicit def y = ... // error: type must be given explicitly
21+
22+
val y = {
23+
implicit val ctx = this.ctx // ok
24+
...
25+
}
26+
```
27+
**2.** Nesting is now taken into account for selecting an implicit.Consider for instance the following scenario:
28+
```scala
29+
def f(implicit i: C) = {
30+
def g(implicit j: C) = {
31+
implicitly[C]
3432
}
35-
```
36-
This will now resolve the `implicitly` call to `j`, because `j` is nested
37-
more deeply than `i`. Previously, this would have resulted in an
38-
ambiguity error. The previous possibility of an implicit search failure
39-
due to _shadowing_ (where an implicit is hidden by a nested definition)
40-
no longer applies.
41-
42-
3. Package prefixes no longer contribute to the implicit search scope of a type.
43-
Example:
44-
```scala
45-
package p
46-
given a as A
47-
48-
object o {
49-
given b as B
50-
type C
51-
}
52-
```
53-
Both `a` and `b` are visible as implicits at the point of the definition
54-
of `type C`. However, a reference to `p.o.C` outside of package `p` will
55-
have only `b` in its implicit search scope but not `a`.
56-
57-
4. The treatment of ambiguity errors has changed. If an ambiguity is encountered
58-
in some recursive step of an implicit search, the ambiguity is propagated to the caller.
59-
Example: Say you have the following definitions:
60-
```scala
61-
class A
62-
class B extends C
63-
class C
64-
implicit def a1: A
65-
implicit def a2: A
66-
implicit def b(implicit a: A): B
67-
implicit def c: C
68-
```
69-
and the query `implicitly[C]`.
70-
71-
This query would now be classified as ambiguous. This makes sense, after all
72-
there are two possible solutions, `b(a1)` and `b(a2)`, neither of which is better
73-
than the other and both of which are better than the third solution, `c`.
74-
By contrast, Scala 2 would have rejected the search for `A` as
75-
ambiguous, and subsequently have classified the query `b(implicitly[A])` as a normal fail,
76-
which means that the alternative `c` would be chosen as solution!
77-
78-
Scala 2's somewhat puzzling behavior with respect to ambiguity has been exploited to implement
79-
the analogue of a "negated" search in implicit resolution, where a query `Q1` fails if some
80-
other query `Q2` succeeds and `Q1` succeeds if `Q2` fails. With the new cleaned up behavior
81-
these techniques no longer work. But there is now a new special type `scala.implicits.Not`
82-
which implements negation directly. For any query type `Q`: `Not[Q]` succeeds if and only if
83-
the implicit search for `Q` fails.
84-
85-
5. The treatment of divergence errors has also changed. A divergent implicit is
86-
treated as a normal failure, after which alternatives are still tried. This also makes
87-
sense: Encountering a divergent implicit means that we assume that no finite
88-
solution can be found on the corresponding path, but another path can still be tried. By contrast
89-
most (but not all) divergence errors in Scala 2 would terminate the implicit
90-
search as a whole.
91-
92-
6. Scala-2 gives a lower level of priority to implicit conversions with call-by-name
93-
parameters relative to implicit conversions with call-by-value parameters. Dotty
94-
drops this distinction. So the following code snippet would be ambiguous in Dotty:
95-
```scala
96-
implicit def conv1(x: Int): A = new A(x)
97-
implicit def conv2(x: => Int): A = new A(x)
98-
def buzz(y: A) = ???
99-
buzz(1) // error: ambiguous
100-
```
101-
7. The rule for picking a _most specific_ alternative among a set of overloaded or implicit
102-
alternatives is refined to take context parameters into account. All else
103-
being equal, an alternative that takes some context parameters is taken to be less specific
104-
than an alternative that takes none. If both alternatives take context parameters, we try
105-
to choose between them as if they were methods with regular parameters.
106-
The following paragraph in the SLS is affected by this change:
107-
108-
_Original version:_
109-
110-
> An alternative A is _more specific_ than an alternative B if the relative weight of A over B is greater than the relative weight of B over A.
111-
112-
_Modified version:_
113-
114-
An alternative A is _more specific_ than an alternative B if
115-
116-
- the relative weight of A over B is greater than the relative weight of B over A, or
117-
- the relative weights are the same, and A takes no implicit parameters but B does, or
118-
- the relative weights are the same, both A and B take implicit parameters, and
119-
A is more specific than B if all implicit parameters in either alternative are
120-
replaced by regular parameters.
33+
}
34+
```
35+
This will now resolve the `implicitly` call to `j`, because `j` is nested
36+
more deeply than `i`. Previously, this would have resulted in an
37+
ambiguity error. The previous possibility of an implicit search failure
38+
due to _shadowing_ (where an implicit is hidden by a nested definition)
39+
no longer applies.
40+
41+
**3.** Package prefixes no longer contribute to the implicit search scope of a type. Example:
42+
```scala
43+
package p
44+
given a as A
45+
46+
object o {
47+
given b as B
48+
type C
49+
}
50+
```
51+
Both `a` and `b` are visible as implicits at the point of the definition
52+
of `type C`. However, a reference to `p.o.C` outside of package `p` will
53+
have only `b` in its implicit search scope but not `a`.
54+
55+
**4.** The treatment of ambiguity errors has changed. If an ambiguity is encountered in some recursive step of an implicit search, the ambiguity is propagated to the caller.
56+
57+
Example: Say you have the following definitions:
58+
```scala
59+
class A
60+
class B extends C
61+
class C
62+
implicit def a1: A
63+
implicit def a2: A
64+
implicit def b(implicit a: A): B
65+
implicit def c: C
66+
```
67+
and the query `implicitly[C]`.
68+
69+
This query would now be classified as ambiguous. This makes sense, after all
70+
there are two possible solutions, `b(a1)` and `b(a2)`, neither of which is better
71+
than the other and both of which are better than the third solution, `c`.
72+
By contrast, Scala 2 would have rejected the search for `A` as
73+
ambiguous, and subsequently have classified the query `b(implicitly[A])` as a normal fail,
74+
which means that the alternative `c` would be chosen as solution!
75+
76+
Scala 2's somewhat puzzling behavior with respect to ambiguity has been exploited to implement
77+
the analogue of a "negated" search in implicit resolution, where a query `Q1` fails if some
78+
other query `Q2` succeeds and `Q1` succeeds if `Q2` fails. With the new cleaned up behavior
79+
these techniques no longer work. But there is now a new special type `scala.implicits.Not`
80+
which implements negation directly. For any query type `Q`: `Not[Q]` succeeds if and only if
81+
the implicit search for `Q` fails.
82+
83+
**5.** The treatment of divergence errors has also changed. A divergent implicit is treated as a normal failure, after which alternatives are still tried. This also makes sense: Encountering a divergent implicit means that we assume that no finite solution can be found on the corresponding path, but another path can still be tried. By contrast,
84+
most (but not all) divergence errors in Scala 2 would terminate the implicit search as a whole.
85+
86+
**6.** Scala-2 gives a lower level of priority to implicit conversions with call-by-name parameters relative to implicit conversions with call-by-value parameters. Dotty drops this distinction. So the following code snippet would be ambiguous in Dotty:
87+
88+
```scala
89+
implicit def conv1(x: Int): A = new A(x)
90+
implicit def conv2(x: => Int): A = new A(x)
91+
def buzz(y: A) = ???
92+
buzz(1) // error: ambiguous
93+
```
94+
**7.** The rule for picking a _most specific_ alternative among a set of overloaded or implicit alternatives is refined to take context parameters into account. All else being equal, an alternative that takes some context parameters is taken to be less specific than an alternative that takes none. If both alternatives take context parameters, we try to choose between them as if they were methods with regular parameters. The following paragraph in the SLS is affected by this change:
95+
96+
_Original version:_
97+
98+
> An alternative A is _more specific_ than an alternative B if the relative weight of A over B is greater than the relative weight of B over A.
99+
100+
_Modified version:_
101+
102+
An alternative A is _more specific_ than an alternative B if
103+
104+
- the relative weight of A over B is greater than the relative weight of B over A, or
105+
- the relative weights are the same, and A takes no implicit parameters but B does, or
106+
- the relative weights are the same, both A and B take implicit parameters, and A is more specific than B if all implicit parameters in either alternative are replaced by regular parameters.
107+
108+
**8.** The previous disambiguation of implicits based on inheritance depth is refined to make it transitive. Transitivity is important to guarantee that search outcomes are compilation-order independent. Here's a scenario where the previous rules violated transitivity:
109+
```scala
110+
class A extends B
111+
object A { given a ... }
112+
class B
113+
object B extends C { given b ... }
114+
class C { given c }
115+
```
116+
Here `a` is more specific than `b` since the companion class `A` is a subclass of the companion class `B`. Also, `b` is more specific than `c`
117+
since `object B` extends class `C`. But `a` is not more specific than `c`. This means if `a, b, c` are all applicable implicits, it makes
118+
a difference in what order they are compared. If we compare `b` and `c`
119+
first, we keep `b` and drop `c`. Then, comparing `a` with `b` we keep `a`. But if we compare `a` with `c` first, we fail with an ambiguity error.
120+
121+
The new rules are as follows: An implicit `a` defined in `A` is more specific than an implicit `b` defined in `B` if
122+
123+
- `A` extends `B`, or
124+
- `A` is an object and the companion class of `A` extends `B`, or
125+
- `A` and `B` are objects,
126+
`B` does not inherit any implicit members from base classes (*),
127+
and the companion class of `A` extends the companion class of `B`.
128+
129+
Condition (*) is new. It is necessary to ensure that the defined relation is transitive.
130+
131+
132+
133+
121134

122135
[//]: # todo: expand with precise rules

0 commit comments

Comments
 (0)