Fix cppcheck 1.5.1 warning

[nlohmann]

In https://github.com/nlohmann/json/pull/4753, a new warning was raised by the new Cppcheck version:

include/nlohmann/detail/output/serializer.hpp:835:36: warning: %g in format string (no. 2) requires ‘double’ but the argument type is ‘number_float_t’. [invalidPrintfArgType_float]
        std::ptrdiff_t len = (std::snprintf)(number_buffer.data(), number_buffer.size(), “%.*g”, d, x);
                                   ^

We need to check whether to suppress or fix it.

[nlohmann]

I can reproduce the issue locally.

The code path to this snprintf call is chosen if

        static constexpr bool is_ieee_single_or_double
            = (std::numeric_limits<number_float_t>::is_iec559 && std::numeric_limits<number_float_t>::digits == 24 && std::numeric_limits<number_float_t>::max_exponent == 128) ||
              (std::numeric_limits<number_float_t>::is_iec559 && std::numeric_limits<number_float_t>::digits == 53 && std::numeric_limits<number_float_t>::max_exponent == 1024);

is false. However, if number_float_t is double or long double, then is_ieee_single_or_double is true.

So the warning seems correct, because the snprintf call is only executed when number_float_t is neither double nor long double.

Related: #3929

[nlohmann]

I wanted to add a test for a WIP PR #4858 and found out that the two code paths return different results for number_float_t == double:

With an example like

const nlohmann::json j = 3.141592653589793116;
j.dump();

calling

void dump_float(number_float_t x, std::false_type)  // snprintf

yields 3.1415926535897931 and calling

void dump_float(number_float_t x, std::true_type)  // grisu2

yields 3.141592653589793.

I realized that void dump_float(number_float_t x, std::false_type) is never called by a test case and that it should be equivalent to grisu2 in case of double. Any ideas?

[gregmarr]

I'm not sure how this function can work. We don't know what type number_float_t is so we can't create a proper format string for snprintf without casting it to something fixed. Maybe we just always cast to double? It looks like @theodelrieu wrote this function back in 2017.

[nlohmann]

I'm not sure how this function can work. We don't know what type number_float_t is so we can't create a proper format string for snprintf without casting it to something fixed. Maybe we just always cast to double? It looks like @theodelrieu wrote this function back in 2017.

Yes, I think casting to double is the best thing we can do here.

But any idea on the different behavior between the old approach (snprintf) and the new (grisu2) in https://github.com/nlohmann/json/issues/4755#issuecomment-2826575259?

[t-b]

Can't we use an iostream instead of snprintf and let the compiler figure out the type? From my understanding this is a fallback path and thus not a hot one.

[gregmarr]

Yes, I think casting to double is the best thing we can do here.

But any idea on the different behavior between the old approach (snprintf) and the new (grisu2) in #4755 (comment)?

My understanding is that grisu2 provides the minimal length that is unambiguously convertible back to the original value. That means that the 1 at the end is not strictly necessary, but is there because we specified the length.

Can't we use an iostream instead of snprintf and let the compiler figure out the type? From my understanding this is a fallback path and thus not a hot one.

That's certainly possible, and sounds reasonable.

[nlohmann]

I did this in #4755. PTAL.

[gregmarr]

@nlohmann I think you mean #4758

[nlohmann]

Yes, sorry.

[gregmarr]

Okay, so that's the "cast to double" solution rather than the "use iostreams" solution that @t-b suggested. I think that since we don't know what type x is, and whether or not it can safely be cast to double, then iostream may be the best solution.

[nlohmann]

Ok - I think it will also be easier to test. I'll make a proposal.

[nlohmann]

PTAL: #4758. I currently fail to create a test case for this.

[github-actions[bot]]

This issue has been marked as stale because it has been open for 90 days without activity. If this issue is still relevant, please add a comment or remove the "stale" label. Otherwise, it will be closed in 10 days. Thank you for helping us prioritize our work!

[github-actions[bot]]

This issue has been closed after being marked as stale for 10 days without any further activity. If this was done in error or the issue is still relevant, please feel free to reopen it or create a new issue. We appreciate your understanding and contributions.