C4244 implicit sign-conversion

[JurgenLB]

completed the fix for the MSVC C4244 warning that was appearing in the AppVeyor build.

Two compiler warnings from include/picojson/picojson.h line 387 when compiling with -Wsign-conversion -Wconversion:

implicit conversion from 'int64_t' to 'double' may lose precision — u_.int64_ was implicitly cast to double for the u_.number_ assignment
implicit conversion turns floating-point number into integer: 'double' to 'bool' — the assignment result (double) was the right operand of &&, requiring implicit float→bool conversion

Kind regards

[prince-chrismc]

Can you share the compilation flags you are using? It would be a shame to fix all of these and have them come back.

I'd like to add a test in CI if possible

[prince-chrismc]

Any reason why auto wassufficemt?

[JurgenLB]

Your right looking at the code auto should do just fine,
I will try to change that.

Kind regards

[JurgenLB]

copilot now says;
I can see from PR #405 that it fixed:
Several data.size() calls to use static_cast(data.size())
std::chrono::seconds(std::llround(as_number())) to std::chrono::seconds(static_cast<int64_t>(std::llround(as_number())))
But it did NOT fix: date(std::chrono::seconds(as_integer())) - this line still lacks an explicit cast.
Now I'm looking at the actual issue in the current codebase: the basic_claim constructor is trying to narrow a long long count value down to json_traits::integer_type, which on jsoncpp is just int.

Let me look at what functions these are and their signatures to understand if there's a C4244 warning:

OSSL_PARAM_BLD_push_utf8_string(param_bld.get(), "group", group.data(), group.size()) - takes size_t. group.size() returns size_t. No C4244.

EVP_DigestSignUpdate(ctx.get(), ..., data.size()) - on wolfSSL, this takes size_t. data.size() is size_t. No C4244.

EVP_DigestSign(ctx.get(), ..., data.size()) - takes size_t. No C4244.

EVP_DigestVerifyUpdate(ctx.get(), ..., data.size()) - takes size_t. No C4244.

EVP_DigestVerify(ctx.get(), ..., data.size()) - takes size_t. No C4244.

Now I need to focus on where the actual issue is. Looking at the basic_claim constructor that takes a date parameter...

I'm examining how the constructor converts the epoch time to an integer type. When integer_type is int32_t, we're casting a long_long value from the duration count, which should be an explicit conversion that MSVC wouldn't warn about. But I need to verify whether MSVC treats functional casts differently from static_cast when it comes to C4244 warnings—the documentation says it fires for implicit conversions.

Try with stricter conversion warnings

g++ -std=c++17 -Wall -Wextra -Wconversion -Wsign-conversion -I/home/runner/work/jwt-cpp/jwt-cpp/include -I/home/runner/work/jwt-cpp/jwt-cpp/include/picojson -o /tmp/test_jwt3 /tmp/test_jwt3.cpp -lssl -lcrypto 2>&1 | head -80
In file included from /tmp/test_jwt3.cpp:1:
/home/runner/work/jwt-cpp/jwt-cpp/include/jwt-cpp/jwt.h: In member function ‘std::string jwt::algorithm::hmacsha::sign(const std::string&, std::error_code&) const’:
/home/runner/work/jwt-cpp/jwt-cpp/include/jwt-cpp/jwt.h:1406:103: warning: conversion to ‘size_t’ {aka ‘long unsigned int’} from ‘int’ may change the sign of the result [-Wsign-conversion]
 1406 |                                                  reinterpret_cast<const unsigned char*>(data.data()), static_cast<int>(data.size()),
      |                                                                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<exited with exit code 0>

EVP_DigestVerifyFinaltookunsigned intfor the signature length in 1.0.x but switched tosize_tin 1.1.0+.

g++ -std=c++17 -Wconversion -Wsign-conversion -I/home/runner/work/jwt-cpp/jwt-cpp/include -I/home/runner/work/jwt-cpp/jwt-cpp/include/picojson -c /home/runner/work/jwt-cpp/jwt-cpp/include/picojson/picojson.h 2>&1 | head -30

<exited with exit code 0>