nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.43k stars 1.47k forks source link

Runtime error when "kind type" loop variable is captured in a closure #19434

Open johnnovak opened 2 years ago

johnnovak commented 2 years ago

What the title says... Probably easier to understand the problem by reading this example carefully, as it seems to be quite specific:

Example

type
  ItemKind = enum
    ikOne, ikTwo

  Item = object
    case kind: ItemKind
    of ikOne:  one: int
    of ikTwo:  two: bool

let items = @[
  Item(kind: ikOne, one: 1),
  Item(kind: ikTwo, two: false)
]

var deferred: proc ()

# ---------------------------
# TEST 1 - PASS
deferred = proc () =
  echo items[0].one, items[1].two

deferred()  # OK: prints "1false"

# ---------------------------
# TEST 2 - PASS
for i in items:
  case i.kind
  of ikOne:
    let ii = i  # passing an explicit copy to the closure works
    deferred = proc () =
      echo ii.one:
  else: discard

deferred()  # OK: prints "1"

# ---------------------------
# TEST 3 - FAIL
for i in items:
  case i.kind
  of ikOne:
    deferred = proc () =
      echo i.one:  # passing in the loop variable directly results in a runtime error when trying to use the variable in the closure!

deferred()  # Error: unhandled exception: field 'one' is not accessible for type 'Item' using 'kind = 1' [FieldDefect]

Current Output

1false
1
D:\Work\Code\test\main.nim(45) main
D:\Work\Code\test\main.nim(42) :anonymous
C:\dev\nim-1.6.2-x64\lib\system\fatal.nim(53) sysFatal
Error: unhandled exception: field 'one' is not accessible for type 'Item' using 'kind = ikTwo' [FieldDefect]
Error: execution of an external program failed: 'D:\Work\Code\test\main.exe '

Expected Output

1false
1
1

Additional Information

Nim Compiler Version 1.6.2 [Windows: amd64]
Compiled at 2021-12-17
Copyright (c) 2006-2021 by Andreas Rumpf

active boot switches: -d:release

Tried with all memory management options, except for go, and I got the same results every time.

Also tried with 1.6.0 -- same results.

Tried a few variations (var instead of let, using mitem, using local vars instead of globals), but the error always happened. In any case, this is a valid scenario and it should work.

metagn commented 2 years ago
var deferred: proc()

for i in 1..3:
  if i == 1:
    deferred = proc() = echo i

deferred() # 3

sugar.capture seems to be created for this.

johnnovak commented 2 years ago

Okay, so closures capture variables by reference... Well, that's news to me. I always thought they capture the current values! (so they make a copy)

Any captured variables are stored in a hidden additional argument to the closure (its environment) and they are accessed by reference by both the closure and its enclosing scope (i.e. any modifications made to them are visible in both places).

But in any case, shouldn't my example then just use the value of the last iteration? It still seems to be a bug, why does it crash?

metagn commented 2 years ago

The value of the last iteration is Item(kind: ikTwo, two: false). The closure tries to obtain the one field from this object, but this field is not accessible since its kind is not ikOne.