Skip to content

Commit de3a82c

Browse files
oderskyodersky
authored
Merge pull request #15481 from dotty-staging/fix-15474
Find more looping implicits
2 parents 69f205d + af4c365 commit de3a82c

File tree

3 files changed

+171
-3
lines changed

3 files changed

+171
-3
lines changed

‎compiler/src/dotty/tools/dotc/transform/CheckLoopingImplicits.scala

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,28 @@ import MegaPhase.MiniPhase
66
import Contexts.*, Types.*, Symbols.*, SymDenotations.*, Flags.*
77
import ast.*
88
import Decorators.*
9-
9+
import StdNames.*
1010

1111
object CheckLoopingImplicits:
1212
val name: String = "checkLoopingImplicits"
1313
val description: String = "check that implicit defs do not call themselves in an infinite loop"
1414

15-
/** Checks that implicit defs do not call themselves in an infinite loop */
15+
/** Checks that some definitions do not call themselves in an infinite loop
16+
* This is an incomplete check, designed to catch some likely bugs instead
17+
* of being exhaustive. The situations where infinite loops are diagnosed are
18+
* 1. A given method should not directly call itself
19+
* 2. An apply method in a given object should not directly call itself
20+
* 3. A lazy val should not directly force itself
21+
* 4. An extension method should not directly call itself
22+
*
23+
* In all these cases, there are some situations which would not lead to
24+
* an infinite loop at runtime. For instance, the call could go at runtime to an
25+
* overriding version of the method or val which breaks the loop. That's why
26+
* this phase only issues warnings, not errors, and also why we restrict
27+
* checks to the 4 cases above, where a recursion is somewhat hidden.
28+
* There are also other more complicated calling patterns that could also
29+
* be diagnosed as loops with more effort. This could be improved in the future.
30+
*/
1631
class CheckLoopingImplicits extends MiniPhase:
1732
thisPhase =>
1833
import tpd._
@@ -60,6 +75,9 @@ class CheckLoopingImplicits extends MiniPhase:
6075
case Block(stats, expr) =>
6176
stats.foreach(checkNotLooping)
6277
checkNotLooping(expr)
78+
case Inlined(_, bindings, expr) =>
79+
bindings.foreach(checkNotLooping)
80+
checkNotLooping(expr)
6381
case Typed(expr, _) =>
6482
checkNotLooping(expr)
6583
case Assign(lhs, rhs) =>
@@ -84,7 +102,9 @@ class CheckLoopingImplicits extends MiniPhase:
84102
checkNotLooping(t.rhs)
85103
case _ =>
86104

87-
if sym.isOneOf(GivenOrImplicit | Lazy | ExtensionMethod) then
105+
if sym.isOneOf(GivenOrImplicit | Lazy | ExtensionMethod)
106+
|| sym.name == nme.apply && sym.owner.is(Module) && sym.owner.sourceModule.isOneOf(GivenOrImplicit)
107+
then
88108
checkNotLooping(mdef.rhs)
89109
mdef
90110
end transform

‎tests/neg-custom-args/fatal-warnings/i15474.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import scala.language.implicitConversions
2+
3+
object Test1:
4+
given c: Conversion[ String, Int ] with
5+
def apply(from: String): Int = from.toInt // error
6+
7+
object Test2:
8+
given c: Conversion[ String, Int ] = _.toInt // loop not detected, could be used as a fallback to avoid the warning.
9+
10+
object Prices {
11+
opaque type Price = BigDecimal
12+
13+
object Price{
14+
given Ordering[Price] = summon[Ordering[BigDecimal]] // error
15+
}
16+
}

‎tests/neg/i13044.check

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,135 @@
1+
-- Warning: tests/neg/i13044.scala:50:40 -------------------------------------------------------------------------------
2+
50 | implicit def typeSchema: Schema[A] = Schema.gen // error // error
3+
| ^^^^^^^^^^
4+
|Infinite loop in function body
5+
|{
6+
| val SchemaDerivation_this: Schema.type = Schema
7+
| {
8+
| val SchemaDerivation_this: (SchemaDerivation_this : Schema.type) = SchemaDerivation_this
9+
| {
10+
| val $scrutinee1:
11+
| scala.deriving.Mirror.Product{
12+
| MirroredMonoType = A; MirroredType = A; MirroredLabel = ("A" : String); MirroredElemTypes = (A, B);
13+
| MirroredElemLabels = (("a" : String), ("b" : String))
14+
| }
15+
| =
16+
| A.$asInstanceOf[
17+
| scala.deriving.Mirror.Product{
18+
| MirroredMonoType = A; MirroredType = A; MirroredLabel = ("A" : String); MirroredElemTypes = (A, B);
19+
| MirroredElemLabels = (("a" : String), ("b" : String))
20+
| }
21+
| ]
22+
| val m:
23+
| scala.deriving.Mirror.Product{
24+
| MirroredMonoType = A; MirroredType = A; MirroredLabel = ("A" : String); MirroredElemTypes = (A, B);
25+
| MirroredElemLabels = (("a" : String), ("b" : String))
26+
| }
27+
| = $scrutinee1
28+
| lazy val fields: List[Schema[Any]] =
29+
| {
30+
| val SchemaDerivation_this: (SchemaDerivation_this : (SchemaDerivation_this : Schema.type)) =
31+
| SchemaDerivation_this
32+
| {
33+
| val builder: Schema[Any] = TestApp.typeSchema.asInstanceOf[Schema[Any]]
34+
| {
35+
| val SchemaDerivation_this:
36+
| (SchemaDerivation_this : (SchemaDerivation_this : (SchemaDerivation_this : Schema.type)))
37+
| = SchemaDerivation_this
38+
| (
39+
| {
40+
| val builder: Schema[Any] =
41+
| {
42+
| val SchemaDerivation_this: Schema.type = Schema
43+
| (
44+
| {
45+
| val SchemaDerivation_this: (SchemaDerivation_this : Schema.type) = SchemaDerivation_this
46+
| {
47+
| val $scrutinee4:
48+
| scala.deriving.Mirror.Product{
49+
| MirroredMonoType = B; MirroredType = B; MirroredLabel = ("B" : String);
50+
| MirroredElemTypes = C *: EmptyTuple.type
51+
| ; MirroredElemLabels = ("c" : String) *: EmptyTuple.type
52+
| }
53+
| =
54+
| B.$asInstanceOf[
55+
| scala.deriving.Mirror.Product{
56+
| MirroredMonoType = B; MirroredType = B; MirroredLabel = ("B" : String);
57+
| MirroredElemTypes = C *: EmptyTuple.type
58+
| ; MirroredElemLabels = ("c" : String) *: EmptyTuple.type
59+
| }
60+
| ]
61+
| val m:
62+
| scala.deriving.Mirror.Product{
63+
| MirroredMonoType = B; MirroredType = B; MirroredLabel = ("B" : String);
64+
| MirroredElemTypes = C *: EmptyTuple.type
65+
| ; MirroredElemLabels = ("c" : String) *: EmptyTuple.type
66+
| }
67+
| = $scrutinee4
68+
| lazy val fields: List[Schema[Any]] =
69+
| {
70+
| val SchemaDerivation_this:
71+
| (SchemaDerivation_this : (SchemaDerivation_this : Schema.type))
72+
| = SchemaDerivation_this
73+
| {
74+
| val builder: Schema[Any] =
75+
| {
76+
| val SchemaDerivation_this: Schema.type = Schema
77+
| (
78+
| {
79+
| val SchemaDerivation_this: (...SchemaDerivation_this : ....type) =
80+
| SchemaDerivation_this
81+
| {
82+
| val $scrutinee6:
83+
| ...{
84+
| MirroredMonoType...; MirroredType...; MirroredLabel...;
85+
| MirroredElemTypes...
86+
| ; MirroredElemLabels...
87+
| }
88+
| = ....$asInstanceOf[...]
89+
| val m: ... = ...$scrutinee6
90+
| lazy val fields: ... =
91+
| {
92+
| val SchemaDerivation_this: ... = ...
93+
| ...:...
94+
| }
95+
| {
96+
| final class $anon() extends ...(), ... {
97+
| def build: ... = ...
98+
| }
99+
| ...():...
100+
| }
101+
| }:...[...]
102+
| }
103+
| :Schema[C])
104+
| }.asInstanceOf[Schema[Any]]
105+
| SchemaDerivation_this.recurse[EmptyTuple.type].::[Schema[Any]](builder)
106+
| }:List[Schema[Any]]
107+
| }
108+
| {
109+
| final class $anon() extends Object(), Schema[B] {
110+
| def build: B = ???
111+
| }
112+
| new Object with Schema[B] {...}():Schema[B]
113+
| }
114+
| }:Schema[B]
115+
| }
116+
| :Schema[B])
117+
| }.asInstanceOf[Schema[Any]]
118+
| SchemaDerivation_this.recurse[EmptyTuple.type].::[Schema[Any]](builder)
119+
| }
120+
| :List[Schema[Any]])
121+
| }.::[Schema[Any]](builder)
122+
| }:List[Schema[Any]]
123+
| }
124+
| {
125+
| final class $anon() extends Object(), Schema[A] {
126+
| def build: A = ???
127+
| }
128+
| new Object with Schema[A] {...}():Schema[A]
129+
| }
130+
| }:Schema[A]
131+
| }:Schema[A]
132+
|}
1133
-- Error: tests/neg/i13044.scala:50:40 ---------------------------------------------------------------------------------
2134
50 | implicit def typeSchema: Schema[A] = Schema.gen // error // error
3135
| ^^^^^^^^^^

0 commit comments

Comments
 (0)