more progress on destructors; removed old destructor based code as it proved confusing

2026-02-26 04:45:08 +00:00 · 2017-12-01 01:52:00 +01:00
parent 255902f9a5
commit fa92c519aa
9 changed files with 152 additions and 270 deletions
--- a/compiler/destroyer.nim
+++ b/compiler/destroyer.nim
@@ -167,10 +167,13 @@ template interestingSym(s: PSym): bool =
 proc patchHead(n: PNode) =
  if n.kind in nkCallKinds and n[0].kind == nkSym and n.len > 1:
    let s = n[0].sym
-    if sfFromGeneric in s.flags and s.name.s[0] == '=' and
-        s.name.s in ["=sink", "=", "=destroy"]:
-      excl(s.flags, sfFromGeneric)
-      patchHead(s.getBody)
+    if s.name.s[0] == '=' and s.name.s in ["=sink", "=", "=destroy"]:
+      if sfFromGeneric in s.flags:
+        excl(s.flags, sfFromGeneric)
+        patchHead(s.getBody)
+      if n[1].typ.isNil:
+        # XXX toptree crashes without this workaround. Figure out why.
+        return
      let t = n[1].typ.skipTypes({tyVar, tyGenericInst, tyAlias, tyInferred})
      template patch(op, field) =
        if s.name.s == op and field != nil and field != s:
@@ -181,24 +184,30 @@ proc patchHead(n: PNode) =
  for x in n:
    patchHead(x)

+proc patchHead(s: PSym) =
+  if sfFromGeneric in s.flags:
+    patchHead(s.ast[bodyPos])
+
+template genOp(opr, opname) =
+  let op = opr
+  if op == nil:
+    globalError(dest.info, "internal error: '" & opname & "' operator not found for type " & typeToString(t))
+  elif op.ast[genericParamsPos].kind != nkEmpty:
+    globalError(dest.info, "internal error: '" & opname & "' operator is generic")
+  patchHead op
+  result = newTree(nkCall, newSymNode(op), newTree(nkHiddenAddr, dest))
+
 proc genSink(t: PType; dest: PNode): PNode =
  let t = t.skipTypes({tyGenericInst, tyAlias})
-  let op = if t.sink != nil: t.sink else: t.assignment
-  assert op != nil
-  patchHead op.ast[bodyPos]
-  result = newTree(nkCall, newSymNode(op), newTree(nkHiddenAddr, dest))
+  genOp(if t.sink != nil: t.sink else: t.assignment, "=sink")

 proc genCopy(t: PType; dest: PNode): PNode =
  let t = t.skipTypes({tyGenericInst, tyAlias})
-  assert t.assignment != nil
-  patchHead t.assignment.ast[bodyPos]
-  result = newTree(nkCall, newSymNode(t.assignment), newTree(nkHiddenAddr, dest))
+  genOp(t.assignment, "=")

 proc genDestroy(t: PType; dest: PNode): PNode =
  let t = t.skipTypes({tyGenericInst, tyAlias})
-  assert t.destructor != nil
-  patchHead t.destructor.ast[bodyPos]
-  result = newTree(nkCall, newSymNode(t.destructor), newTree(nkHiddenAddr, dest))
+  genOp(t.destructor, "=destroy")

 proc addTopVar(c: var Con; v: PNode) =
  c.topLevelVars.add newTree(nkIdentDefs, v, emptyNode, emptyNode)
@@ -287,6 +296,7 @@ proc p(n: PNode; c: var Con): PNode =
    recurse(n, result)

 proc injectDestructorCalls*(owner: PSym; n: PNode): PNode =
+  echo "injecting into ", n
  var c: Con
  c.owner = owner
  c.tmp = newSym(skTemp, getIdent":d", owner, n.info)
--- a/compiler/semasgn.nim
+++ b/compiler/semasgn.nim
@@ -7,8 +7,8 @@
 #    distribution, for details about the copyright.
 #

-## This module implements lifting for assignments. Later versions of this code
-## will be able to also lift ``=deepCopy`` and ``=destroy``.
+## This module implements lifting for type-bound operations
+## (``=sink``, ``=``, ``=destroy``, ``=deepCopy``).

 # included from sem.nim

@@ -302,6 +302,7 @@ proc liftBody(c: PContext; typ: PType; kind: TTypeAttachedOp;
  n.sons[paramsPos] = result.typ.n
  n.sons[bodyPos] = body
  result.ast = n
+  incl result.flags, sfFromGeneric


 proc getAsgnOrLiftBody(c: PContext; typ: PType; info: TLineInfo): PSym =
@@ -319,8 +320,10 @@ proc liftTypeBoundOps*(c: PContext; typ: PType; info: TLineInfo) =
  ## to ensure we lift assignment, destructors and moves properly.
  ## The later 'destroyer' pass depends on it.
  if not newDestructors or not hasDestructor(typ): return
-  # do not produce wrong liftings while we're still instantiating generics:
-  if c.typesWithOps.len > 0: return
+  when false:
+    # do not produce wrong liftings while we're still instantiating generics:
+    # now disabled; breaks topttree.nim!
+    if c.typesWithOps.len > 0: return
  let typ = typ.skipTypes({tyGenericInst, tyAlias})
  # we generate the destructor first so that other operators can depend on it:
  if typ.destructor == nil:
@@ -329,3 +332,6 @@ proc liftTypeBoundOps*(c: PContext; typ: PType; info: TLineInfo) =
    liftBody(c, typ, attachedAsgn, info)
  if typ.sink == nil:
    liftBody(c, typ, attachedSink, info)
+
+#proc patchResolvedTypeBoundOp*(c: PContext; n: PNode): PNode =
+#  if n.kind == nkCall and
--- a/compiler/semdestruct.nim
+++ b/compiler/semdestruct.nim
@@ -1,185 +0,0 @@
-#
-#
-#           The Nim Compiler
-#        (c) Copyright 2013 Andreas Rumpf
-#
-#    See the file "copying.txt", included in this
-#    distribution, for details about the copyright.
-#
-
-## This module implements destructors.
-
-# included from sem.nim
-
-# special marker values that indicates that we are
-# 1) AnalyzingDestructor: currently analyzing the type for destructor
-# generation (needed for recursive types)
-# 2) DestructorIsTrivial: completed the analysis before and determined
-# that the type has a trivial destructor
-var analyzingDestructor, destructorIsTrivial: PSym
-new(analyzingDestructor)
-new(destructorIsTrivial)
-
-var
-  destructorName = getIdent"destroy_"
-  destructorParam = getIdent"this_"
-
-proc instantiateDestructor(c: PContext, typ: PType): PType
-
-proc doDestructorStuff(c: PContext, s: PSym, n: PNode) =
-  var t = s.typ.sons[1].skipTypes({tyVar})
-  if t.kind == tyGenericInvocation:
-    for i in 1 ..< t.sonsLen:
-      if t.sons[i].kind != tyGenericParam:
-        localError(n.info, errDestructorNotGenericEnough)
-        return
-    t = t.base
-  elif t.kind == tyCompositeTypeClass:
-    t = t.base
-    if t.kind != tyGenericBody:
-      localError(n.info, errDestructorNotGenericEnough)
-      return
-
-  t.destructor = s
-  # automatically insert calls to base classes' destructors
-  if n.sons[bodyPos].kind != nkEmpty:
-    for i in countup(0, t.sonsLen - 1):
-      # when inheriting directly from object
-      # there will be a single nil son
-      if t.sons[i] == nil: continue
-      let destructableT = instantiateDestructor(c, t.sons[i])
-      if destructableT != nil:
-        n.sons[bodyPos].addSon(newNode(nkCall, t.sym.info, @[
-            useSym(destructableT.destructor, c.graph.usageSym),
-            n.sons[paramsPos][1][0]]))
-
-proc destroyFieldOrFields(c: PContext, field: PNode, holder: PNode): PNode
-
-proc destroySym(c: PContext, field: PSym, holder: PNode): PNode =
-  let destructableT = instantiateDestructor(c, field.typ)
-  if destructableT != nil:
-    result = newNode(nkCall, field.info, @[
-      useSym(destructableT.destructor, c.graph.usageSym),
-      newNode(nkDotExpr, field.info, @[holder, useSym(field, c.graph.usageSym)])])
-
-proc destroyCase(c: PContext, n: PNode, holder: PNode): PNode =
-  var nonTrivialFields = 0
-  result = newNode(nkCaseStmt, n.info, @[])
-  # case x.kind
-  result.addSon(newNode(nkDotExpr, n.info, @[holder, n.sons[0]]))
-  for i in countup(1, n.len - 1):
-    # of A, B:
-    let ni = n[i]
-    var caseBranch = newNode(ni.kind, ni.info, ni.sons[0..ni.len-2])
-
-    let stmt = destroyFieldOrFields(c, ni.lastSon, holder)
-    if stmt == nil:
-      caseBranch.addSon(newNode(nkStmtList, ni.info, @[]))
-    else:
-      caseBranch.addSon(stmt)
-      nonTrivialFields += stmt.len
-
-    result.addSon(caseBranch)
-
-  # maybe no fields were destroyed?
-  if nonTrivialFields == 0:
-    result = nil
-
-proc destroyFieldOrFields(c: PContext, field: PNode, holder: PNode): PNode =
-  template maybeAddLine(e) =
-    let stmt = e
-    if stmt != nil:
-      if result == nil: result = newNode(nkStmtList)
-      result.addSon(stmt)
-
-  case field.kind
-  of nkRecCase:
-    maybeAddLine destroyCase(c, field, holder)
-  of nkSym:
-    maybeAddLine destroySym(c, field.sym, holder)
-  of nkRecList:
-    for son in field:
-      maybeAddLine destroyFieldOrFields(c, son, holder)
-  else:
-    internalAssert false
-
-proc generateDestructor(c: PContext, t: PType): PNode =
-  ## generate a destructor for a user-defined object or tuple type
-  ## returns nil if the destructor turns out to be trivial
-
-  # XXX: This may be true for some C-imported types such as
-  # Tposix_spawnattr
-  if t.n == nil or t.n.sons == nil: return
-  internalAssert t.n.kind == nkRecList
-  let destructedObj = newIdentNode(destructorParam, unknownLineInfo())
-  # call the destructods of all fields
-  result = destroyFieldOrFields(c, t.n, destructedObj)
-  # base classes' destructors will be automatically called by
-  # semProcAux for both auto-generated and user-defined destructors
-
-proc instantiateDestructor(c: PContext, typ: PType): PType =
-  # returns nil if a variable of type `typ` doesn't require a
-  # destructor. Otherwise, returns the type, which holds the
-  # destructor that must be used for the varialbe.
-  # The destructor is either user-defined or automatically
-  # generated by the compiler in a member-wise fashion.
-  var t = typ.skipGenericAlias
-  let typeHoldingUserDefinition = if t.kind == tyGenericInst: t.base else: t
-
-  if typeHoldingUserDefinition.destructor != nil:
-    # XXX: This is not entirely correct for recursive types, but we need
-    # it temporarily to hide the "destroy is already defined" problem
-    if typeHoldingUserDefinition.destructor notin
-        [analyzingDestructor, destructorIsTrivial]:
-      return typeHoldingUserDefinition
-    else:
-      return nil
-
-  t = t.skipTypes({tyGenericInst, tyAlias})
-  case t.kind
-  of tySequence, tyArray, tyOpenArray, tyVarargs:
-    t.destructor = analyzingDestructor
-    if instantiateDestructor(c, t.sons[0]) != nil:
-      t.destructor = getCompilerProc"nimDestroyRange"
-      return t
-    else:
-      return nil
-  of tyTuple, tyObject:
-    t.destructor = analyzingDestructor
-    let generated = generateDestructor(c, t)
-    if generated != nil:
-      internalAssert t.sym != nil
-      let info = t.sym.info
-      let fullDef = newNode(nkProcDef, info, @[
-        newIdentNode(destructorName, info),
-        emptyNode,
-        emptyNode,
-        newNode(nkFormalParams, info, @[
-          emptyNode,
-          newNode(nkIdentDefs, info, @[
-            newIdentNode(destructorParam, info),
-            symNodeFromType(c, makeVarType(c, t), t.sym.info),
-            emptyNode]),
-          ]),
-        emptyNode,
-        emptyNode,
-        generated
-        ])
-      let semantizedDef = semProc(c, fullDef)
-      t.destructor = semantizedDef[namePos].sym
-      return t
-    else:
-      t.destructor = destructorIsTrivial
-      return nil
-  else:
-    return nil
-
-proc createDestructorCall(c: PContext, s: PSym): PNode =
-  let varTyp = s.typ
-  if varTyp == nil or sfGlobal in s.flags: return
-  let destructableT = instantiateDestructor(c, varTyp)
-  if destructableT != nil:
-    let call = semStmt(c, newNode(nkCall, s.info, @[
-      useSym(destructableT.destructor, c.graph.usageSym),
-      useSym(s, c.graph.usageSym)]))
-    result = newNode(nkDefer, s.info, @[call])
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -53,7 +53,6 @@ proc semExprWithType(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
  else:
    if efNoProcvarCheck notin flags: semProcvarCheck(c, result)
    if result.typ.kind == tyVar: result = newDeref(result)
-    semDestructorCheck(c, result, flags)

 proc semExprNoDeref(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
  result = semExpr(c, n, flags)
@@ -66,7 +65,6 @@ proc semExprNoDeref(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
    result.typ = errorType(c)
  else:
    semProcvarCheck(c, result)
-    semDestructorCheck(c, result, flags)

 proc semSymGenericInstantiation(c: PContext, n: PNode, s: PSym): PNode =
  result = symChoice(c, n, s, scClosed)
@@ -671,6 +669,7 @@ proc afterCallActions(c: PContext; n, orig: PNode, flags: TExprFlags): PNode =
    if callee.magic != mNone:
      result = magicsAfterOverloadResolution(c, result, flags)
    if result.typ != nil: liftTypeBoundOps(c, result.typ, n.info)
+    #result = patchResolvedTypeBoundOp(c, result)
  if c.matchedConcept == nil:
    result = evalAtCompileTime(c, result)

--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -97,27 +97,12 @@ template semProcvarCheck(c: PContext, n: PNode) =

 proc semProc(c: PContext, n: PNode): PNode

-include semdestruct
-
-proc semDestructorCheck(c: PContext, n: PNode, flags: TExprFlags) {.inline.} =
-  if not newDestructors:
-    if efAllowDestructor notin flags and
-        n.kind in nkCallKinds+{nkObjConstr,nkBracket}:
-      if instantiateDestructor(c, n.typ) != nil:
-        localError(n.info, warnDestructor)
-    # This still breaks too many things:
-    when false:
-      if efDetermineType notin flags and n.typ.kind == tyTypeDesc and
-          c.p.owner.kind notin {skTemplate, skMacro}:
-        localError(n.info, errGenerated, "value expected, but got a type")
-
 proc semExprBranch(c: PContext, n: PNode): PNode =
  result = semExpr(c, n)
  if result.typ != nil:
    # XXX tyGenericInst here?
    semProcvarCheck(c, result)
    if result.typ.kind == tyVar: result = newDeref(result)
-    semDestructorCheck(c, result, {})

 proc semExprBranchScope(c: PContext, n: PNode): PNode =
  openScope(c)
@@ -421,15 +406,6 @@ proc addToVarSection(c: PContext; result: var PNode; orig, identDefs: PNode) =
  else:
    result.add identDefs

-proc addDefer(c: PContext; result: var PNode; s: PSym) =
-  let deferDestructorCall = createDestructorCall(c, s)
-  if deferDestructorCall != nil:
-    if result.kind != nkStmtList:
-      let oldResult = result
-      result = newNodeI(nkStmtList, result.info)
-      result.add oldResult
-    result.add deferDestructorCall
-
 proc isDiscardUnderscore(v: PSym): bool =
  if v.name.s == "_":
    v.flags.incl(sfGenSym)
@@ -609,7 +585,6 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
        if def.kind == nkPar: v.ast = def[j]
        setVarType(v, tup.sons[j])
        b.sons[j] = newSymNode(v)
-      if not newDestructors: addDefer(c, result, v)
      checkNilable(v)
      if sfCompileTime in v.flags: hasCompileTime = true
  if hasCompileTime: vm.setupCompileTimeVar(c.module, c.cache, result)
@@ -1041,6 +1016,8 @@ proc typeSectionFinalPass(c: PContext, n: PNode) =
      checkConstructedType(s.info, s.typ)
      if s.typ.kind in {tyObject, tyTuple} and not s.typ.n.isNil:
        checkForMetaFields(s.typ.n)
+  instAllTypeBoundOp(c, n.info)
+

 proc semAllTypeSections(c: PContext; n: PNode): PNode =
  proc gatherStmts(c: PContext; n: PNode; result: PNode) {.nimcall.} =
@@ -1095,9 +1072,11 @@ proc semTypeSection(c: PContext, n: PNode): PNode =
  ## to allow the type definitions in the section to reference each other
  ## without regard for the order of their definitions.
  if sfNoForward notin c.module.flags or nfSem notin n.flags:
+    inc c.inTypeContext
    typeSectionLeftSidePass(c, n)
    typeSectionRightSidePass(c, n)
    typeSectionFinalPass(c, n)
+    dec c.inTypeContext
  result = n

 proc semParamList(c: PContext, n, genericParams: PNode, s: PSym) =
@@ -1318,7 +1297,7 @@ proc semOverride(c: PContext, s: PSym, n: PNode) =
        var obj = t.sons[1].sons[0]
        while true:
          incl(obj.flags, tfHasAsgn)
-          if obj.kind == tyGenericBody: obj = obj.lastSon
+          if obj.kind in {tyGenericBody, tyGenericInst}: obj = obj.lastSon
          elif obj.kind == tyGenericInvocation: obj = obj.sons[0]
          else: break
        if obj.kind in {tyObject, tyDistinct}:
@@ -1331,10 +1310,6 @@ proc semOverride(c: PContext, s: PSym, n: PNode) =
      if not noError and sfSystemModule notin s.owner.flags:
        localError(n.info, errGenerated,
          "signature for '" & s.name.s & "' must be proc[T: object](x: var T)")
-    else:
-      doDestructorStuff(c, s, n)
-      if not experimentalMode(c):
-        localError n.info, "use the {.experimental.} pragma to enable destructors"
    incl(s.flags, sfUsed)
  of "deepcopy", "=deepcopy":
    if s.typ.len == 2 and
@@ -1561,8 +1536,11 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
  s.options = gOptions
  if sfOverriden in s.flags or s.name.s[0] == '=': semOverride(c, s, n)
  if s.name.s[0] in {'.', '('}:
-    if s.name.s in [".", ".()", ".=", "()"] and not experimentalMode(c):
+    if s.name.s in [".", ".()", ".="] and not experimentalMode(c) and not newDestructors:
      message(n.info, warnDeprecated, "overloaded '.' and '()' operators are now .experimental; " & s.name.s)
+    elif s.name.s == "()" and not experimentalMode(c):
+      message(n.info, warnDeprecated, "overloaded '()' operators are now .experimental; " & s.name.s)
+
  if n.sons[bodyPos].kind != nkEmpty:
    # for DLL generation it is annoying to check for sfImportc!
    if sfBorrow in s.flags:
--- a/tests/destructor/tatomicptrs.nim
+++ b/tests/destructor/tatomicptrs.nim
@@ -0,0 +1,101 @@
+discard """
+  output: '''allocating
+allocating
+allocating
+55
+60
+99
+deallocating
+deallocating
+deallocating
+'''
+  cmd: '''nim c --newruntime $file'''
+"""
+
+type
+  SharedPtr*[T] = object
+    x: ptr T
+
+#proc isNil[T](s: SharedPtr[T]): bool {.inline.} = s.x.isNil
+
+template incRef(x) =
+  atomicInc(x.refcount)
+
+template decRef(x): untyped = atomicDec(x.refcount)
+
+proc makeShared*[T](x: T): SharedPtr[T] =
+  # XXX could benefit from 'sink' parameter.
+  # XXX could benefit from a macro that generates it.
+  result = cast[SharedPtr[T]](allocShared(sizeof(x)))
+  result.x[] = x
+  echo "allocating"
+
+proc `=destroy`*[T](dest: var SharedPtr[T]) =
+  var s = dest.x
+  if s != nil and decRef(s) == 0:
+    `=destroy`(s[])
+    deallocShared(s)
+    echo "deallocating"
+    dest.x = nil
+
+proc `=`*[T](dest: var SharedPtr[T]; src: SharedPtr[T]) =
+  var s = src.x
+  if s != nil: incRef(s)
+  #atomicSwap(dest, s)
+  # XXX use an atomic store here:
+  swap(dest.x, s)
+  if s != nil and decRef(s) == 0:
+    `=destroy`(s[])
+    deallocShared(s)
+    echo "deallocating"
+
+proc `=sink`*[T](dest: var SharedPtr[T]; src: SharedPtr[T]) =
+  ## XXX make this an atomic store:
+  if dest.x != src.x:
+    let s = dest.x
+    if s != nil:
+      `=destroy`(s[])
+      deallocShared(s)
+      echo "deallocating"
+    dest.x = src.x
+
+template `.`*[T](s: SharedPtr[T]; field: untyped): untyped =
+  s.x.field
+
+template `.=`*[T](s: SharedPtr[T]; field, value: untyped) =
+  s.x.field = value
+
+from macros import unpackVarargs
+
+template `.()`*[T](s: SharedPtr[T]; field: untyped, args: varargs[untyped]): untyped =
+  unpackVarargs(s.x.field, args)
+
+
+type
+  Tree = SharedPtr[TreeObj]
+  TreeObj = object
+    refcount: int
+    le, ri: Tree
+    data: int
+
+proc takesTree(a: Tree) =
+  if not a.isNil:
+    takesTree(a.le)
+    echo a.data
+    takesTree(a.ri)
+
+proc createTree(data: int): Tree =
+  result = makeShared(TreeObj(refcount: 1, data: data))
+
+proc createTree(data: int; le, ri: Tree): Tree =
+  result = makeShared(TreeObj(refcount: 1, le: le, ri: ri, data: data))
+
+
+proc main =
+  let le = createTree(55)
+  let ri = createTree(99)
+  let t = createTree(60, le, ri)
+  takesTree(t)
+
+main()
+
--- a/tests/destructor/tdestructor.nim
+++ b/tests/destructor/tdestructor.nim
@@ -20,10 +20,10 @@ myobj destroyed
 ----
 myobj destroyed
 '''
+  cmd: '''nim c --newruntime $file'''
+  disabled: "true"
 """

-{.experimental.}
-
 type
  TMyObj = object
    x, y: int
@@ -61,7 +61,7 @@ proc `=destroy`(o: var TMyObj) =
  if o.p != nil: dealloc o.p
  echo "myobj destroyed"

-proc `=destroy`(o: var TMyGeneric1) =
+proc `=destroy`(o: var TMyGeneric1[int]) =
  echo "mygeneric1 destroyed"

 proc `=destroy`[A, B](o: var TMyGeneric2[A, B]) =
--- a/tests/destructor/tdestructor2.nim
+++ b/tests/destructor/tdestructor2.nim
@@ -1,27 +0,0 @@
-discard """
-  line: 23
-  nimout: " usage of a type with a destructor in a non destructible context"
-"""
-
-{.experimental.}
-
-type
-  TMyObj = object
-    x, y: int
-    p: pointer
-
-proc `=destroy`(o: var TMyObj) =
-  if o.p != nil: dealloc o.p
-
-proc open: TMyObj =
-  result = TMyObj(x: 1, y: 2, p: alloc(3))
-
-
-proc `$`(x: TMyObj): string = $x.y
-
-proc foo =
-  discard open()
-
-# XXX doesn't trigger this yet:
-#echo open()
-
--- a/tests/destructor/tdestructor3.nim
+++ b/tests/destructor/tdestructor3.nim
@@ -2,14 +2,14 @@ discard """
  output: '''assign
 destroy
 destroy
-destroy Foo: 5
 5
-destroy Foo: 123
-123'''
+123
+destroy Foo: 5
+destroy Foo: 123'''
+  cmd: '''nim c --newruntime $file'''
 """

 # bug #2821
-{.experimental.}

 type T = object