-
Notifications
You must be signed in to change notification settings - Fork 216
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 BOOST_GEOMETRY_CONSTEXPR call in index #1265
Conversation
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.
lgtm
@@ -328,11 +328,11 @@ class insert | |||
// Enlarge it in case if it's not bounding geometry type. | |||
// It's because Points and Segments are compared WRT machine epsilon | |||
// This ensures that leafs bounds correspond to the stored elements | |||
if BOOST_GEOMETRY_CONSTEXPR (std::is_same<Element, value_type>::value | |||
if (BOOST_GEOMETRY_CONSTEXPR ((std::is_same<Element, value_type>::value |
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.
Have you tested it with C++17?
My guess is that this would not compile because it'd expand to
if (constepxr ((...)))
The problem here is the comma. I think you only have to put std::is_same<Element, value_type>::value
in parentheses.
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 are right, the problem is the first parenthesis, either putting std::is_same<Element, value_type>::value
or the whole expression in parantheses is working, but I will follow your suggestion because it is simpler and focuses on the problem which is the comma. I will open a new PR.
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.
Do we want to add testing with C++17 on CI?
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.
Yes, I think it's a good idea. We have optional C++17-dependent code: inline variables, std::any
adaptation, std::variant
adaptation, if constexpr
.
This PR fixes the compilation error
error: macro "BOOST_GEOMETRY_CONSTEXPR" passed 2 arguments, but takes just 1
introduced by cb2f53d