Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix char_traits deprecation warning #4179

Merged
merged 8 commits into from
Nov 27, 2023
Merged

Conversation

colbychaskell
Copy link
Contributor

@colbychaskell colbychaskell commented Oct 6, 2023

Addresses issue #4163.

Fixes the deprecation warning for char_traits with signed and unsigned char. These types have been explicitly marked as deprecated in the latest version of LLVM.

More information on the deprecation can be found in this announcement from LLVM.

This creates a custom char_traits struct that will extend the std::char_traits struct. The primary template will simply use the std::char_traits for a given type, but the specializations will provide char_traits methods and traits for non-standard char types (unsigned and signed char).

Functions that relied on std::char_traits for these unsupported types have been updated to use the custom char_traits implementation.


Pull request checklist

Read the Contribution Guidelines for detailed information.

  • Changes are described in the pull request, or an existing issue is referenced.
  • The test suite compiles and runs without error.
  • Code coverage is 100%. Test cases can be added by editing the test suite.
  • The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header files single_include/nlohmann/json.hpp and single_include/nlohmann/json_fwd.hpp. The whole process is described here.

Please don't

  • The C++11 support varies between different compilers and versions. Please note the list of supported compilers. Some compilers like GCC 4.7 (and earlier), Clang 3.3 (and earlier), or Microsoft Visual Studio 13.0 and earlier are known not to work due to missing or incomplete C++11 support. Please refrain from proposing changes that work around these compiler's limitations with #ifdefs or other means.
  • Specifically, I am aware of compilation problems with Microsoft Visual Studio (there even is an issue label for this kind of bug). I understand that even in 2016, complete C++11 support isn't there yet. But please also understand that I do not want to drop features or uglify the code just to make Microsoft's sub-standard compiler happy. The past has shown that there are ways to express the functionality such that the code compiles with the most recent MSVC - unfortunately, this is not the main objective of the project.
  • Please refrain from proposing changes that would break JSON conformance. If you propose a conformant extension of JSON to be supported by the library, please motivate this extension.
  • Please do not open pull requests that address multiple issues.

@coveralls
Copy link

coveralls commented Oct 6, 2023

Coverage Status

coverage: 100.0%. remained the same
when pulling 543158b on colbychaskell:develop
into f56c6e2 on nlohmann:develop.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

🔴 Amalgamation check failed! 🔴

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

@github-actions
Copy link

github-actions bot commented Oct 7, 2023

🔴 Amalgamation check failed! 🔴

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

@colbychaskell
Copy link
Contributor Author

colbychaskell commented Oct 27, 2023

Pushed a new commit that handles this a lot more cleanly. Will fix check errors soon

@github-actions github-actions bot added L and removed M labels Nov 5, 2023
Copy link

github-actions bot commented Nov 5, 2023

🔴 Amalgamation check failed! 🔴

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

@colbychaskell
Copy link
Contributor Author

@nlohmann Sorry for the delay, this is ready to be reviewed when you get a chance!

@colbychaskell
Copy link
Contributor Author

@nlohmann it looks like something unrelated to these changes is breaking in the CI compiler tests as it's trying to get CMake and Ninja.

Would this be related to recent CI fixes? The checks passed successfully with the same changes about a week ago.

@nlohmann
Copy link
Owner

No, unfortunately this is unrelated. I am getting so tired of fixing CI issues rather than improving the library. I'll check during the weekend.

@colbychaskell
Copy link
Contributor Author

The failing checks are an unrelated CI issue that will be fixed in #4215

@github-actions github-actions bot removed the CI label Nov 25, 2023
@nlohmann
Copy link
Owner

The failing checks are an unrelated CI issue that will be fixed in #4215

#4215 is merged.

@nlohmann nlohmann added the please rebase Please rebase your branch to origin/develop label Nov 26, 2023
@nlohmann
Copy link
Owner

Please rebase to the develop branch. This should fix the CI. I would like to take this as last item for the upcoming release.

@nlohmann nlohmann added this to the Release 3.11.3 milestone Nov 26, 2023
@nlohmann nlohmann linked an issue Nov 26, 2023 that may be closed by this pull request
2 tasks
@colbychaskell
Copy link
Contributor Author

@nlohmann rebased and good to go now!

@nlohmann nlohmann removed the please rebase Please rebase your branch to origin/develop label Nov 27, 2023
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann merged commit 1d59774 into nlohmann:develop Nov 27, 2023
111 checks passed
@nlohmann
Copy link
Owner

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecation warning about std::char_traits<unsigned char>
4 participants