Skip to content
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 compilation with C++11 and above #10

Merged
merged 3 commits into from
Nov 3, 2016
Merged

Fix compilation with C++11 and above #10

merged 3 commits into from
Nov 3, 2016

Conversation

flannelhead
Copy link
Contributor

@flannelhead flannelhead commented Nov 2, 2016

There were several problems in the C++11 and above specific code that prevented compilation with gcc >= 6.1 where C++14 is used as the default standard instead of C++98 (previous gcc versions).

In ExPolygon.cpp and Surface.hpp there are trivial fixes to typos in the C++11 specific code.

In PolylineCollection there was an ambiguity between overloaded functions with Polylines &&src and Polylines src arguments. In such a situation the compiler can't resolve between those two functions.

My attempt at a solution is to make a new private _chained_path_from functions which allows explicitly specifying whether the Polyline objects from the const Polylines &src vector can be moved. In the public interface, there are overloads of chained_path and chained_path_from functions with const lvalue reference (const Polylines &src) and rvalue reference (Polylines &&src) parameters. These call _chained_path_from respectively asking either to move or not to move from src. A user of the PolylineCollection can trigger the move variant by passing an rvalue reference (using std::move).

This solution might need some reviewing as I'm still not entirely comfortable with move semantics. In particular, I'm not sure if there could be some other way to trigger the std::move in _chained_path_from without specifying it explicitly (i.e. based on whether the input value is const Polylines & or Polylines &&). Of course one could just do two overloaded variants of chained_path_from, but there would be a difference of only one line and that sounds like unnecessary duplication.

@flannelhead
Copy link
Contributor Author

While I was at it, I also fixed auto_ptr deprecation warnings.

@bubnikv
Copy link
Collaborator

bubnikv commented Nov 3, 2016

Thanks, I am looking into it.
Interestingly, the move semantics in my original form compiles on the Visual Studio 2013.

Copy link
Collaborator

@bubnikv bubnikv left a comment

Choose a reason for hiding this comment

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

great, thanks

@bubnikv bubnikv merged commit f278fa4 into prusa3d:master Nov 3, 2016
@flannelhead
Copy link
Contributor Author

@bubnikv good.

Strange indeed that it compiled on VS2013. The compiler there might have different logic for overload resolution. If you happen to figure out a better way to resolve this ambiguity, please enlighten me too :)

@flannelhead flannelhead mentioned this pull request Nov 3, 2016
@bubnikv bubnikv mentioned this pull request May 20, 2019
@lukasmatena lukasmatena mentioned this pull request Aug 6, 2019
@zaitcev zaitcev mentioned this pull request Nov 5, 2021
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.

2 participants