Provide fallback for missing char8_t support

[sergiud]

Clang 11.0.x with libc++ fails to compile tests in C++20 mode due to incomplete char8_t support in std::filesystem::path. Use a specific library feature guard instead of performing a generic C++20 language standard check.

Fixes #4733

• The changes are described in detail, both the what and why.
• If applicable, an existing issue is referenced.
• The Code coverage remained at 100%. A test case for every new line of code.
• If applicable, the documentation is updated.
• The source code is amalgamated by running make amalgamate.

Read the Contribution Guidelines for detailed information.

[coveralls]

coverage: 99.188%. remained the same
when pulling 34868f9 on sergiud:fs-char8_t-fallback
into 88c92e6 on nlohmann:develop.

[gregmarr]

Removing the conditional compilation adds an unnecessary copy to those using it in C++17.

[sergiud]

Unfortunately, without this change the tests fail because BasicJsonType::is_string no longer holds in corresponding from_json.

[sergiud]

Testing against 201811L seems to solve the issue. However, compilation using Clang 11.0.x starts failing again.

[gregmarr]

@sergiud Oh, that's annoying. So, libc++ in Clang 11.0.x sets __cpp_lib_char8_t 201811L but doesn't actually implement p.u8string() returning std::u8string until it's set to 201907L?

That's ugly.

However, this still means that everything using C++17 has to pay for this copy unnecessarily, where they didn't before. Would the following work?

    const auto& s = p.u8string();
#if defined(__cpp_lib_char8_t) && (__cpp_lib_char8_t >= 201907L)
    j = std::string(s.begin(), s.end());
#else
    j = s; // returns std::string in C++17
#endif

[sergiud]

Yes, __cpp_lib_char8_t version is somewhat unreliable. Therefore, instead of feature testing against a specific version for char8_t library support a better approach is probably to provide a std::u8string overload whenever __cpp_lib_char8_t is defined.

[gregmarr]

Looks reasonable.

[nlohmann]

Would it make sense to extend the CI to run the Clang tests additionally with -stdlib=libc++?

[sergiud]

The ci_test_standards_clang job in the Ubuntu Github workflow seems to be doing exactly that already. The libc++ version used there is likely much more recent and does not expose the regression.

Given GCC 14 (using libstdc++) suffers from a related issue, reproducing the setup for the latter is probably also sensible going forward.

[nlohmann]

The ci_test_standards_clang job in the Ubuntu Github workflow seems to be doing exactly that already. The libc++ version used there is likely much more recent and does not expose the regression.

Yes, these jobs use the latest Clang version. Question is if it makes sense to also do this with (some/all) of the other Clang versions we use in ci_test_compilers_clang (3.4 .. 20).

Given GCC 14 (using libstdc++) suffers from a related issue, reproducing the setup for the latter is probably also sensible going forward.

Not sure what additional setup is used there. We already use GCC 14 in the CI. Maybe @jelly can help?

[sergiud]

According to #4733 (comment) it's -fno-char8_t.

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @sergiud
Please read and follow the Contribution Guidelines.

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @sergiud
Please read and follow the Contribution Guidelines.

[sergiud]

I've no idea what Codacy's problem is.

[nlohmann]

I've no idea what Codacy's problem is.

I think we can ignore it.

[nlohmann]

I suppressed the issue in Codacy.

[nlohmann]

Looks good to me.

[nlohmann]

@gregmarr Any further comments or is this ready to merge?

[jonesmz]

Hi, my project can't update to json 3.12.0, because we build with char8_t disabled.

Is there appetite for cutting a new release with this fix included, possibly as a 3.12.1 ?