Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong number of deallocations when using destructors #9263

Closed
b3liever opened this issue Oct 9, 2018 · 8 comments

Comments

Projects
None yet
4 participants
@b3liever
Copy link
Contributor

commented Oct 9, 2018

type
   Matrix* = object
      # Array for internal storage of elements.
      data: ptr UncheckedArray[float]
      # Row and column dimensions.
      m*, n*: int

var
   allocCount, deallocCount: int

proc `=destroy`*(m: var Matrix) =
   if m.data != nil:
      dealloc(m.data)
      deallocCount.inc
      m.data = nil
      m.m = 0
      m.n = 0

proc `=sink`*(a: var Matrix; b: Matrix) =
   echo "=sink called"
   writeStackTrace()
   if a.data != nil and a.data != b.data:
      dealloc(a.data)
      deallocCount.inc
   a.data = b.data
   a.m = b.m
   a.n = b.n

proc `=`*(a: var Matrix; b: Matrix) =
   echo "= called"
   writeStackTrace()
   if a.data != nil and a.data != b.data:
      dealloc(a.data)
      deallocCount.inc
      a.data = nil
   a.m = b.m
   a.n = b.n
   if b.data != nil:
      a.data = cast[type(a.data)](alloc(a.m * a.n * sizeof(float)))
      allocCount.inc
      copyMem(a.data, b.data, b.m * b.n * sizeof(float))

proc matrix*(m, n: int, s: float): Matrix =
   ## Construct an m-by-n constant matrix.
   result.m = m
   result.n = n
   result.data = cast[type(result.data)](alloc(m * n * sizeof(float)))
   allocCount.inc
   for i in 0 ..< m * n:
      result.data[i] = s

proc `[]`*(m: Matrix, i, j: int): float {.inline.} =
   ## Get a single element.
   m.data[i * m.n + j]

proc `[]`*(m: var Matrix, i, j: int): var float {.inline.} =
   ## Get a single element.
   m.data[i * m.n + j]

proc `[]=`*(m: var Matrix, i, j: int, s: float) =
   ## Set a single element.
   m.data[i * m.n + j] = s

proc `-`*(m: sink Matrix): Matrix =
   ## Unary minus
   result = m
   for i in 0 ..< m.m:
      for j in 0 ..< m.n:
         result[i, j] = -m[i, j]

proc `+`*(a: sink Matrix; b: Matrix): Matrix =
   ## ``C = A + B``
   assert(b.m == a.m and b.n == a.n, "Matrix dimensions must agree.")
   result = a
   for i in 0 ..< a.m:
      for j in 0 ..< a.n:
         result[i, j] = a[i, j] + b[i, j]

proc `-`*(a: sink Matrix; b: Matrix): Matrix =
   ## ``C = A - B``
   assert(b.m == a.m and b.n == a.n, "Matrix dimensions must agree.")
   result = a
   for i in 0 ..< a.m:
      for j in 0 ..< a.n:
         result[i, j] = a[i, j] - b[i, j]

proc info =
   echo "after ", allocCount, " ", deallocCount
   allocCount = 0
   deallocCount = 0

proc test1 =
   var a = matrix(5, 5, 1.0)
   var b = a
   var c = a + b

proc test2 =
   var a = matrix(5, 5, 1.0)
   var b = a
   var c = -a

proc test3 =
   var a = matrix(5, 5, 1.0)
   var b = matrix(5, 5, 2.0)
#    a = a - b
   b = -b + a

test1()
info()

test2()
info()

test3()
info()

Output

2 3
2 3
3 4
@b3liever

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2018

Maybe I am using them wrong, but still it shouldn't produce wrong code.

@Araq

This comment has been minimized.

Copy link
Member

commented Oct 9, 2018

Yup, excellent test, looking into it.

@timotheecour

This comment has been minimized.

Copy link
Contributor

commented Oct 10, 2018

@b3liever any chance you could minimize your test case as much as possible?

@Araq

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

@timotheecour I like it this way and I'm working on a fix already.

@Araq Araq closed this in 0803b53 Oct 10, 2018

@b3liever

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2018

Awesome thank you

@b3liever

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2018

@Araq, I don't know if it is another bug or expected behaviour but:

proc main =
   var a = matrix(5, 5, 1.0)
   a = -a + a
   echo a[1, 1]

gives 1 1 and -2.0
while:

proc main =
   var a = matrix(5, 5, 1.0)
#    a = a + a
    a = -a

Either of the two produce 2 2. Both look wrong because:

proc main =
   var a = matrix(5, 5, 1.0)
   var b  = -a

Only does 1 1

@Araq

This comment has been minimized.

Copy link
Member

commented Oct 11, 2018

Well, tbh I'm still working on this feature...

@timotheecour

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2018

I don't know if it is another bug or expected behaviour but:

=> seems fixed via closed #9294

krux02 added a commit to krux02/Nim that referenced this issue Oct 15, 2018

narimiran added a commit to narimiran/Nim that referenced this issue Oct 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.