-
Notifications
You must be signed in to change notification settings - Fork 306
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
Port to C++20. #106
base: master
Are you sure you want to change the base?
Port to C++20. #106
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
What's the minimum c++ version needed for this to work? |
|
This isn't compiling: https://travis-ci.org/github/google/s2geometry/builds/696839883 C++20 is too new to require. C++14 is definitely ok and C++17 is probably ok. |
gcc 10 works without this patch. |
src/s2/base/logging.h
Outdated
@@ -91,7 +91,7 @@ class S2FatalLogMessage : public S2LogMessage { | |||
absl::LogSeverity severity, std::ostream& stream) | |||
ABSL_ATTRIBUTE_COLD | |||
: S2LogMessage(file, line, severity, stream) {} | |||
ABSL_ATTRIBUTE_NORETURN ~S2FatalLogMessage() { abort(); } | |||
ABSL_ATTRIBUTE_NORETURN ~S2FatalLogMessage() { throw std::runtime_error("s2geometry fatal error"); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please separate out changes unrelated to c++17-deprecated interfaces.
@@ -272,8 +272,7 @@ class S2ShapeIndex { | |||
// for (S2Shape* shape : index) { ... } | |||
// | |||
// CAVEAT: Returns nullptr for shapes that have been removed from the index. | |||
class ShapeIterator | |||
: public std::iterator<std::forward_iterator_tag, S2Shape*> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This provided typedefs. Where do they come from now?
@@ -13,7 +13,7 @@ if (APPLE) | |||
set(CMAKE_MACOSX_RPATH TRUE) | |||
endif() | |||
|
|||
set(CMAKE_CXX_STANDARD 11) | |||
set(CMAKE_CXX_STANDARD 20) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can bump this to 17 if it's really needed to compile any of the replacements.
I will close and re-open this in hope of getting travis-ci to notice the |
This breaks both compilers that support c++20 and those that don't |
This patch replaces various constructs that were deprecated in C++17 and removed in C++20 with their C++20 counterparts.
With this patch, s2geometry can be compiled with gcc 10.