cJSON icon indicating copy to clipboard operation
cJSON copied to clipboard

Integers as first-class citizens; choose int vs long long (64-bit), double vs float at compile-time

Open abzmr opened this issue 2 years ago • 5 comments

Create, parse & print integers as-such by annotation using new flag in type field. valuedouble and valueint still work as before; intended to be API and ABI compatible. This prevents losing precision in all cases where integer type has more bits than floating point mantissa.

With integer and floating-point converted to typedef names, they are changeable to long long and float at compile-time. int as 16-bit is also supported. float has proven very useful for esp32 target which has super-slow double.

Fully unit-tested in all possible combinations.

abzmr avatar May 24 '22 21:05 abzmr

@abzmr Is it normal that when using this branch c4429d5 my cJSON_AddNumberToObject() calls now result in integers being printed as decimal numbers with decimal points?

I could fix it by replacing those calls by cJSON_AddIntToObject(). But is it supposed to be like that?

canique avatar Jul 22 '22 10:07 canique

Hi @canique , nice to see your interest!

Integers created via Number functions should still print as integers, and they do on my machine with gcc (Debian 10.2.1-6) 10.2.1 20210110. This is tested by tests/print_number.c: print_number_should_print_positive_integers(). If you build the project "stand-alone" using cmake, then make all test will compile and run these tests, and report any failure.

Could you show a small example of your code and corresponding output? Also, what does a 0.123 number print like?

For any new code, I would suggest always using the new Int functions, since these will make sure it will be treated as integer. Specifically for a 64-bit compile with CJSON_INT_USE_LONGLONG, using the Int functions is required to retain full precision for large values.

abzmr avatar Jul 22 '22 21:07 abzmr

What I can tell as of now is, I'm using GCC 9.2.1 arm-none-eabi on Linux Mint.

Code was something like cJSON_AddItemToObject(link, "speed", cJSON_CreateNumber(10));

Code was executed on a Raspberry Pico (Cortex M0+).

I solved it by changing the createNumber statements to create integers.

(Reason for using this modification of cJSON is: I need to print 64bit integers.)

canique avatar Jul 23 '22 07:07 canique

The Raspberry Pico has no floating-point hardware, so any floating-point will be emulated very slowly by software. Indeed, switching to integer is the best thing to do in this case.

Digging somewhat deeper, the Pico compiler/toolchain appears to have a bug that it confuses doubles and floats, at least when printing, as witnessed here: https://forums.raspberrypi.com/viewtopic.php?t=308794#p1849547 Since cJSON formatting decision is based on printing and re-parsing, this may easily lead to wrong conclusions. An apparent fix via linker options is here: https://forums.raspberrypi.com/viewtopic.php?t=308794&sid=20ad00a4401f9c908a396f26ecf6c7ac&start=25#p1850618 Yet, whenever double precision is not really required, it is suggested to compile with CJSON_FLOAT_USE_FLOAT for some speedup, and hopefully proper formatting without needing linker options.

[Added later:] There may another thing in the Pico printf function, at least if this is the one being used: https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_printf/printf.c From my reading, the g format conversion as implemented there, does not appear to strip trailing zeros and decimal point as required by POSIX: https://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html So anyone able to verify that, please raise an issue for that printf implementation.

abzmr avatar Jul 23 '22 12:07 abzmr

Thanks for digging. I'm using CJSON_FLOAT_USE_FLOAT already + CJSON_INT_USE_LONGLONG. After all I'm printing only 2 decimal places (4 digits in total) anyway so that should have no effect on precision.

Actually for printing floats in JSON I use a different method. I'm using snprintf() where I specify the number of decimal places needed and then let cJSON know with cJSON_AddRawToObject().

canique avatar Jul 23 '22 12:07 canique