From ce9a57fcfd21a1a3cb17087107c246f0492bec7f Mon Sep 17 00:00:00 2001
From: Reimer Behrends <behrends@gmail.com>
Date: Sun, 2 Nov 2014 23:35:41 +0100
Subject: [PATCH] Fix dispatcher creation for method prototypes.

When method prototypes were involved (e.g. forward declarations
for mutual recursion), calls were sometimes dispatched to the
wrong method implementation. One of the reasons was that method
dispatchers were then not always attached to method ASTs in
the correct place.
---
 compiler/cgmeth.nim      | 88 +++++++++++++++++++++++++++++-----------
 compiler/transf.nim      |  7 ++--
 tests/method/tmproto.nim | 25 ++++++++++++
 3 files changed, 92 insertions(+), 28 deletions(-)
 create mode 100644 tests/method/tmproto.nim

diff --git a/compiler/cgmeth.nim b/compiler/cgmeth.nim
index 9755bc674..d881226d4 100644
--- a/compiler/cgmeth.nim
+++ b/compiler/cgmeth.nim
@@ -11,7 +11,7 @@
 
 import 
   intsets, options, ast, astalgo, msgs, idents, renderer, types, magicsys,
-  sempass2
+  sempass2, strutils
 
 proc genConv(n: PNode, d: PType, downcast: bool): PNode = 
   var dest = skipTypes(d, abstractPtrs)
@@ -44,7 +44,8 @@ proc methodCall*(n: PNode): PNode =
     result.sons[i] = genConv(result.sons[i], disp.typ.sons[i], true)
 
 # save for incremental compilation:
-var gMethods: seq[TSymSeq] = @[]
+var
+  gMethods: seq[tuple[methods: TSymSeq, dispatcher: PSym]] = @[]
 
 proc sameMethodBucket(a, b: PSym): bool = 
   result = false
@@ -80,32 +81,70 @@ proc attachDispatcher(s: PSym, dispatcher: PNode) =
   else:
     s.ast.add(dispatcher)
 
+proc createDispatcher(s: PSym): PSym =
+  var disp = copySym(s)
+  incl(disp.flags, sfDispatcher)
+  excl(disp.flags, sfExported)
+  disp.typ = copyType(disp.typ, disp.typ.owner, false)
+  # we can't inline the dispatcher itself (for now):
+  if disp.typ.callConv == ccInline: disp.typ.callConv = ccDefault
+  disp.ast = copyTree(s.ast)
+  disp.ast.sons[bodyPos] = ast.emptyNode
+  disp.loc.r = nil
+  if s.typ.sons[0] != nil:
+    if disp.ast.sonsLen > resultPos:
+      disp.ast.sons[resultPos].sym = copySym(s.ast.sons[resultPos].sym)
+    else:
+      # We've encountered a method prototype without a filled-in
+      # resultPos slot. We put a placeholder in there that will
+      # be updated in fixupDispatcher().
+      disp.ast.addSon(ast.emptyNode)
+  attachDispatcher(s, newSymNode(disp))
+  # attach to itself to prevent bugs:
+  attachDispatcher(disp, newSymNode(disp))
+  return disp
+
+proc fixupDispatcher(meth, disp: PSym) =
+  # We may have constructed the dispatcher from a method prototype
+  # and need to augment the incomplete dispatcher with information
+  # from later definitions, particularly the resultPos slot. Also,
+  # the lock level of the dispatcher needs to be updated/checked
+  # against that of the method.
+  if disp.ast.sonsLen > resultPos and meth.ast.sonsLen > resultPos and
+     disp.ast.sons[resultPos] == ast.emptyNode:
+    disp.ast.sons[resultPos] = copyTree(meth.ast.sons[resultPos])
+
+  # The following code works only with lock levels, so we disable
+  # it when they're not available.
+  when declared(TLockLevel):
+    proc `<`(a, b: TLockLevel): bool {.borrow.}
+    proc `==`(a, b: TLockLevel): bool {.borrow.}
+    if disp.typ.lockLevel == UnspecifiedLockLevel:
+      disp.typ.lockLevel = meth.typ.lockLevel
+    elif meth.typ.lockLevel != UnspecifiedLockLevel and
+         meth.typ.lockLevel != disp.typ.lockLevel:
+      message(meth.info, warnLockLevel,
+        "method has lock level $1, but another method has $2" %
+        [$meth.typ.lockLevel, $disp.typ.lockLevel])
+      # XXX The following code silences a duplicate warning in
+      # checkMethodeffects() in sempass2.nim for now.
+      if disp.typ.lockLevel < meth.typ.lockLevel:
+        disp.typ.lockLevel = meth.typ.lockLevel
+
 proc methodDef*(s: PSym, fromCache: bool) =
   var L = len(gMethods)
   for i in countup(0, L - 1):
-    let disp = gMethods[i][0]
+    var disp = gMethods[i].dispatcher
     if sameMethodBucket(disp, s):
-      add(gMethods[i], s)
+      add(gMethods[i].methods, s)
       attachDispatcher(s, lastSon(disp.ast))
+      fixupDispatcher(s, disp)
       when useEffectSystem: checkMethodEffects(disp, s)
       return 
-  add(gMethods, @[s])
   # create a new dispatcher:
-  if not fromCache:
-    var disp = copySym(s)
-    incl(disp.flags, sfDispatcher)
-    excl(disp.flags, sfExported)
-    disp.typ = copyType(disp.typ, disp.typ.owner, false)
-    # we can't inline the dispatcher itself (for now):
-    if disp.typ.callConv == ccInline: disp.typ.callConv = ccDefault
-    disp.ast = copyTree(s.ast)
-    disp.ast.sons[bodyPos] = ast.emptyNode
-    disp.loc.r = nil
-    if s.typ.sons[0] != nil: 
-      disp.ast.sons[resultPos].sym = copySym(s.ast.sons[resultPos].sym)
-    attachDispatcher(s, newSymNode(disp))
-    # attach to itself to prevent bugs:
-    attachDispatcher(disp, newSymNode(disp))
+  add(gMethods, (methods: @[s], dispatcher: createDispatcher(s)))
+  if fromCache:
+    internalError(s.info, "no method dispatcher found")
 
 proc relevantCol(methods: TSymSeq, col: int): bool =
   # returns true iff the position is relevant
@@ -195,8 +234,9 @@ proc generateMethodDispatchers*(): PNode =
   result = newNode(nkStmtList)
   for bucket in countup(0, len(gMethods) - 1): 
     var relevantCols = initIntSet()
-    for col in countup(1, sonsLen(gMethods[bucket][0].typ) - 1): 
-      if relevantCol(gMethods[bucket], col): incl(relevantCols, col)
-    sortBucket(gMethods[bucket], relevantCols)
-    addSon(result, newSymNode(genDispatcher(gMethods[bucket], relevantCols)))
+    for col in countup(1, sonsLen(gMethods[bucket].methods[0].typ) - 1): 
+      if relevantCol(gMethods[bucket].methods, col): incl(relevantCols, col)
+    sortBucket(gMethods[bucket].methods, relevantCols)
+    addSon(result,
+           newSymNode(genDispatcher(gMethods[bucket].methods, relevantCols)))
 
diff --git a/compiler/transf.nim b/compiler/transf.nim
index 20fd5620f..1ec31d605 100644
--- a/compiler/transf.nim
+++ b/compiler/transf.nim
@@ -625,10 +625,9 @@ proc transformCall(c: PTransf, n: PNode): PTransNode =
     # use the dispatcher for the call:
     if s.sons[0].kind == nkSym and s.sons[0].sym.kind == skMethod:
       let t = lastSon(s.sons[0].sym.ast)
-      if t.kind == nkSym and sfDispatcher in t.sym.flags:
-        result = methodCall(s).PTransNode
-      else:
-        result = s.PTransNode
+      if t.kind != nkSym or sfDispatcher notin t.sym.flags:
+        methodDef(s.sons[0].sym, false)
+      result = methodCall(s).PTransNode
     else:
       result = s.PTransNode
 
diff --git a/tests/method/tmproto.nim b/tests/method/tmproto.nim
new file mode 100644
index 000000000..5d75cff1a
--- /dev/null
+++ b/tests/method/tmproto.nim
@@ -0,0 +1,25 @@
+type
+  Obj1 = ref object {.inheritable.}
+  Obj2 = ref object of Obj1
+
+method beta(x: Obj1): int
+
+proc delta(x: Obj2): int =
+  beta(x)
+
+method beta(x: Obj2): int
+
+proc alpha(x: Obj1): int =
+  beta(x)
+
+method beta(x: Obj1): int = 1
+method beta(x: Obj2): int = 2
+
+proc gamma(x: Obj1): int =
+  beta(x)
+
+doAssert alpha(Obj1()) == 1
+doAssert gamma(Obj1()) == 1
+doAssert alpha(Obj2()) == 2
+doAssert gamma(Obj2()) == 2
+doAssert delta(Obj2()) == 2