Merge pull request #1609 from rbehrends/fix-method-dispatch

Fix method recursion bug.

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,31 +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:
+  add(gMethods, (methods: @[s], dispatcher: createDispatcher(s)))
-    var disp = copySym(s)
+  if fromCache:
-    incl(disp.flags, sfDispatcher)
+    internalError(s.info, "no method dispatcher found")
-    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
-    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))
 
 proc relevantCol(methods: TSymSeq, col: int): bool =
   # returns true iff the position is relevant
@@ -194,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): 
+    for col in countup(1, sonsLen(gMethods[bucket].methods[0].typ) - 1): 
-      if relevantCol(gMethods[bucket], col): incl(relevantCols, col)
+      if relevantCol(gMethods[bucket].methods, col): incl(relevantCols, col)
-    sortBucket(gMethods[bucket], relevantCols)
+    sortBucket(gMethods[bucket].methods, relevantCols)
-    addSon(result, newSymNode(genDispatcher(gMethods[bucket], relevantCols)))
+    addSon(result,
+           newSymNode(genDispatcher(gMethods[bucket].methods, relevantCols)))
 

compiler/transf.nim

@@ -624,6 +624,9 @@ proc transformCall(c: PTransf, n: PNode): PTransNode =
     # bugfix: check after 'transformSons' if it's still a method call:
     # 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 or sfDispatcher notin t.sym.flags:
+        methodDef(s.sons[0].sym, false)
       result = methodCall(s).PTransNode
     else:
       result = s.PTransNode

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

tests/method/trecmeth.nim

@@ -0,0 +1,22 @@
+# Note: We only compile this to verify that code generation
+# for recursive methods works, no code is being executed
+
+type
+  Obj = ref object of TObject
+
+# Mutual recursion
+
+method alpha(x: Obj)
+method beta(x: Obj)
+
+method alpha(x: Obj) =
+  beta(x)
+
+method beta(x: Obj) =
+  alpha(x)
+
+# Simple recursion
+
+method gamma(x: Obj) =
+  gamma(x)
+