CGAL
Move enable CGAL types
When working with CGAL in C++11 and upwards one of the biggest issues is that most types cannot be efficiently moved even if they should. Users end up writing code that works around that instead of using the straight forward approach. This primarily affects types which carry large geometric structures.
A non-exhaustive list:
- [x] Compact_container and Concurrent_compact_container (PR #4496)
- [ ] Surface_mesh
- [ ] HDS and Polyhedron_3: movability depends on underlying storage
- [x] Triangulation_data_structure_2 and Triangulation_2: depends on Compact_container (PR #4496)
- [x] Triangulation_data_structure and Triangulation: depends on Compact_container (PR #4496)
I'm not volunteering my self, but could you give a specific example.
/_____/) o /_________ __ // (____ ( ( ( (/ (/-(-'_(/ _/
On Wed, Apr 8, 2015 at 2:55 PM, Laurent Rineau [email protected] wrote:
Who could spend time on that task, among the @CGAL/developers https://github.com/orgs/CGAL/teams/developers ?
— Reply to this email directly or view it on GitHub https://github.com/CGAL/cgal/issues/34#issuecomment-90890710.
@efifogel Polygon_2 is a very simple example: it is templated by the container. All possible containers support move construction. For some unknown reason a dubious copy-constructor is provided, which prevents generation of a move constructor etc. Fixing this would make it possible to cheaply pass a Polygon_2 by value.
Everybody should know something about the C++ Rule of 3 (in C++98), that became Rule of 5 or Rule of 0, in C++11:
- Here is a simplified reference, in Wikipedia: Rule of three (C++ programming.
- For better examples, but a longer explanation: http://en.cppreference.com/w/cpp/language/rule_of_three
- if you want more, the Wikipedia page has the useful reference.
I personally prefer the Rule of 0: do not deal with resources yourself, but we could apply the Rule of 5 for our own containers.
One problem is that we need to identify classes with unnecessary constructors and sometimes even destructors. They didn't do much harm in C++03 but are actively harmful in C++11.
I guess one starting point is to raise awareness and fix it as we go along.
Le Friday 10 April 2015 05:34:23 Philipp Moeller a écrit :
One problem is that we need to identify classes with unnecessary constructors and sometimes even destructors. They didn't do much harm in C++03 but are actively harmful in C++11.
I guess one starting point is to raise awareness and fix it as we go along.
Could there be a way to modify the test suite to check the extra copies?
(My policy in CGAL is that anything that is not tested can be considered as broken, or having the potential to break after a while.)
Laurent Rineau, PhD R&D Engineer at GeometryFactory http://www.geometryfactory.com/ Release Manager of the CGAL Project http://www.cgal.org/
It is hard to test for MoveConstructible, since it will fall-back on
CopyConstructible. This means that std::is_move_constructible and the
related type traits will only tell you if construction from an rvalue is
OK, but not if it is efficient.
One way can be a hand written test like:
X x; // fill x with stuff
X y = std::move(x);
// now check if x is empty or something
This only works if you know something about X and its invariants when
being moved from, so you can only do this on a case-by-case basis.
Yes, case by case seems the only way to test (I wouldn't bother). is_nothrow_move_constructible is often the easiest test (since very often copy constructors throw and move constructors don't).
If I understand correctly, the conclusion is to "raise awareness and fix it as we go along". Does everyone agree on this?
If so, I suggest we add a paragraph about this somewhere in the developer manual (with the links that @lrineau provided about the rules of 5 and 0) and close the issue. It seems unrealistic to review all classes of CGAL at once, so we should just expect new issues about this problem and fix them when they are raised.
Let's say that I will raise the topic at next developers meeting. I can probably present the topic, problems, and solutions with a few slides.
The very recent issue #2925 asks for move-semantic in AABB_tree. C++11 more that 10 years old now (even before 2011 it was available under the name C++0x), and CGAL should do something!
HDS and Polyhedron_3: movability depends on underlying storage
let's not do it.