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

Using var return types will result in segfaults in some cases #5113

Closed
johnnovak opened this issue Dec 10, 2016 · 1 comment
Closed

Using var return types will result in segfaults in some cases #5113

johnnovak opened this issue Dec 10, 2016 · 1 comment

Comments

@johnnovak
Copy link
Contributor

Okay, I'm not even sure that using var in conjunction with return types does make any sense at all. The thing is, the compiler allows it in some circumstances, but then we'll get segfaults at runtime.

Short version:

proc makeSeqVar(size: Natural): var seq[int] =
  newSeq(result, size)

# This will result in a segfault
discard makeSeqVar(1000)

# var.nim(4)               var
# var.nim(2)               makeSeqVar
# system.nim(1758)         unsureAsgnRef
# SIGSEGV: Illegal storage access. (Attempt to read from nil?)

However, the compiler seems to be doing a good job of preventing this from happening for simple types:

proc stuff(): var string =
  result = "mayhem"

# var.nim(2, 12) Error: expression has no address
discard stuff()

How I ended up with this construct in the first place:

proc doStuff(s: var seq[int]) =
  for i in 0..s.high:
    s[i] = 42

proc makeSeq(size: Natural): seq[int] =
  newSeq(result, size)

# Case #1 - this is all good so far
var s1 = makeSeq(1000)
doStuff(s1)

# Case #2 - generates compiler error, which is CORRECT
# doStuff(makeSeq(1000))

# var.nim(11, 8) Error: type mismatch: got (seq[int])
# but expected one of:
# proc doStuff(s: var seq[int])
# for a 'var' type a variable needs to be passed, but 'makeSeq(1000)' is immutable

# Case #3 - let's try changing the return type to var seq[int]
proc makeSeqVar(size: Natural): var seq[int] =
  newSeq(result, size)

# This will compile fine now...
doStuff(makeSeqVar(1000))

# ...but not so fast, it will FAIL spectacularly at runtime:
#
# var.nim(25)              var
# var.nim(23)              makeSeqVar
# system.nim(1758)         unsureAsgnRef
# SIGSEGV: Illegal storage access. (Attempt to read from nil?)

# And it turns out that just doing this triggers the failure:
discard makeSeqVar(1000)
@branpk
Copy link
Contributor

branpk commented Jan 8, 2017

It does make sense and is safe in some cases, e.g.

type A = object
  x: int

proc v(a: var A): var int = a.x

var a: A
inc(a.v)

Since newSeq returns a seq and not a var seq, it seems like the compiler should know to fail in that case.

@Araq Araq closed this as completed in b43025b Apr 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants