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

better importjs: func fn(a: var int, b: int) {.importjs: "$2 = $3 + $2".} #315

Open
timotheecour opened this issue Jan 14, 2021 · 5 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Jan 14, 2021

problem 1: var params

when var params are involved, the wrappers in std/jsbigints are more complicated than they should, and maybe even create overhead:

func inc*(this: var JsBigInt; amount: JsBigInt) {.importjs: "([#][0][0] += #)".} =

problem 2: referring to arguments out of order or multiple times

importjs doesn't allow referring to arguments by position, creating more complex and potentially less efficient wrappers:

func wrapToUint*(this: JsBigInt; bits: Natural): JsBigInt {.importjs:
  "(() => { const i = #, b = #; return BigInt.asUintN(b, i) })()".} =

proposal

  • P1 extent the $1 notation (which maps to the proc name) to allow referring to params (eg: $2 is 1st param)
    example:
func wrapToUint*(this: JsBigInt; bits: Natural): JsBigInt {.importjs:
  "(() => { const i = #, b = #; return BigInt.asUintN(b, i) })()".} =
=>
func wrapToUint*(this: JsBigInt; bits: Natural): JsBigInt {.importjs: "BigInt.asUintN($3, $2)"
  • P2 make the dereferencing for var params implicit: $2 maps to ($2)[0] in js
    example:
func inc*(this: var JsBigInt; amount: JsBigInt) {.importjs: "([#][0][0] += #)".}
=>
func inc*(this: var JsBigInt; amount: JsBigInt) {.importjs: "$2 += $3".}

example with a var param and a param referred to more than once:

func myFun*(a: var int, b: int) {.importjs: "$2 = $2 + $3".}
  • P3 make jsgen auto-insert () around each param so these don't need to appear in declaration (to prevent
    example:
func `**`*(x, y: JsBigInt): JsBigInt {.importjs: "((#) $1 #)".}
=>
func `**`*(x, y: JsBigInt): JsBigInt {.importjs: "$2 $1 $3".}
# instead of
func `**`*(x, y: JsBigInt): JsBigInt {.importjs: "($2) $1 $3".}

note: see nim-lang/Nim#16409 (comment) as to why parens are needed in the first place

  • P4 (implicitly assumed in above examples): don't require a () wrapping the whole importjs expression, as soon as at least one un-escaped $ is detected; this is not the same as P3.
func `==`*(x, y: JsBigInt): bool {.importjs: "(# == #)".}
=>
func `==`*(x, y: JsBigInt): bool {.importjs: "$2 == $3".}
# instead of:
func `==`*(x, y: JsBigInt): bool {.importjs: "($2 == $3)".}

TODO

figure out what to do with:

func debugger*() {.importjs: "debugger".} # should codegen as `debugger;`, not `debugger();`

refs nim-lang/Nim#17296 (comment)
can this work non-ambiguously?

func debugger*() {.importjs: "debugger".}
func debugger*() {.importjs: "$1".} # or even this

Using importjs there gives importjs for routines requires a pattern errors, since apparently importjs doesn't behave the same as importc and importcpp. Should I open an issue for this or just add patterns for the imports (in a future PR)?

links

importjs pragma is underdocumented and buggy · Issue #488 · timotheecour/Nim

@juancarlospaco
Copy link
Contributor

Automatic implicit dereferencing is disabled by default for C target tho.

@timotheecour
Copy link
Member Author

Automatic implicit dereferencing is disabled by default for C target tho.

I'm assuming you're referring to P2.
i think that's a different issue though; P2 applies to var params only and says nothing about ptr or ref types; all it does is translate x: var T to x[0] when referred to via $:

func fn(x: var T; b: T2) {.importjs: "$2 += $3".}

fn(expr1, expr2) will map to js code: (expr1)[0] += (expr2)

please elaborate

@timotheecour
Copy link
Member Author

@juancarlospaco since you'd also be one benefitting from this with your js PR's, would you consider working on this?

@konsumlamm
Copy link

konsumlamm commented Mar 20, 2021

While working on nim-lang/Nim#17422, I got a lot of `importjs` for routines requires a pattern errors, this restriction doesn't seem to apply when using importc or importcpp (for JS). It seems weird that these import pragmas can do more than importjs, even when using them to import JS functions/types, so I think this should be fixed as well.

(EDIT(timotheecour): i had to change the link you wrote since it's in another repo: #17422 => nim-lang/Nim#17422)

@timotheecour
Copy link
Member Author

timotheecour commented Jul 2, 2021

maybe instead of importjs this should be: pattern, because it'd also be useful for importc, eg, I'm working on std/int128 and would need it there:

proc `+`*(a, b: int128): int128 {.importcpp: "(# + #)".} # works, but requires C++
proc `+`*(a, b: int128): int128 {.importc: "(# + #)".} # doesn't work obviously
proc `+`*(a, b: int128): int128 {.pattern: "# + #".} # what i want (this RFC)

the same syntax would work in c,cpp,js

see also: nim-lang/Nim#15653 (comment) and #399 (comment)

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

No branches or pull requests

3 participants