Merge pull request #4701 from mbaulch/fix4675

pickBestCandidate: pre-calculate candidates when symbol table modified

4 files changed, 99 insertions, 42 deletions

diff --git a/compiler/semcall.nim b/compiler/semcall.nim
index b3fc020b1..5f7e5084b 100644
--- a/compiler/semcall.nim
+++ b/compiler/semcall.nim
@@ -34,55 +34,85 @@ proc sameMethodDispatcher(a, b: PSym): bool =
 
 proc determineType(c: PContext, s: PSym)
 
+proc initCandidateSymbols(c: PContext, headSymbol: PNode,
+                       initialBinding: PNode,
+                       filter: TSymKinds,
+                       best, alt: var TCandidate,
+                       o: var TOverloadIter): seq[tuple[s: PSym, scope: int]] =
+  result = @[]
+  var symx = initOverloadIter(o, c, headSymbol)
+  while symx != nil:
+    if symx.kind in filter:
+      result.add((symx, o.lastOverloadScope))
+      symx = nextOverloadIter(o, c, headSymbol)
+  if result.len > 0:
+    initCandidate(c, best, result[0].s, initialBinding, result[0].scope)
+    initCandidate(c, alt, result[0].s, initialBinding, result[0].scope)
+    best.state = csNoMatch
+
 proc pickBestCandidate(c: PContext, headSymbol: PNode,
                        n, orig: PNode,
                        initialBinding: PNode,
                        filter: TSymKinds,
                        best, alt: var TCandidate,
                        errors: var CandidateErrors) =
-  while true:
-    block pickAttempt:
-      var o: TOverloadIter
-      var sym = initOverloadIter(o, c, headSymbol)
-      # Thanks to the lazy semchecking for operands, we need to check whether
-      # 'initCandidate' modifies the symbol table (via semExpr).
-      # This can occur in cases like 'init(a, 1, (var b = new(Type2); b))'
-      let counterInitial = c.currentScope.symbols.counter
+  var o: TOverloadIter
+  var sym = initOverloadIter(o, c, headSymbol)
+  var scope = o.lastOverloadScope
+  # Thanks to the lazy semchecking for operands, we need to check whether
+  # 'initCandidate' modifies the symbol table (via semExpr).
+  # This can occur in cases like 'init(a, 1, (var b = new(Type2); b))'
+  let counterInitial = c.currentScope.symbols.counter
+  var syms: seq[tuple[s: PSym, scope: int]]
+  var nextSymIndex = 0
+  while sym != nil:
+    if sym.kind in filter:
       # Initialise 'best' and 'alt' with the first available symbol
-      while sym != nil:
-        if sym.kind in filter:
-          initCandidate(c, best, sym, initialBinding, o.lastOverloadScope)
-          initCandidate(c, alt, sym, initialBinding, o.lastOverloadScope)
-          best.state = csNoMatch
-          break
-        else:
-          sym = nextOverloadIter(o, c, headSymbol)
-      var z: TCandidate
-      while sym != nil:
-        if sym.kind notin filter:
-          sym = nextOverloadIter(o, c, headSymbol)
-          continue
-        determineType(c, sym)
-        initCandidate(c, z, sym, initialBinding, o.lastOverloadScope)
-        if c.currentScope.symbols.counter != counterInitial: break pickAttempt
-        matches(c, n, orig, z)
-        if errors != nil:
-          errors.safeAdd((sym, int z.mutabilityProblem))
-          if z.errors != nil:
-            for err in z.errors:
-              errors.add(err)
-        if z.state == csMatch:
-          # little hack so that iterators are preferred over everything else:
-          if sym.kind == skIterator: inc(z.exactMatches, 200)
-          case best.state
-          of csEmpty, csNoMatch: best = z
-          of csMatch:
-            var cmp = cmpCandidates(best, z)
-            if cmp < 0: best = z   # x is better than the best so far
-            elif cmp == 0: alt = z # x is as good as the best so far
-            else: discard
-        sym = nextOverloadIter(o, c, headSymbol)
-    break # pick attempt was successful
+      initCandidate(c, best, sym, initialBinding, scope)
+      initCandidate(c, alt, sym, initialBinding, scope)
+      best.state = csNoMatch
+      break
+    else:
+      sym = nextOverloadIter(o, c, headSymbol)
+      scope = o.lastOverloadScope
+  var z: TCandidate
+  while sym != nil:
+    if sym.kind notin filter:
+      sym = nextOverloadIter(o, c, headSymbol)
+      scope = o.lastOverloadScope
+      continue
+    determineType(c, sym)
+    initCandidate(c, z, sym, initialBinding, scope)
+    if c.currentScope.symbols.counter == counterInitial or syms != nil:
+      matches(c, n, orig, z)
+      if errors != nil:
+        errors.safeAdd((sym, int z.mutabilityProblem))
+        if z.errors != nil:
+          for err in z.errors:
+            errors.add(err)
+      if z.state == csMatch:
+        # little hack so that iterators are preferred over everything else:
+        if sym.kind == skIterator: inc(z.exactMatches, 200)
+        case best.state
+        of csEmpty, csNoMatch: best = z
+        of csMatch:
+          var cmp = cmpCandidates(best, z)
+          if cmp < 0: best = z   # x is better than the best so far
+          elif cmp == 0: alt = z # x is as good as the best so far
+    else:
+      # Symbol table has been modified. Restart and pre-calculate all syms
+      # before any further candidate init and compare. SLOW, but rare case.
+      syms = initCandidateSymbols(c, headSymbol, initialBinding, filter, best, alt, o)
+    if syms == nil:
+      sym = nextOverloadIter(o, c, headSymbol)
+      scope = o.lastOverloadScope
+    elif nextSymIndex < syms.len:
+      # rare case: retrieve the next pre-calculated symbol
+      sym = syms[nextSymIndex].s
+      scope = syms[nextSymIndex].scope
+      nextSymIndex += 1
+    else:
+      break
 
 proc notFoundError*(c: PContext, n: PNode, errors: CandidateErrors) =
   # Gives a detailed error message; this is separated from semOverloadedCall,
diff --git a/tests/overload/importA.nim b/tests/overload/importA.nim
new file mode 100644
index 000000000..f045d11b4
--- /dev/null
+++ b/tests/overload/importA.nim
@@ -0,0 +1,5 @@
+type
+  Field* = object
+    elemSize*: int
+
+template `+`*(x: untyped, y: Field): untyped = x
diff --git a/tests/overload/importB.nim b/tests/overload/importB.nim
new file mode 100644
index 000000000..2dc3adf7a
--- /dev/null
+++ b/tests/overload/importB.nim
@@ -0,0 +1,15 @@
+type
+  Foo*[T] = object
+    v*: T
+
+template `+`*(x: Foo, y: Foo): untyped = x
+
+template newvar*(r: untyped): untyped {.dirty.} =
+  var r: float
+
+template t1*(x: Foo): untyped =
+  newvar(y1)
+  x
+template t2*(x: Foo): untyped =
+  newvar(y2)
+  x
diff --git a/tests/overload/timport.nim b/tests/overload/timport.nim
new file mode 100644
index 000000000..8ea65e54d
--- /dev/null
+++ b/tests/overload/timport.nim
@@ -0,0 +1,7 @@
+# issue 4675
+import importA  # comment this out to make it work
+import importB
+
+var x: Foo[float]
+var y: Foo[float]
+let r = t1(x) + t2(y)