[resolved] JsBigInt as primitive type, not ref type (for js BigInt PR 16409)

[timotheecour]

refs nim-lang#16409 (comment)

instead of type JsBigInt* = ref object of JsRoot which gives incorrect ref semantics to jsBigInt, I propose this:

when true:
  type JsBigIntImpl {.importc: "bigint".} = int
  type JsBigInt* = distinct JsBigIntImpl
  func big*(integer: SomeInteger): JsBigInt {.importjs: "BigInt(#)".}
  func big*(integer: cstring): JsBigInt {.importjs: "BigInt(#)".}
  func `<=`*(x, y: JsBigInt): bool {.importjs: "(# $1 #)".}
  func `==`*(x, y: JsBigInt): bool {.importjs: "(# === #)".}
  func inc*(x: var JsBigInt) {.importjs: "[#][0][0]++".}
  func toString*(this: JsBigInt): cstring {.importjs: "#.toString()".}
  func `$`*(this: JsBigInt): string =
    $toString(this)

  import jsconsole
  let z1=big"10"
  let z2=big"15"
  doAssert z1 == big"10"
  doAssert z1 == z1
  doAssert z1 != z2
  for i in z1 .. z2:
    console.log(i)
    echo i
  block:
    var a=big"3"
    a.inc
    doAssert a == big"4"
  var z: JsBigInt
  echo z
  console.log(z)
  doAssert z != big(1)
  echo "before bug"
  doAssert z == big"0" # fails

it seems like a strict improvement over type JsBigInt* = ref object of JsRoot but there's 1 more problem to address: the last assert fails (note: it would still fail with type JsBigInt* = ref object of JsRoot )

this may require a codegen fix, which could be backported if we want 1.0.x and 1.2.x to be able to use jsBigInt

alternatives

• type JsBigInt* = ref object of JsRoot would fail here: echo z, giving:
  result_704643094 = nimCopy(null, cstrToNimstr(this_704643093.toString()), NTI33554437);
  and the nimCopy is inefficient

• type JsBigInt* = object of JsRoot would fail here:
  for i in z1 .. z2: would be skipped, and doAssert a == big"4" would fail

• type JsBigInt* = distinct int would also work (might be simpler in fact)

links

D20210105T181036

[juancarlospaco]

If you remove the var it still works, so maybe can be done without bigger changes.

I dont know, I am neutral to this, but would be nice to not block the PR way too long, is +90% done. How other JS modules do it then?.

[timotheecour]

If you remove the var it still works

what do you mean?

is +90% done

yes; IMO that's the last thing to figure out, the rest is done (modulo a few minor stuff that we can address after PR is merged)

not block the PR way too long

i agree, but I want to solve this problem before committing to a suboptimal design; I think it's solvable

How other JS modules do it then?.

you mean other languages? or other nim modules ? not sure what you mean; BigInt really is a primitive type in js, not an "object"; it's on par with Number or string

[juancarlospaco]

func inc*(x: JsBigInt) {.importjs: "(#++)".}

Works, try it.

[timotheecour]

ya i knew about that, but it violates nim semantics, not good enough :)

[juancarlospaco]

Last resource workaround can be that because it wont break public api (?).

Whats the problem with distinct int way ?.

[timotheecour]

it also fails the last test

[timotheecour]

I think I have a perfect solution: nim-lang#16606

[timotheecour]

this has been incorporated; (is there a way to close conversations, like we close PRs/issues ?)

deleting is obviously bad, so is freezing; I miss following features: closing/re-opening, and adding labels

EDIT: i guess that's community/community#3097 and community/community#19

[juancarlospaco]

I see a Lock Conversation top right.

[juancarlospaco]

Some "Categories" have the option to mark as "Resolved",
by default only "Q&A" has the option to mark as Resolved,
you can create a new category and it should give the option to allow to mark as Resolved, or not.