janet-lang / jhydro

Crypto for Janet
https://janet-lang.github.io/jhydro/
MIT License
22 stars 7 forks source link

add math header #6

Closed tsujp closed 3 years ago

tsujp commented 3 years ago

Without the header the build fails:

compiling jhydro.c to build/jhydro.o...
jhydro.c:81:36: error: implicitly declaring library function 'floor' with type 'double (double)' [-Werror,-Wimplicit-function-declaration]
    if (d < 0 || d > UINT32_MAX || floor(d) != d) {
                                   ^
jhydro.c:81:36: note: include the header <math.h> or explicitly provide a declaration for 'floor'
1 error generated.
error: command failed with non-zero exit code 1
  in os/execute
  in shell [/macports/bin/jpm] (tailcall) on line 141, column 3
  in do-rule [/macports/bin/jpm] on line 260, column 26
  in do-rule [/macports/bin/jpm] on line 256, column 44
  in do-rule [/macports/bin/jpm] (tailcall) on line 256, column 44
  in _thunk [/macports/bin/jpm] on line -1, column -1
  in cli-main [boot.janet] on line 3371, column 39

With:

compiling jhydro.c to build/jhydro.o...
compiling hydrogen.c to build/hydrogen.o...
linking build/jhydro.so...
generating meta file build/jhydro.meta.janet...
compiling jhydro.c to build/jhydro.static.o...
compiling hydrogen.c to build/hydrogen.static.o...
creating static library build/jhydro.a...

Tests:

running test/kx.janet ...
✔✔✔✔✔✔
6 of 6 tests passed.

running test/suite1.janet ...
✔✔✔✔✔
5 of 5 tests passed.

running test/suite2.janet ...
✔✔
2 of 2 tests passed.

All tests passed.
tsujp commented 3 years ago

@pepe you mentioned this builds fine on Linux, do we want this header conditionally included IFF macOS or always (as the PR currently is)?

pepe commented 3 years ago

I would say in ifdef but maybe it is OK like this.

pepe commented 3 years ago

It builds just OK without the ifdef on my void musl Linux. So maybe let it be like this and @bakpakin will have final word.

tsujp commented 3 years ago

Ack

bakpakin commented 3 years ago

LGTM