constexpr in CGAL kernel

[lrineau]

Issue Details

I think all constructors of CGAL kernel objects can be constexpr, and probably also CGAL functors operator().

That should be a goal of the CGAL project to use that possibility.

@mglisse, do you see objections to that? I am not a specialist. I see that, for templates, that is valid if specializations of constexpr function templates cannot actually appear in constant expressions.

[lrineau]

For example, the following patch

diff --git a/Algebraic_foundations/include/CGAL/Algebraic_structure_traits.h b/Algebraic_foundations/include/CGAL/Algebraic_structure_traits.h
index 5c3b6f0ee2e..1b364bbeec3 100644
--- a/Algebraic_foundations/include/CGAL/Algebraic_structure_traits.h
+++ b/Algebraic_foundations/include/CGAL/Algebraic_structure_traits.h
@@ -153,7 +153,7 @@ class Algebraic_structure_traits_base< Type_,
     class Square
       : public CGAL::cpp98::unary_function< Type, Type > {
       public:
-        Type operator()( const Type& x ) const {
+        constexpr Type operator()( const Type& x ) const {
           return x*x;
         }
     };
diff --git a/Algebraic_foundations/include/CGAL/number_utils.h b/Algebraic_foundations/include/CGAL/number_utils.h
index abb7894ccf4..8709c1aff0a 100644
--- a/Algebraic_foundations/include/CGAL/number_utils.h
+++ b/Algebraic_foundations/include/CGAL/number_utils.h
@@ -64,7 +64,7 @@ is_square( const AS& x){

 template< class AS >
 inline
-typename Algebraic_structure_traits< AS >::Square::result_type
+constexpr typename Algebraic_structure_traits< AS >::Square::result_type
 square( const AS& x ) {
     typename Algebraic_structure_traits< AS >::Square square;
     return square( x );

allows to write:

  constexpr double dist = 0.002;
  constexpr auto sq_dist = CGAL::square(dist);

The goal would be to be able to declare kernel objects as constexpr:

  constexpr double dist = 0.002;
  constexpr auto sq_dist = CGAL::square(dist);
  constexpr Sphere sphere{p, sq_dist};

[mglisse]

I don't think it is going to revolutionize the way we write code, so I wouldn't give it a high priority, but I also don't think it hurts. IIRC there were some versions of C++ where constexpr changed a bit how eagerly templates got instantiated (or something related) which might cause some issues with the fragile mutual dependencies in our kernels. I wouldn't try to add it everywhere at once, starting from some number-type stuff as in your comment makes sense. Maybe check that compilation time&memory doesn't grow too much (some compilers handle constexpr functions very differently) and look at the binary size of some random cgal example to see if it decreases slightly.

[lrineau]

I agree this should not have a high priority. I wrote that issue hoping that someone (maybe an external user) would like to pick it up and work on it.

I had not thought about the compilation time/resources effect. And the size of binaries. Good points. Thanks.

[lrineau]

IIRC there were some versions of C++ where constexpr changed a bit how eagerly templates got instantiated (or something related) which might cause some issues with the fragile mutual dependencies in our kernels.

Actually, about that point: I was influenced by that blog postconstexpr is a platform. There are two goals in making CGAL kernel code constexpr:

1. The first one is what I stated: being able to declare constexpr kernel objects.
2. The second one is to make our code more portable, or more bullet-proof: if we can test that our constexpr code can work with all the supported compilers, then it will mean it will be tested not only by the compilers, but also by the constexpr-interpreters of the same compilers.

[Koushikk-24]

hi , I'm interested in contributing to this project can you suggest me ways to get started?

[sotpapathe]

If I'm not mistaken CGAL redefines the operators for C++ data types like float and double so that the following code no longer compiles when linking against CGAL:

constexpr float deg_to_rad = M_PI / 180.0f;

GCC 9.3.0 using the --std=c++17 flag produces the following error:

error: ‘(3.1415926535897931e+0 / 1.8e+2)’ is not a constant expression

This code compiles fine without linking CGAL.

So I would argue this is more of an issue to be fixed than a new feature.

[lrineau]

If I'm not mistaken CGAL redefines the operators for C++ data types like float and double so that the following code no longer compiles when linking against CGAL:

constexpr float deg_to_rad = M_PI / 180.0f;

Off-topic. I redirect to a new issue https://github.com/CGAL/cgal/issues/5862.

[Khalaq04]

Hi, I am interested in contributing to this project and would like to work on this issue. Can you please guide me on how to and from where to start working on this project and this issue? Thank you.