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

type conversion of distinct creates erroneous copy in VM #12282

Closed
Clyybber opened this issue Sep 27, 2019 · 3 comments · Fixed by #17594
Closed

type conversion of distinct creates erroneous copy in VM #12282

Clyybber opened this issue Sep 27, 2019 · 3 comments · Fixed by #17594
Labels
Distinct Types VM see also `const` label

Comments

@Clyybber
Copy link
Contributor

Clyybber commented Sep 27, 2019

Compiling the snippet below outputs an empty line, while it should output c

Example

(EDIT) problem with distinct remains even after #15423 which deprecates TaintedString

proc test(s: var TaintedString) =
  s.string.add('c') #Works with s.add('c')

static:
  var s: string
  test(s)
  echo s

Current Output


Expected Output

c

EDIT: Example 2

see also example with seq in #9423 (comment)

Additional Information

$ nim -v
Nim Compiler Version 1.0.0
@Araq Araq added VM see also `const` label Easy labels Sep 27, 2019
@narimiran narimiran added the Hacktoberfest (Open) Issues suitable for Hacktoberfest, open for working on. label Oct 1, 2019
@krux02
Copy link
Contributor

krux02 commented Oct 22, 2019

I ran into a variation of the same bug.

type
  MyArr = distinct array[3,int]

proc foobar(dst: var array[3,int]) =
  dst[0] = 1
  dst[1] = 2
  dst[2] = 3

proc test() =
  var z: MyArr
  foobar(array[3,int](z))
  echo array[3,int](z)

test()
static:
  test()

output:

Hint: scratch [Processing]
[0, 0, 0]
Hint:  [Link]
Hint: operation successful (21904 lines compiled; 0.131 sec total; 25.664MiB peakmem; Dangerous Release Build) [SuccessX]
Hint: /tmp/scratch  [Exec]
[1, 2, 3]

This is important since my rewrite of the json.to macro requires this to work.

@timotheecour
Copy link
Member

timotheecour commented Apr 24, 2020

@Clyybber can you change the title to something like: T <=> distinct[T] conversion creates a copy in VM

  • it's specific to distinct IIUC
  • it's not limited to var param, eg see this:
when true:  #D20200424T162848:here
  type Foo = distinct string
  proc test() =
    var s: Foo
    s.string.add('c')
    doAssert s.string == "c" #fails
  static: test()

borrow works:

when true:
  type Foo = distinct string
  proc add(a: var Foo, b: char) {.borrow.}
  proc test() =
    var s: Foo
    s.add('c')
    doAssert s.string == "c" #ok
  static: test()

conversion TO distinct also fails

when true:
  type Foo = distinct string
  proc add(a: var Foo, b: char) {.borrow.}
  proc test() =
    var s: string
    s.Foo.add('c')
    doAssert s.string == "c" # fails
  static: test()

@Clyybber Clyybber changed the title var param with type conversion doesn't work in VM type conversion of distinct creates erroneous copy in VM Apr 25, 2020
@Clyybber Clyybber removed Easy good first issue Hacktoberfest (Open) Issues suitable for Hacktoberfest, open for working on. labels Nov 15, 2020
@timotheecour
Copy link
Member

see also example with seq from #9423 (comment), closing #9423 as duplicate of this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Distinct Types VM see also `const` label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants