Skip to content

Fix/warnings #218

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 12 commits into from
Feb 11, 2015
Merged

Fix/warnings #218

merged 12 commits into from
Feb 11, 2015

Conversation

awulkiew
Copy link
Member

This PR fixes the MSVC compiler warning C4127: conditional expression is constant in all parts of the library.

For this purpose a macro BOOST_GEOMETRY_CONDITION was added. It should be used in cases when a conditional expression may be constant.
Note that the same expression may or may not be constant in various cases when template specialization is involved.

@awulkiew
Copy link
Member Author

This fixes the ticket: https://svn.boost.org/trac/boost/ticket/10666

//namespace boost { namespace geometry { namespace detail {
//BOOST_FORCEINLINE bool condition(bool const b) { return b; }
//}}} // boost::geometry::detail
//#define BOOST_GEOMETRY_CONDITION(CONDITION) boost::geometry::detail::condition(CONDITION)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these comments go away?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd prefer to keep it in case if some other or future version of MSVC didn't work as expected or started to generate a warning in additional cases, e.g. for the workaround below.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK - please add a comment then, in case anyone later thinks to remove old code

@@ -123,7 +125,7 @@ struct convex_hull<Box, box_tag>
boost::array<typename point_type<Box>::type, 4> range;
geometry::detail::assign_box_corners_oriented<Reverse>(box, range);
geometry::append(out, range);
if (Close)
if ( BOOST_GEOMETRY_CONDITION(Close) )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove the extra spacing you added - it is not according to our guidelines, and never used in this sourcefile (at least not in the original version).

@barendgehrels
Copy link
Collaborator

Thanks for the PR. See my comments but basically I'm OK having this in our library

@awulkiew
Copy link
Member Author

Ok, thanks for the review. I addressed all of the issues. I'm merging. In case of problems I'll act accordingly.

awulkiew added a commit that referenced this pull request Feb 11, 2015
@awulkiew awulkiew merged commit 91b80a9 into boostorg:develop Feb 11, 2015
@awulkiew awulkiew deleted the fix/warnings branch February 13, 2015 19:51
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.

3 participants