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

Avoid passing temporaries as function parameters when an L-value reference is expected #518

Merged
merged 1 commit into from Oct 19, 2018

Conversation

Projects
None yet
3 participants
@xventura81
Copy link

xventura81 commented Oct 18, 2018

Solve problem described in #517

@awulkiew

This comment has been minimized.

Copy link
Member

awulkiew commented Oct 18, 2018

Boost.Geometry supports C++03 so your proposal has to do it as well.

append_point<ring_type, Point>::apply(
ext_ring, point);
ext_ring, point);

This comment has been minimized.

@mloskot

mloskot Oct 18, 2018

Member

@xventura81 Has your editor changed spaces vs tabs? Not sure what happened here.

This comment has been minimized.

@xventura81

xventura81 Oct 18, 2018

Yeah, Visual Studio does it by default.

This comment has been minimized.

@mloskot

mloskot Oct 18, 2018

Member

IMHO, that is not good to let accidental style changes happen. Every editor can be controlled to prevent it from replacing spaces with tabs or auto-reformatting. Imagine N authors working on common code base use N editors and each editor has different auto-formatting style as default... :-)

This comment has been minimized.

@xventura81

xventura81 Oct 18, 2018

You are right. Let's revert those changes in the indentation.


static inline void apply(Polygon& polygon, Point const& point,
int ring_index, int = 0)
{
if (ring_index == -1)
{
ring_type& ext_ring = exterior_ring(polygon);

This comment has been minimized.

@awulkiew

awulkiew Oct 18, 2018

Member

exterior_ring() may return not-real reference so you should use ring_return_type<Polygon>::type here, similar to interior_return_type<Polygon>::type you used. Otherwise you may end up with dangling reference,

This comment has been minimized.

@xventura81

xventura81 Oct 19, 2018

Thank you for this observation.


static inline void apply(Polygon& polygon, Range const& range,
int ring_index, int = 0)
{
if (ring_index == -1)
{
ring_type& ext_ring = exterior_ring(polygon);

This comment has been minimized.

@awulkiew

awulkiew Oct 18, 2018

Member

ring_return_type<Polygon>::type

@awulkiew

This comment has been minimized.

Copy link
Member

awulkiew commented Oct 18, 2018

I made some comments but they are outdated now. Please check them out. I won't create new ones to avoid spamming.

@awulkiew

This comment has been minimized.

Copy link
Member

awulkiew commented Oct 19, 2018

Thanks! Now I'd only suggest to squash all of the commits into one. What do you think?

Something like:

git reset --soft HEAD~4
git commit
git push -f

@xventura81 xventura81 force-pushed the xventura81:bugfix/append branch from 3b4c312 to 3b67dea Oct 19, 2018

@xventura81

This comment has been minimized.

Copy link

xventura81 commented Oct 19, 2018

Thanks! Now I'd only suggest to squash all of the commits into one. What do you think?

Something like:

git reset --soft HEAD~4
git commit
git push -f

Done! Thank for all the observations.

@awulkiew

This comment has been minimized.

Copy link
Member

awulkiew commented Oct 19, 2018

Great, thanks!

@awulkiew awulkiew merged commit 01b6a0e into boostorg:develop Oct 19, 2018

@awulkiew awulkiew added the bug label Oct 19, 2018

@awulkiew awulkiew added this to the 1.69 milestone Oct 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment