ianfhunter / GNOLL

GNOLL is an efficient dice notation parser for multiple programming languages that supports a wide set of dice notation
https://www.ianhunter.ie/GNOLL/
GNU General Public License v3.0
39 stars 23 forks source link

JavaScript fails to build on macOS #463

Closed michaeljmcd closed 4 months ago

michaeljmcd commented 4 months ago

Running make javascript on macOS fails with errors like the following.

  lex.yy.c:1465:59: error: implicit conversion changes signedness: 'int' to 'yy_size_t' (aka 'unsigned long') [-Werror,-Wsign-conversion]
   1464 |                         yy_size_t num_to_read =
        |                                   ~~~~~~~~~~~
   1465 |                         YY_CURRENT_BUFFER_LVALUE->yy_buf_size - number_to_move - 1;
        |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~

To reproduce, I used the following steps:

$ brew install emscripten
$ make all
$ make javascript

The last command errors as above, full log file attached as log.txt.

Expected behavior

JavaScript to build fully on macOS.

Environment (please complete the following information):

Additional context

emcc --version                                                                                                                                                           
emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.55-git
Copyright (C) 2014 the Emscripten authors (see AUTHORS.txt)
This is free and open source software under the MIT license.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Building from main.

github-actions[bot] commented 4 months ago

Thanks for filing an issue! We'll get back to you as soon as we can with a response.

ianfhunter commented 4 months ago

@michaeljmcd it looks like lex/flex is generating some code that is creating that issue.

There was a gap in our testing, in that we only tested javascript on Linux. While I look at updating our tests, maybe you will find it works if you add -Wno-error=sign-conversion to src/js/target.mk.

e.g. https://github.com/ianfhunter/GNOLL/blob/b9be54bd3065834db465c3120ffef6185a513156/src/js/target.mk

michaeljmcd commented 4 months ago

If I pull the mac-js branch and build, I still get a build error:

lex.yy.c:1534:38: error: comparison of integers of different signs: 'yy_size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
 1534 |         if (((yy_n_chars) + number_to_move) > YY_CURRENT_BUFFER_LVALUE->yy_buf_size) {
      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ianfhunter commented 4 months ago

@michaeljmcd I'm having some issues setting up a mac machine, but you can try pull the latest version of that branch now. I just tried to disable all warning-to-error elevations so we (hopefully) don't have to go back and forth

michaeljmcd commented 4 months ago

The JavaScript file builds off latest, but something seems off when I attempt to invoke it from the command line.

$ ./build/dice 3d20
Result:
37;% 
$ node build/js/a.out.js 3d20  
Result:
$ node --version
v21.7.1

I'm not sure if the lack of a result is based on an invocation error or something in the build.

ianfhunter commented 4 months ago

This appears to be the same in linux. I'll have to take a deeper look at it and get back to you Thanks for debugging with me

ianfhunter commented 4 months ago

@michaeljmcd I think it should print the right result now. The old code wrote to the file system before printing out the result. However, it seems Emscripten doesn't save file changes. I've updated it to just print directly.

michaeljmcd commented 4 months ago

Latest works for me. I do see what looks like a debugging statement in the output at the end.

$ node build/js/a.out.js "3d20+1"
Trying to roll '3d20+1'
12;
ErrorCheck: No Errors.
ianfhunter commented 4 months ago

👍 Great. I've eliminated that debug statement in that branch + I'll be merging the changes to main soon