Skip to content

Miscellaneous warnings and errors which showed up in GCC 12 #257

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

Merged
merged 13 commits into from
May 22, 2022

Conversation

wravery
Copy link
Contributor

@wravery wravery commented May 22, 2022

Fix #255

At least, I think this will fix that issue, as originally reported. I have not been able to reproduce the original linker error, but by enabling LTO and testing with both GCC 10 and GCC 12 (after an Ubuntu upgrade), I flushed out a lot of ODR (one definition rule) warnings where template methods were specialized the same way in multiple translation units.

To fix the ODR warnings, and hopefully the original linker issue, I added inline specifiers to every non-constexpr inline method I could find in the headers. I also separated the template methods from the static methods in the Modified... template structs and put the template methods in an anonymous namespace. That makes their linkage private to each translation unit which instantiates them, so the linker cannot treat them as the same symbol (although it may still monomorphize them to remove duplicate code).

While I was tracking that down, I decided to simplify the SFINAE template logic in the Modified... template structs. I replaced/consolidated pretty much every reference to the std::is_same_v<> or std::is_base_of_v<> type_traits with concepts and and std::enable_if_t with requires clauses on the template methods. They're all much easier to read now.

Last of all, GCC 12 seems to have added some potential memory safety warnings which it evaluates at compile time. I found that it wanted me to pass keys (which are pretty much always std::string_view) by value rather than const reference in SortedMap.h. In a few tests where I copy strings into a byte vector to make a Base64 response::IdType, I needed to use std::string_view instead of std::string to avoid hitting the small-string optimization and confusing the compile-time bounds checks.

@wravery wravery merged commit 0bb0ac9 into microsoft:main May 22, 2022
@wravery wravery deleted the inline-methods branch May 22, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link error in release mode
1 participant