Merge pull request #4701 from mbaulch/fix4675

pickBestCandidate: pre-calculate candidates when symbol table modified

compiler/semcall.nim

@@ -34,37 +34,56 @@ 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)
+  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
-      # Initialise 'best' and 'alt' with the first available symbol
+  var syms: seq[tuple[s: PSym, scope: int]]
+  var nextSymIndex = 0
   while sym != nil:
     if sym.kind in filter:
-          initCandidate(c, best, sym, initialBinding, o.lastOverloadScope)
+      # Initialise 'best' and 'alt' with the first available symbol
-          initCandidate(c, alt, sym, initialBinding, o.lastOverloadScope)
+      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, o.lastOverloadScope)
+    initCandidate(c, z, sym, initialBinding, scope)
-        if c.currentScope.symbols.counter != counterInitial: break pickAttempt
+    if c.currentScope.symbols.counter == counterInitial or syms != nil:
       matches(c, n, orig, z)
       if errors != nil:
         errors.safeAdd((sym, int z.mutabilityProblem))
@@ -80,9 +99,20 @@ proc pickBestCandidate(c: PContext, headSymbol: PNode,
           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
+    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)
-    break # pick attempt was successful
+      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,

tests/overload/importA.nim

@@ -0,0 +1,5 @@
+type
+  Field* = object
+    elemSize*: int
+
+template `+`*(x: untyped, y: Field): untyped = x

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

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)