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

cstring implicit conversion to pointer broken on JS #14097

Closed
metagn opened this issue Apr 23, 2020 · 2 comments
Closed

cstring implicit conversion to pointer broken on JS #14097

metagn opened this issue Apr 23, 2020 · 2 comments

Comments

@metagn
Copy link
Collaborator

metagn commented Apr 23, 2020

For some reason cstring can implicitly convert to pointer but not ptr char, and that's true for all backends. Converting to pointer in JS though is broken because of the following:

Example

let x: pointer = cstring"abcd"
let y = x
discard y

Current Output

Generated codegen:

var x_622003 = "abcd";
var y_622004 = [x_622003[0], x_622003[1]];

Nim treats the cstring like it's a pointer already and tries to copy instead of converting. This will break, ["a", "b"] is not a valid pointer.

Expected Output

meaning expected codegen:

var x_622003 = "abcd";
var y_622004 = [x_622003, 0];

Additional Information

  • ["abcd", 0] being a valid pointer is another question, since JS cstrings are immutable, but this already happens when used like this:
var x = cstring"abcd"
let y = addr x[0]
discard y

y in this case is ["abcd", 0]. A good solution to this would be to make cstrings in JS unsafe addresses like let variables, but the same would have to apply to the implicit pointer conversion.

$ nim -v
Nim Compiler Version 1.2.0
@timotheecour
Copy link
Member

timotheecour commented Apr 24, 2020

This is nasty, and there are other issues with mapping cstring to js strings.

I feel Uint8Array is a better fit to map cstring. Considerations of backward compatibility aside, I think we should explore this option. I'm eying at something like this:

  • introduce nim type jstring that maps to js' immutable strings; nim type system forbids mutations, eg does not provide proc []=(a: jstring, index: int)
  • map nim type cstring to Uint8Array. It has very similar semantics to cstring, including mutability and ability to take views over some data

example

proc `$`(a: cstring): jstring =
  {.emit: "`result` = new TextDecoder().decode(`a`);".}

proc toCstring(a: jstring): cstring =
  {.emit: "`result` = new TextEncoder().encode(`a`);".}

@metagn
Copy link
Collaborator Author

metagn commented Nov 27, 2022

Closing because of #20761

@metagn metagn closed this as completed Nov 27, 2022
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

2 participants