codex-storage / questionable

Elegant optional types for Nim
Other
116 stars 5 forks source link

Binding error - Attempt to read from nil #53

Closed tbekas closed 8 months ago

tbekas commented 8 months ago

Tested on nim 1.6.14

Happens within procs that have a type param and that are called by one of imports within without macro

main.nim

import ./somefn
import ./otherfn

discard int.someFn()

somefn.nim

import pkg/questionable
import pkg/questionable/results

proc someFn*(
  X: type int
): ?!int =
  let res = success(1)

  if err =? res.errorOption:
    echo "err" & err.msg
  else:
    echo "no err"

  failure("")

otherfn.nim

import pkg/questionable
import pkg/questionable/results

import ./somefn

proc otherFn*(): void =
  without x =? int.someFn(), err:
    echo "was err" & err.msg

If we run nim c -r main.nim we get:

Traceback (most recent call last)
/Users/tbekas/Workspace/nim-codex/main.nim(8) main
/Users/tbekas/Workspace/nim-codex/vendor/questionable/questionable/private/binderror.nim(24) someFn
/Users/tbekas/Workspace/nim-codex/vendor/nimbus-build-system/vendor/Nim/lib/system/gc.nim(302) unsureAsgnRef
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Error: execution of an external program failed: '/Users/tbekas/Workspace/nim-codex/main '
markspanbroek commented 8 months ago

Thanks @tbekas, the reproduction code is really helpful. I was able to make it a bit smaller still:

proc foo(_: type): ?!int =
  if error =? success(1).errorOption:
    discard

proc bar = # defined, but not used
  without x =? bool.foo(), error:
    return

discard bool.foo() # same type parameter 'bool' as used in bar()

It's an interesting problem, basically the hack that I put in binderror.nim to check whether we need to capture an error doesn't work here. The generic proc foo is first instantiated in the compiler when compiling bar, and here we do need to capture the error. But the same instantiation of the generic proc foo is then used in a context where we don't need to capture the error.

tbekas commented 8 months ago

Thanks for checking, confirming it and making it yet smaller :)