Add math.copySign

[ringabout]

ref nim-lang/RFCs#92

C99 function
http://www.cplusplus.com/reference/cmath/copysign/

Python:
https://docs.python.org/3/library/math.html#math.copysign

Crystal:
https://github.com/crystal-lang/crystal/blob/5999ae29beacf4cfd54e232ca83c1a46b79f26a5/src/math/math.cr#L95

Dlang:
https://dlang.org/library/std/math/copysign.html

Rust:
https://doc.rust-lang.org/beta/std/primitive.f64.html#method.copysign

sgn is ugly
https://stackoverflow.com/questions/1986152/why-doesnt-python-have-a-sign-function
And copysign is more useful
https://bugs.python.org/msg59154

[ringabout]

• remove generics C function
• add VM version

[ringabout]

Better implementation for JS backend

import math

proc copysign(x, y: float): float =
  if y > 0.0 or (y == 0.0 and 1.0 / y > 0.0):
    result = abs(x)
  else:
    result = -abs(x)

proc main() =
  echo copySign(3.0, 0.0)
  echo copySign(3.0, -0.0)

static: main()
main()

[ringabout]

Why I can't register a VM verision of copySign?
Error: undeclared identifier: 'copySign'
FAILURE
https://builds.sr.ht/~araq/job/377122

[timotheecour]

P1

Why I can't register a VM verision of copySign?

because of bootstrap (building from 0.20.0, which disables {.since: (1,5,1).}), you can reproduce this locally with:

rm bin/nim_csources
rm bin/nim
sh build_all.sh

or even simpler: bin/nim_csources c --lib:lib compiler/nim.nim

solution:
in vmops.nim:

# on top
when declared(math.copySign):
  from math import copySign

# in registerAdditionalOps:
  when declared(copySign):
    wrap2f_math(copySign)

this works.
(refs #11865 and #14648 which would've simplified this a bit)

P2

But after that, there's another issue,

nim r tests/stdlib/tmath.nim will fail at CT with:
doAssert copySign(-1.0, 0.0) == 1.0

after debugging by adding debugEcho (x, y, $T) inside copySign, it shows:
(-1.0, -0.0, "BiggestFloat")

so looks like 0.0 is interpreted as -0.0 in VM for some reason.

EDIT: fixing P2 in #16470

[timotheecour]

now that P2 was fixed (refs #16406 (comment)), I guess this PR can be continued?

[ringabout]

I will continue this PR soon.

[timotheecour]

until timotheecour#499 is fixed, this test is meaningless:

        doAssert copySign(-1.0, -NaN) == 1.0
        doAssert copySign(-1.0, NaN) == 1.0

both pass currently, which is not correct behavior IMO

IMO doAssert copySign(-1.0, NaN) == 1.0 is probably correct (we can check with signbit once we have it), but doAssert copySign(-1.0, -NaN) == 1.0 is probably not correct, and should be:

when false: # xxx bug
  doAssert copySign(-1.0, -NaN) == -1.0

likewise with doAssert copySign(10.0, -NaN) == 10.0

[timotheecour]

this test actually works in VM so long nim is compiled after this PR.
Simplest is to defined nimHasCopySign in condsyms.nim and then:

    template fn() = doAssert copySign(1.0, copySign(NaN, -1.0)) == -1.0
    when defined(js): discard # xxx bug
    else:
      when defined(nimHasCopySign): fn()
      else:
        when nimvm: discard
        else: fn()

(refs timotheecour#498)

note, the same nimHasCopySign could be re-used in your other math PR's that need vmops to avoid creating too many condsyms symbols (until we finally reconsider the much better nimVersionCT approach, refs #14648)

[ringabout]

I would rather make a follow-up PR to enable this test.

[ringabout]

And tmath.nim should support JS, I could try to enable JS tests(at least for copySign) in the next PR.

[timotheecour]

ya we definitely need this (in another PR as you say); refs timotheecour#494 (comment) ; it just requires a few disabling's for js to work (and vm already works)

[timotheecour]

it won't fail in VM for a nim binary compiled after your PR

[ringabout]

The same story as #16406 (comment)

[timotheecour]

LGTM, remaining comments can be addressed in followup PR's

[timotheecour]

better code for js via Object.is, see #16505 (comment)