Fix compiler warning (clang, cxx17)

[JonasBorchelt]

warnings:

'wstring_convert<std::codecvt_utf8<wchar_t, 1114111, 0>>' is deprecated
'wstring_convert<std::codecvt_utf8<wchar_t, 1114111, 0>>' has been explicitly marked deprecated here

[prince-chrismc]

For the linters there's a command you can run :) https://github.com/Thalhammer/jwt-cpp/actions/runs/4939213168/jobs/8838847109?pr=290 its just a copy paste from the logs

[prince-chrismc]

Found this gem doing some research on the issue 🤣

The replacement is praying for C++23 to have proper Unicode support

3 yr. ago
!remindme 2023

---

https://isocpp.org/files/papers/D2139R3.html#3.21

Seems like there's no replacement but the tests are passing so 🤞

[Thalhammer]

Looks good, except a small nitpick and a question.

Given that the vast majority of data going through this function will be ascii, all three of str, wide and out will usually have the same size and it might be a good idea to call reserve on wide and out with the input size to prevent reallocations. With reserve worst case we overallocate a bit (on wide) or have to reallocate once (for out).

The other, bigger thing I am worried about is whether its really a good idea to replace the old/deprecated wstring_convert with the even older mbrtowc C api (which by the way is defined in cwchar, not in cstring. While I agree that we shouldn't use deprecated functions if it can be avoided, the wstring_convert code is just so much cleaner and I just can't see them removing it before there's an usable replacement.

EDIT: Oh and linting. you can just apply the patch command thats in the details of the failed ci run.

[prince-chrismc]

replace the old/deprecated wstring_convert with the even older mbrtowc C api

Would it make sense to drop the the C++11 ifdef? Or have a seperate >= C++17 section?

[Thalhammer]

replace the old/deprecated wstring_convert with the even older mbrtowc C api

Would it make sense to drop the the C++11 ifdef? Or have a seperate >= C++17 section?

If we go with this solution we can certainly drop the ifdef because the c-style functions should be available in all C++ versions.

[JonasBorchelt]

Hey guys,

sorry for being late. Thank you both for the hint regarding the patch command.

@Thalhammer

• You raise a good point. While it is generally advisable to avoid deprecated APIs, sometimes there isn't a clear replacement. As of my knowledge, there is no new C++ API that directly replaces std::wstring_convert. The code provided uses the C API as a workaround (as you already stated), which can be of course less clean and intuitive than the C++ API. Once std::text becomes available, it would be the recommended way to convert between string encodings in C++.

• Another good point regarding the reserve, I will add it.

• cstring is needed for std::strerror(errno)

@prince-chrismc Good point, I will drop it.

Thank you both for your feedback!

[Thalhammer]

@laoshanxi Do you still need support for GCC 4.8 ?
I cancelled the check because the 18.04 ubuntu image has been deprecated end of 2022 and apparently has been removed now because it can't find a runner anymore. If you still need it, could you check if those changes work in your setup ?

[laoshanxi]

@laoshanxi Do you still need support for GCC 4.8 ? I cancelled the check because the 18.04 ubuntu image has been deprecated end of 2022 and apparently has been removed now because it can't find a runner anymore. If you still need it, could you check if those changes work in your setup ?

Let me have a try.

[laoshanxi]

I tried build jwt-cpp in gcc 4.8 works, sorry I can not trigger whole app-mesh which used jwt-cpp due to other dependency can not pass this compiler.

gcc version 4.8.5 20150623 (Red Hat 4.8.5-44) (GCC)

For me, I am not using this compiler now, Thanks for asking.