Issues with int64_t (and related) type due to ambiguity

[lss4]

It seems json5pp can't be used with int64_t (long in Linux, long long in Windows) by default, generating this error:

error: conversion from ‘int64_t’ {aka ‘long int’} to ‘const json5pp::value’ is ambiguous

../../json5pp/json5pp.hpp:179:3: note: candidate: ‘json5pp::value::value(json5pp::value::integer_type)’
  179 |   value(integer_type integer) noexcept : type(TYPE_INTEGER), content(integer) {}
      |   ^~~~~
../../json5pp/json5pp.hpp:173:3: note: candidate: ‘json5pp::value::value(json5pp::value::number_type)’
  173 |   value(number_type number) noexcept : type(TYPE_NUMBER), content(number) {}
      |   ^~~~~
../../json5pp/json5pp.hpp:167:3: note: candidate: ‘json5pp::value::value(json5pp::value::boolean_type)’
  167 |   value(boolean_type boolean) noexcept : type(TYPE_BOOLEAN), content(boolean) {}
      |   ^~~~~

It seems json5pp's integer_type is int. Not sure if there's a way to accommodate every integer types.

For now I need to explicitly specify a larger integer type in the header for 64-bit integers. In my case, changing this: using integer_type: int; to using integer_type: long long; would enable me to use it with long long type to accommodate 64-bit integers. However, this has a price: all integers must be explicitly cast to long long regardless, and immediate values, like 0, need to be written as 0LL.

[kimushu]

Thanks for your comment. Basically, all numbers in JSON are 64-bit floating point (double in C/C++) based on JavaScript specification. Integer support has added since 2.2.0 to handle integers more efficiently.

To support int64_t, some fixes are required:

(1) Modifying internal buffer (json5pp::value::content):

• (1-A) Modify integer_type from int to long long
  • Easy. No additional space required (because union content already has 64-bit length)
  • Generated code will be bigger/slow on some 32-bit architectures.
• (1-B) Add long long content::longlong into union content
  • Parse logic should be fixed to support both of int and long long
  • Smaller extra cost on 32-bit architectures.

(2) Add setter operator=() overloads for long long type.

• Easy. int can be casted to long long without loss (LLP64/LP64/ILP32)

(3) Add getter for long long type:

• (3-A) Modify return type of as_integer() to long long (Used with 1-A)
  • This breaks API compatibility.
  • Generated code will be bigger/slow on some 32-bit architectures.
• (3-B) Add as_longlong() and is_longlong() to support long long
  • This could be annoying to json5pp users because the users must choose the right one to get integer values.

How do you think?

[lss4]

Actually, 1-A was exactly what I did to enable 64-bit integers. However, this doesn't fix the ambiguity issue because all integer types can be implicitly cast to double or boolean, which caused the error in the first place.

Since the constructor for integer values is meant for int, passing an integer value of any other type than int would lead to this ambiguity, as it can be accepted by all 3 constructors.

Using the 1-A method would require all integer inputs to be explicitly cast to long long. For immediate values, the LL suffix would become mandatory.

The constructor ambiguity needs to be resolved in order to allow integer types to be used freely.

On the other hand, if performance/code size on 32-bit architecture is a concern, maybe it's a good idea to actually check the target system's architecture (if possible) to decide which code path is the best.

EDIT: Revised a bit.