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

unsafeAddr broken in JS for proc parameters #14100

Open
metagn opened this issue Apr 24, 2020 · 5 comments
Open

unsafeAddr broken in JS for proc parameters #14100

metagn opened this issue Apr 24, 2020 · 5 comments

Comments

@metagn
Copy link
Collaborator

metagn commented Apr 24, 2020

Example

proc foo[T](x: T) = # generics doesn't matter here, you can try writing a proc for each type used below
  echo(unsafeAddr(x)[])

foo("hello")
foo(@[1, 2, 3])
foo([1, 2, 3])
foo(1)

import jsconsole

proc consoleFoo[T](x: T) =
  console.log(unsafeAddr(x)[])

consoleFoo("hello")
foo(@[1, 2, 3])
foo([1, 2, 3])
foo(1)

Current Output


@[]
[undefined, undefined, undefined]
undefined

1
1
undefined

Expected Output

"hello"
@[1, 2, 3]
[1, 2, 3]
1
"hello"
@[1, 2, 3]
[1, 2, 3]
1

Additional Information

$ nim -v
Nim Compiler Version 1.2.0
@bung87
Copy link
Collaborator

bung87 commented Nov 11, 2020

this was introduced in 61f2f1f

@ringabout
Copy link
Member

ringabout commented Nov 12, 2020

this was introduced in 61f2f1f

No, I guess this never works, Nim v1.0.4 doesn't work too.

@bung87
Copy link
Collaborator

bung87 commented Nov 12, 2020

there's lot changes after v1.0.4, test agains it does not results "never works"

worked version maybe #15078

@ringabout
Copy link
Member

In Nim 0.19.0

proc foo[T](x: T) = # generics doesn't matter here, you can try writing a proc for each type used below
  echo(unsafeAddr(x)[])

foo("hello")
foo(@[1, 2, 3])
foo([1, 2, 3])
foo(1)

gave:


@[]
D:\QQPCmgr\Desktop\Nim\nimcache\test.js:475
    throw new Error(cbuf_15801);
    ^

Error: Error: unhandled exception: index out of bounds [IndexError]
Traceback (most recent call last)
foo.foo, line: 2
$.$, line: 3486
collectionToString.collectionToString, line: 2176

    at unhandledException (D:\QQPCmgr\Desktop\Nim\nimcache\test.js:475:11)
    at raiseException (D:\QQPCmgr\Desktop\Nim\nimcache\test.js:264:3)
    at raiseIndexError (D:\QQPCmgr\Desktop\Nim\nimcache\test.js:490:3)
    at chckIndx (D:\QQPCmgr\Desktop\Nim\nimcache\test.js:284:3)
    at collection_to_string_25683 (D:\QQPCmgr\Desktop\Nim\nimcache\test.js:645:29)
    at HEX24_25674 (D:\QQPCmgr\Desktop\Nim\nimcache\test.js:683:32)
    at foo_25666 (D:\QQPCmgr\Desktop\Nim\nimcache\test.js:694:11)
    at Object.<anonymous> (D:\QQPCmgr\Desktop\Nim\nimcache\test.js:711:1)
    at Module._compile (internal/modules/cjs/loader.js:1200:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1220:10)
Error: execution of an external program failed: 'E:\node\node.exe D:\QQPCmgr\Desktop\Nim\nimcache\test.js '

@metagn
Copy link
Collaborator Author

metagn commented Nov 12, 2020

I don't think there's really a way for this to be "introduced", this is very special behavior that the compiler must account for that I haven't seen implemented. You would have to track whether a proc uses unsafeAddr for a parameter, then generate the function signature differently depending on it. I think doing that would be dumb, a better way would be to disallow it (which I know we do a lot, but here it makes sense) and tell the user to use a ptr T parameter instead.

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