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

Arc sink arg crash #15238

Closed
cooldome opened this issue Aug 28, 2020 · 7 comments · Fixed by #15262
Closed

Arc sink arg crash #15238

cooldome opened this issue Aug 28, 2020 · 7 comments · Fixed by #15262

Comments

@cooldome
Copy link
Member

cooldome commented Aug 28, 2020

Compile with --gc:arc -d:danger

Example

proc sinkArg(x: sink seq[string]) =
  discard

proc varArg(lst: var seq[string]) = 
  sinkArg(lst)

var a = @["a", "b"]
varArg(a)
echo a

Should crash

The issue caused by the fact that in generated c code the sinkArg argument is passed by value not by pointer/reference

@cooldome
Copy link
Member Author

Another interesting question.
Do you expect output to be @["a", "b"] or @[] ?

@Clyybber
Copy link
Contributor

Clyybber commented Aug 28, 2020

I think @["a", "b"], since we didn't explicitly do sinkArg(move lst)

@cooldome
Copy link
Member Author

cooldome commented Aug 28, 2020

@Clyybber, your answer is surprise for me.

var param into sink should produce copy. Is this your the statement?

@alaviss
Copy link
Collaborator

alaviss commented Aug 28, 2020

This is how that should work from what I can tell:

proc sinkArg(x: sink seq[string]) =
  discard

proc varArg(lst: var seq[string]) = 
  # `lst` is `a`
  sinkArg(lst) # analyze if `lst` can be moved
  # last call in scope, switch to analyze `a` lifetime.

var a = @["a", "b"]
varArg(a) # analysis start
echo a # `a` is used after call to varArg() => `a` can't be moved => `lst` can't be moved.

I'm not too familiar with how Nim actually does the analysis, so this might not be the desired result, though IMO it's the least surprising.

@cooldome
Copy link
Member Author

cooldome commented Aug 28, 2020

If Nim will pass lst at line sinkArg(lst) it will be destroyed and you should get @[] in the end.
If Nim will pass a copy of lst at line sinkArg(lst) you will get @["a", "b"].

Question: what is the correct behavior according to our spec? to copy or not to copy.

@Clyybber
Copy link
Contributor

Clyybber commented Aug 28, 2020

var param into sink should produce copy. Is this your the statement?

Yeah, it should copy.

@cooldome
Copy link
Member Author

OK, then the issue is that varArg never went through injectdestructord analysis

@cooldome cooldome mentioned this issue Sep 3, 2020
Araq pushed a commit that referenced this issue Sep 4, 2020
* fix_15238

* fix test
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
* fix_15238

* fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants