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 C++20/gcc-12 issues (Part 1) #3379

Merged

Conversation

falbrechtskirchinger
Copy link
Contributor

@falbrechtskirchinger falbrechtskirchinger commented Mar 6, 2022

This is the first small batch of fixes to address build failures in C++20 mode and/or using gcc-12.

It incorporates 3 commits from @nlohmann, although cc01459 should arguably not be part of this (should I remove that commit?).
The spaceship operator is needed to fully address #3138 and #3207 (Part 2) and CI will not pass all checks.

Fixes #3138.
Closes #3226.

NOTE: Based on my update-ci branch. The (currently) 3 bottom commits are not part of this PR. Once #3368 is merged I can rebase and remove draft status.

@coveralls
Copy link

coveralls commented Mar 6, 2022

Coverage Status

Coverage remained the same at 100.0% when pulling 5c51ac7 on falbrechtskirchinger:topic/fix-c++20-issues-part1 into 4a6e6ca on nlohmann:develop.

@falbrechtskirchinger
Copy link
Contributor Author

falbrechtskirchinger commented Mar 6, 2022

The remaining CI failure is the one observed here: #3138 (comment)
To be fixed in part 2 (I don't have a 100% working version yet and have refocused on improved unit testing to aid debugging).

The changes here are needed for PR #3380 not to reveal even more broken tests.

Do you want me to disable the failing test temporarily? As selectively as possible.
I've disabled the failing comparisons.

@falbrechtskirchinger falbrechtskirchinger force-pushed the topic/fix-c++20-issues-part1 branch 2 times, most recently from 738a5c1 to 3ab32b7 Compare March 6, 2022 16:19
nlohmann and others added 5 commits March 6, 2022 23:55
Allocator tests fail to compile in C++20 mode with clang+MS STL due
to missing copy constructors.
* alt_string has multiple member functions that should be marked noexcept.
* nlohmann::ordered_map constructors can be noexcept.

Compilation failures result from the warning flag -Werror=noexcept and
gcc-12.
@falbrechtskirchinger falbrechtskirchinger marked this pull request as ready for review March 6, 2022 23:39
@nlohmann
Copy link
Owner

nlohmann commented Mar 7, 2022

Thanks for incorporating #3226, and please leave cc01459 in - it's part of the GCC update.

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 self-assigned this Mar 7, 2022
@nlohmann nlohmann added this to the Release 3.10.6 milestone Mar 7, 2022
@nlohmann
Copy link
Owner

nlohmann commented Mar 7, 2022

Will merge when the CI is done.

@falbrechtskirchinger
Copy link
Contributor Author

Will merge when the CI is done.

Great. Feel free to cancel all my unrelated, pending AppVeyor CI runs.

@nlohmann
Copy link
Owner

nlohmann commented Mar 7, 2022

Good point - it's a pity AppVeyor is so slow...

@falbrechtskirchinger Thanks so much for your recent contributions!

@falbrechtskirchinger
Copy link
Contributor Author

@falbrechtskirchinger Thanks so much for your recent contributions!

@nlohmann You're very welcome! There are more on the way, so I'd like to thank you for reviewing them as juggling all these items is becoming mentally taxing.

@nlohmann nlohmann merged commit f208a9c into nlohmann:develop Mar 7, 2022
@falbrechtskirchinger falbrechtskirchinger deleted the topic/fix-c++20-issues-part1 branch March 7, 2022 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants