Strange Table memory error

[Stuffe]

I encountered a strange error where the inner scope reference to a table becomes all zeros under very specific conditions.

I am not sure I can explain it better than the POC below:

import tables

var table* = initTable[int, int]()

# Must be this sequence...
for id in @[1, 2, 3, 4, 5, 6, 7, 8, 100, 101, 102, 200, 201, 202, 203, 204, 300, 301, 302, 303, 304, 305, 306, 308, 309, 310, 311, 312, 313, 314, 315, 316]:
  table[id] = id

proc inner() =
  for i in 0..11: # Must be larger than 10
    table[-i] = i # Doesn't work without negating i

proc outer*(table2: Table[int, int]) =
  inner()
  assert table2.contains(1) # All values are zeroed in inner scope...

assert table.contains(1) # Works
outer(table)

https://play.nim-lang.org/#ix=2c2F

[disruptek]

When you grow your table beyond its allocated size, it reallocates the seq used for storage. Use a TableRef, for starters, or provide a large enough initial size when creating the table.

[disruptek]

It may help to play with this shortened version of the original POC:

import tables

const
  n = 4
var table* = initTable[int, int](n)

for id in 1 .. n:
  table[id] = id

proc inner() =
  for id in n+1 .. n+n:
    table[-id] = -id

proc outer*(table2: Table[int, int]) =
  inner()
  for k, v in table2.pairs:
    echo k, "=", v
outer(table)

[Stuffe]

Use a TableRef, for starters, or provide a large enough initial size when creating the table.

Thank you for the work around. You are not implying that this is expected behavior, right?

[timotheecour]

probably related (or caused) by #13431

You are not implying that this is expected behavior, right?

I don't think this should be expected behavior

note

an alternative to using a TableRef is to pass by var instead, eg:

proc outer*(table2: var Table[int, int]) =

obviously, with different semantics

[disruptek]

It's expected behavior because you are passing a Table (and not a TableRef); ie. pass by value. So table2 is a copy of the Table object, and its underlying seq gets clobbered.

Use a TableRef, or provide a large enough initial size that a realloc does not occur.

[Stuffe]

It's expected behavior because you are passing a Table (and not a TableRef); ie. pass by value

I would like to argue that this should not be expected behavior. If this was really expected behavior, you are asking the user to understand the implementation details of Table. I feel like that is not reasonable, the whole point of programming languages in my opinion is the powerful abstractions they give us so we don't have to reason on those lower levels. And this breaks 2 of those abstractions that Nim implicitly promises. Specifically I expected that:

• Passing by value (immutably), the argument would never change in the scope of the function
• A table is a collection of key value pairs, each key and value do not change, except if reassigned or deleted

[andreaferretti]

It's expected behavior because you are passing a Table (and not a TableRef); ie. pass by value. So table2 is a copy of the Table object, and its underlying seq gets clobbered.

This is where it gets wrong, I think. Now I have to admit that I do not know the meaning of "clobbered", but what I would expect is:

It's expected behavior because you are passing a Table (and not a TableRef); ie. pass by value. So table2 is a copy of the Table object, and its underlying seq gets copied.

Hence, modifying table has no effect whatsoever on table2.

That said, most programming languages do not have the equivalent of Table at all, but only what we call TableRef. For what it's worth, I think we could unexport Table and related procs and only provide TableRef in the stdlib (possibly with a deprecation period). A Table uses the heap anyway (it has a backing seq) so I do not understand at all why one would ever want the strange semantics of Table

[timotheecour]

I agree, IMO there's 0 performance benefit to having Table as an object; the bulk of the data is on the heap so allocating TableRef on the heap shouldn't incur any meaningful overhead.

All that's needed is a way to deep-copy a TableRef and then Table has no purpose.

[Araq]

https://nim-lang.org/docs/manual_experimental.html#aliasing-restrictions-in-parameter-passing

For now, no bug here, aliasing is very hard and your code is bad.

[Araq]

All that's needed is a way to deep-copy a TableRef and then Table has no purpose.

No.

1. you can use Table in a const section and you can't for TableRef.
2. ref has unclear (multiple) ownership, var Table hasn't. This is more and more important for --gc:arc etc etc.

[Araq]

Alternatively this is not a "won't fix" but it's waiting for a compiler that enforces the aliasing rules the manual mentions. Then the compiler will reject your code (as it should).

[timotheecour]

1. you can use Table in a const section and you can't for TableRef.

you can with #13082

import tables
const t = {1:"foo", 2:"bar"}.newTable
doAssert $t == """{1: "foo", 2: "bar"}"""
doAssert t is TableRef
doAssert t[1] == "foo"
static: doAssert t[1] == "foo"

I think we should re-open #13082 btw, see #13082 (comment); it removes an artificial limitation that makes more code work same at CT vs RT; I still haven't seen a fundamental blocker for this.

[Araq]

I still haven't seen a fundamental blocker for this.

It's fundamentally broken.

[timotheecour]

if it's fundamentally broken, would you mind giving me a concrete example where the PR breaks / doesn't do what one would expect ? (again, not related to case objects which is a separate issue #13081) It only takes 20 seconds to build nim from #13082 with this 1 liner:

git fetch origin pull/13082/head && git checkout FETCH_HEAD && nim c -d:release -o:bin/nim.temp1 compiler/nim.nim

the example you had previously given me didn't do any crash for me see, #13082 (comment)

[Araq]

It allows you to mix ref objects which have an header containing the refcount etc with "const" ref objects that lack these.