Skip to content
This repository
Browse code

[nomaster] SI-7291: No exception throwing for diverging implicit expa…

…nsion

Since we don't throw exceptions for normal errors it was a bit odd
that we don't do that for DivergingImplicit.
As SI-7291 shows, the logic behind catching/throwing exception
was broken for divergence. Instead of patching it, I rewrote
the mechanism so that we now another SearchFailure type related
to diverging expansion, similar to ambiguous implicit scenario.
The logic to prevent diverging expansion from stopping the search
had to be slightly adapted but works as usual.

The upside is that we don't have to catch diverging implicit
for example in the presentation compiler which was again showing
that something was utterly broken with the exception approach.

NOTE: This is a partial backport of #2428,
with a fix for SI-7291, but without removal of error kinds (the former is
absolutely necessary, while the latter is nice to have, but not a must,
therefore I'm not risking porting it to 2.10.x). Also, the fix for SI-7291
is hidden behind a flag named -Xdivergence211 in order not to occasionally
break the code, which relies on pre-fix behavior.
  • Loading branch information...
commit fdead2b3793fd530e05331649e655576f30e59e9 1 parent 8168f11
Hubert Plociniczak authored February 19, 2013 xeno-by committed May 11, 2013
9  src/compiler/scala/tools/nsc/interactive/Global.scala
@@ -1174,8 +1174,13 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
1174 1174
         debugLog("type error caught: "+ex)
1175 1175
         alt
1176 1176
       case ex: DivergentImplicit =>
1177  
-        debugLog("divergent implicit caught: "+ex)
1178  
-        alt
  1177
+        if (settings.Xdivergence211.value) {
  1178
+          debugLog("this shouldn't happen. DivergentImplicit exception has been thrown with -Xdivergence211 turned on: "+ex)
  1179
+          alt
  1180
+        } else {
  1181
+          debugLog("divergent implicit caught: "+ex)
  1182
+          alt
  1183
+        }
1179 1184
     }
1180 1185
   }
1181 1186
 
1  src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
@@ -110,6 +110,7 @@ trait ScalaSettings extends AbsScalaSettings
110 110
   val XoldPatmat    = BooleanSetting    ("-Xoldpatmat", "Use the pre-2.10 pattern matcher. Otherwise, the 'virtualizing' pattern matcher is used in 2.10.")
111 111
   val XnoPatmatAnalysis = BooleanSetting ("-Xno-patmat-analysis", "Don't perform exhaustivity/unreachability analysis. Also, ignore @switch annotation.")
112 112
   val XfullLubs     = BooleanSetting    ("-Xfull-lubs", "Retains pre 2.10 behavior of less aggressive truncation of least upper bounds.")
  113
+  val Xdivergence211 = BooleanSetting   ("-Xdivergence211", "Turn on the 2.11 behavior of implicit divergence not terminating recursive implicit searches (SI-7291).")
113 114
 
114 115
   /** Compatibility stubs for options whose value name did
115 116
    *  not previously match the option name.
29  src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
@@ -60,6 +60,24 @@ trait ContextErrors {
60 60
     def errPos = tree.pos
61 61
   }
62 62
 
  63
+  // Unlike other type errors diverging implicit expansion
  64
+  // will be re-issued explicitly on failed implicit argument search.
  65
+  // This is because we want to:
  66
+  // 1) provide better error message than just "implicit not found"
  67
+  // 2) provide the type of the implicit parameter for which we got diverging expansion
  68
+  //    (pt at the point of divergence gives less information to the user)
  69
+  // Note: it is safe to delay error message generation in this case
  70
+  // becasue we don't modify implicits' infos.
  71
+  // only issued when -Xdivergence211 is turned on
  72
+  case class DivergentImplicitTypeError(tree: Tree, pt0: Type, sym: Symbol) extends AbsTypeError {
  73
+    def errPos: Position = tree.pos
  74
+    def errMsg: String   = errMsgForPt(pt0)
  75
+    def kind = ErrorKinds.Divergent
  76
+    def withPt(pt: Type): AbsTypeError = NormalTypeError(tree, errMsgForPt(pt), kind)
  77
+    private def errMsgForPt(pt: Type) =
  78
+      s"diverging implicit expansion for type ${pt}\nstarting with ${sym.fullLocationString}"
  79
+  }
  80
+
63 81
   case class AmbiguousTypeError(underlyingTree: Tree, errPos: Position, errMsg: String, kind: ErrorKind = ErrorKinds.Ambiguous) extends AbsTypeError
64 82
 
65 83
   case class PosAndMsgTypeError(errPos: Position, errMsg: String, kind: ErrorKind = ErrorKinds.Normal) extends AbsTypeError
@@ -73,6 +91,7 @@ trait ContextErrors {
73 91
       issueTypeError(SymbolTypeError(sym, msg))
74 92
     }
75 93
 
  94
+    // only called when -Xdivergence211 is turned off
76 95
     def issueDivergentImplicitsError(tree: Tree, msg: String)(implicit context: Context) {
77 96
       issueTypeError(NormalTypeError(tree, msg, ErrorKinds.Divergent))
78 97
     }
@@ -1152,9 +1171,13 @@ trait ContextErrors {
1152 1171
     }
1153 1172
 
1154 1173
     def DivergingImplicitExpansionError(tree: Tree, pt: Type, sym: Symbol)(implicit context0: Context) =
1155  
-      issueDivergentImplicitsError(tree,
1156  
-          "diverging implicit expansion for type "+pt+"\nstarting with "+
1157  
-          sym.fullLocationString)
  1174
+      if (settings.Xdivergence211.value) {
  1175
+        issueTypeError(DivergentImplicitTypeError(tree, pt, sym))
  1176
+      } else {
  1177
+        issueDivergentImplicitsError(tree,
  1178
+            "diverging implicit expansion for type "+pt+"\nstarting with "+
  1179
+            sym.fullLocationString)
  1180
+      }
1158 1181
   }
1159 1182
 
1160 1183
   object NamesDefaultsErrorsGen {
78  src/compiler/scala/tools/nsc/typechecker/Implicits.scala
@@ -80,7 +80,7 @@ trait Implicits {
80 80
       printTyping("typing implicit: %s %s".format(tree, context.undetparamsString))
81 81
     val implicitSearchContext = context.makeImplicit(reportAmbiguous)
82 82
     val result = new ImplicitSearch(tree, pt, isView, implicitSearchContext, pos).bestImplicit
83  
-    if (saveAmbiguousDivergent && implicitSearchContext.hasErrors) {
  83
+    if ((result.isFailure || !settings.Xdivergence211.value) && saveAmbiguousDivergent && implicitSearchContext.hasErrors) {
84 84
       context.updateBuffer(implicitSearchContext.errBuffer.filter(err => err.kind == ErrorKinds.Ambiguous || err.kind == ErrorKinds.Divergent))
85 85
       debugwarn("update buffer: " + implicitSearchContext.errBuffer)
86 86
     }
@@ -152,6 +152,8 @@ trait Implicits {
152 152
 
153 153
     def isFailure          = false
154 154
     def isAmbiguousFailure = false
  155
+    // only used when -Xdivergence211 is turned on
  156
+    def isDivergent        = false
155 157
     final def isSuccess    = !isFailure
156 158
   }
157 159
 
@@ -159,6 +161,12 @@ trait Implicits {
159 161
     override def isFailure = true
160 162
   }
161 163
 
  164
+  // only used when -Xdivergence211 is turned on
  165
+  lazy val DivergentSearchFailure = new SearchResult(EmptyTree, EmptyTreeTypeSubstituter) {
  166
+    override def isFailure   = true
  167
+    override def isDivergent = true
  168
+  }
  169
+
162 170
   lazy val AmbiguousSearchFailure = new SearchResult(EmptyTree, EmptyTreeTypeSubstituter) {
163 171
     override def isFailure          = true
164 172
     override def isAmbiguousFailure = true
@@ -425,8 +433,10 @@ trait Implicits {
425 433
       (context.openImplicits find { case OpenImplicit(info, tp, tree1) => !info.sym.isMacro && tree1.symbol == tree.symbol && dominates(pt, tp)}) match {
426 434
          case Some(pending) =>
427 435
            //println("Pending implicit "+pending+" dominates "+pt+"/"+undetParams) //@MDEBUG
428  
-           throw DivergentImplicit
  436
+           if (settings.Xdivergence211.value) DivergentSearchFailure
  437
+           else throw DivergentImplicit
429 438
          case None =>
  439
+           def pre211DivergenceLogic() = {
430 440
            try {
431 441
              context.openImplicits = OpenImplicit(info, pt, tree) :: context.openImplicits
432 442
              // println("  "*context.openImplicits.length+"typed implicit "+info+" for "+pt) //@MDEBUG
@@ -444,6 +454,24 @@ trait Implicits {
444 454
            } finally {
445 455
              context.openImplicits = context.openImplicits.tail
446 456
            }
  457
+           }
  458
+           def post211DivergenceLogic() = {
  459
+             try {
  460
+               context.openImplicits = OpenImplicit(info, pt, tree) :: context.openImplicits
  461
+               // println("  "*context.openImplicits.length+"typed implicit "+info+" for "+pt) //@MDEBUG
  462
+               val result = typedImplicit0(info, ptChecked, isLocal)
  463
+               if (result.isDivergent) {
  464
+                 //println("DivergentImplicit for pt:"+ pt +", open implicits:"+context.openImplicits) //@MDEBUG
  465
+                 if (context.openImplicits.tail.isEmpty && !pt.isErroneous)
  466
+                   DivergingImplicitExpansionError(tree, pt, info.sym)(context)
  467
+               }
  468
+               result
  469
+             } finally {
  470
+               context.openImplicits = context.openImplicits.tail
  471
+             }
  472
+           }
  473
+           if (settings.Xdivergence211.value) post211DivergenceLogic()
  474
+           else pre211DivergenceLogic()
447 475
        }
448 476
     }
449 477
 
@@ -812,6 +840,9 @@ trait Implicits {
812 840
 
813 841
       /** Preventing a divergent implicit from terminating implicit search,
814 842
        *  so that if there is a best candidate it can still be selected.
  843
+       *
  844
+       *  The old way of handling divergence.
  845
+       *  Only enabled when -Xdivergence211 is turned off.
815 846
        */
816 847
       private var divergence = false
817 848
       private val divergenceHandler: PartialFunction[Throwable, SearchResult] = {
@@ -824,6 +855,28 @@ trait Implicits {
824 855
         }
825 856
       }
826 857
 
  858
+      /** Preventing a divergent implicit from terminating implicit search,
  859
+       *  so that if there is a best candidate it can still be selected.
  860
+       *
  861
+       *  The new way of handling divergence.
  862
+       *  Only enabled when -Xdivergence211 is turned on.
  863
+       */
  864
+      object DivergentImplicitRecovery {
  865
+        // symbol of the implicit that caused the divergence.
  866
+        // Initially null, will be saved on first diverging expansion.
  867
+        private var implicitSym: Symbol    = _
  868
+        private var countdown: Int = 1
  869
+
  870
+        def sym: Symbol = implicitSym
  871
+        def apply(search: SearchResult, i: ImplicitInfo): SearchResult =
  872
+          if (search.isDivergent && countdown > 0) {
  873
+            countdown -= 1
  874
+            implicitSym = i.sym
  875
+            log("discarding divergent implicit ${implicitSym} during implicit search")
  876
+            SearchFailure
  877
+          } else search
  878
+      }
  879
+
827 880
       /** Sorted list of eligible implicits.
828 881
        */
829 882
       val eligible = {
@@ -850,11 +903,20 @@ trait Implicits {
850 903
       @tailrec private def rankImplicits(pending: Infos, acc: Infos): Infos = pending match {
851 904
         case Nil      => acc
852 905
         case i :: is  =>
853  
-          def tryImplicitInfo(i: ImplicitInfo) =
  906
+          def pre211tryImplicitInfo(i: ImplicitInfo) =
854 907
             try typedImplicit(i, ptChecked = true, isLocal)
855 908
             catch divergenceHandler
856 909
 
857  
-          tryImplicitInfo(i) match {
  910
+          def post211tryImplicitInfo(i: ImplicitInfo) =
  911
+            DivergentImplicitRecovery(typedImplicit(i, ptChecked = true, isLocal), i)
  912
+
  913
+          {
  914
+            if (settings.Xdivergence211.value) post211tryImplicitInfo(i)
  915
+            else pre211tryImplicitInfo(i)
  916
+          } match {
  917
+            // only used if -Xdivergence211 is turned on
  918
+            case sr if sr.isDivergent =>
  919
+              Nil
858 920
             case sr if sr.isFailure =>
859 921
               // We don't want errors that occur during checking implicit info
860 922
               // to influence the check of further infos.
@@ -906,9 +968,10 @@ trait Implicits {
906 968
           /** If there is no winner, and we witnessed and caught divergence,
907 969
            *  now we can throw it for the error message.
908 970
            */
909  
-          if (divergence)
910  
-            throw DivergentImplicit
911  
-          else invalidImplicits take 1 foreach { sym =>
  971
+          if (divergence || DivergentImplicitRecovery.sym != null) {
  972
+            if (settings.Xdivergence211.value) DivergingImplicitExpansionError(tree, pt, DivergentImplicitRecovery.sym)(context)
  973
+            else throw DivergentImplicit
  974
+          } else invalidImplicits take 1 foreach { sym =>
912 975
             def isSensibleAddendum = pt match {
913 976
               case Function1(_, out) => out <:< sym.tpe.finalResultType
914 977
               case tp                => tp <:< sym.tpe.finalResultType
@@ -1478,5 +1541,6 @@ object ImplicitsStats {
1478 1541
   val implicitCacheHits   = Statistics.newSubCounter("implicit cache hits", implicitCacheAccs)
1479 1542
 }
1480 1543
 
  1544
+// only used when -Xdivergence211 is turned off
1481 1545
 class DivergentImplicit extends Exception
1482 1546
 object DivergentImplicit extends DivergentImplicit
7  src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -138,13 +138,18 @@ trait Typers extends Modes with Adaptations with Tags {
138 138
             mkArg = mkNamedArg // don't pass the default argument (if any) here, but start emitting named arguments for the following args
139 139
             if (!param.hasDefault && !paramFailed) {
140 140
               context.errBuffer.find(_.kind == ErrorKinds.Divergent) match {
141  
-                case Some(divergentImplicit) =>
  141
+                case Some(divergentImplicit) if !settings.Xdivergence211.value =>
142 142
                   // DivergentImplicit error has higher priority than "no implicit found"
143 143
                   // no need to issue the problem again if we are still in silent mode
144 144
                   if (context.reportErrors) {
145 145
                     context.issue(divergentImplicit)
146 146
                     context.condBufferFlush(_.kind  == ErrorKinds.Divergent)
147 147
                   }
  148
+                case Some(divergentImplicit: DivergentImplicitTypeError) if settings.Xdivergence211.value =>
  149
+                  if (context.reportErrors) {
  150
+                    context.issue(divergentImplicit.withPt(paramTp))
  151
+                    context.condBufferFlush(_.kind  == ErrorKinds.Divergent)
  152
+                  }
148 153
                 case None =>
149 154
                   NoImplicitFoundError(fun, param)
150 155
               }
2  test/files/neg/t696.check → test/files/neg/t696a.check
... ...
@@ -1,4 +1,4 @@
1  
-t696.scala:4: error: diverging implicit expansion for type TypeUtil0.Type[Any]
  1
+t696a.scala:4: error: diverging implicit expansion for type TypeUtil0.Type[Any]
2 2
 starting with method WithType in object TypeUtil0
3 3
   as[Any](null);
4 4
          ^
0  test/files/neg/t696a.flags
No changes.
0  test/files/neg/t696.scala → test/files/neg/t696a.scala
File renamed without changes
9  test/files/neg/t696b.check
... ...
@@ -0,0 +1,9 @@
  1
+t696b.scala:5: error: diverging implicit expansion for type TypeUtil0.Type[Any]
  2
+starting with method WithType in object TypeUtil0
  3
+  as[Any](null)
  4
+         ^
  5
+t696b.scala:6: error: diverging implicit expansion for type TypeUtil0.Type[X]
  6
+starting with method WithType in object TypeUtil0
  7
+  def foo[X]() = as[X](null)
  8
+                      ^
  9
+two errors found
1  test/files/neg/t696b.flags
... ...
@@ -0,0 +1 @@
  1
+-Xdivergence211
7  test/files/neg/t696b.scala
... ...
@@ -0,0 +1,7 @@
  1
+object TypeUtil0 {
  2
+  trait Type[+T]
  3
+  implicit def WithType[S,T](implicit tpeS : Type[S], tpeT : Type[T]) : Type[S with T] = null
  4
+  def as[T](x : Any)(implicit tpe : Type[T]) = null
  5
+  as[Any](null)
  6
+  def foo[X]() = as[X](null)
  7
+}
1  test/files/run/t7291a.check
... ...
@@ -0,0 +1 @@
  1
+conjure
0  test/files/run/t7291a.flags
No changes.
19  test/files/run/t7291a.scala
... ...
@@ -0,0 +1,19 @@
  1
+trait Fooable[T]
  2
+object Fooable {
  3
+  implicit def conjure[T]: Fooable[T] = {
  4
+    println("conjure")
  5
+    new Fooable[T]{}
  6
+  }
  7
+
  8
+}
  9
+
  10
+object Test {
  11
+  implicit def traversable[T, Coll[_] <: Traversable[_]](implicit
  12
+elem: Fooable[T]): Fooable[Coll[T]] = {
  13
+    println("traversable")
  14
+    new Fooable[Coll[T]]{}
  15
+  }
  16
+  def main(args: Array[String]) {
  17
+    implicitly[Fooable[List[Any]]]
  18
+  }
  19
+}
2  test/files/run/t7291b.check
... ...
@@ -0,0 +1,2 @@
  1
+conjure
  2
+traversable
1  test/files/run/t7291b.flags
... ...
@@ -0,0 +1 @@
  1
+-Xdivergence211
19  test/files/run/t7291b.scala
... ...
@@ -0,0 +1,19 @@
  1
+trait Fooable[T]
  2
+object Fooable {
  3
+  implicit def conjure[T]: Fooable[T] = {
  4
+    println("conjure")
  5
+    new Fooable[T]{}
  6
+  }
  7
+
  8
+}
  9
+
  10
+object Test {
  11
+  implicit def traversable[T, Coll[_] <: Traversable[_]](implicit
  12
+elem: Fooable[T]): Fooable[Coll[T]] = {
  13
+    println("traversable")
  14
+    new Fooable[Coll[T]]{}
  15
+  }
  16
+  def main(args: Array[String]) {
  17
+    implicitly[Fooable[List[Any]]] 
  18
+  }
  19
+}

0 notes on commit fdead2b

Please sign in to comment.
Something went wrong with that request. Please try again.