From c39ea7965f48061916ab47a68a29b1966ffebb0d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?=
 <mael.rouxel.labbe@geometryfactory.com>
Date: Wed, 10 Nov 2021 15:43:17 +0100
Subject: [PATCH] Do not cast fundamental types into bool in NT_converter

Instead, try to construct a NT1 from them. If it fails, give a compilation error.

This is to avoid converters silently doing nasty things like
Cartesian_converter<SC<CORE::Expr>, SC<double> >(2.) ==> 1 (bool true)
because there is no implicit conversion from double to SC<CORE::Expr>, so it
preferes using Enum_converter(bool) (with a double -> Boolean implicit
conversion...).
---
 .../include/CGAL/Cartesian_converter.h        | 17 ++++++++++--
 .../include/CGAL/Homogeneous_converter.h      | 27 +++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/Cartesian_kernel/include/CGAL/Cartesian_converter.h b/Cartesian_kernel/include/CGAL/Cartesian_converter.h
index d1a22c57160..096e0b4831b 100644
--- a/Cartesian_kernel/include/CGAL/Cartesian_converter.h
+++ b/Cartesian_kernel/include/CGAL/Cartesian_converter.h
@@ -39,6 +39,8 @@
 #include <boost/mpl/logical.hpp>
 #include <boost/mpl/remove.hpp>
 
+#include <type_traits>
+
 namespace CGAL {
 
 // Guess which compiler needs this work around ?
@@ -82,8 +84,7 @@ struct Converting_visitor : boost::static_visitor<> {
 } // namespace internal
 
 template < class K1, class K2,
-//          class Converter = NT_converter<typename K1::FT, typename K2::FT> >
-           class Converter>
+           class Converter /*= typename internal::Default_converter<K1, K2>::Type*/>
 class Cartesian_converter : public Enum_converter
 {
     typedef Enum_converter   Base;
@@ -125,6 +126,18 @@ public:
         return c(a);
     }
 
+    // This intentionally does not check that K1::FT is constructible from T, because otherwise
+    // the function `bool Enum_converter::operator()(bool)` might be called instead, with an implicit
+    // conversion from the fundamental type to bool, which is usually unintended.
+    template <typename T>
+    typename K2::FT
+    operator()(const T& t,
+               typename std::enable_if<std::is_fundamental<T>::value>::type* = nullptr,
+               typename std::enable_if<!std::is_same<T, typename K1::FT>::value>::type* = nullptr)
+    {
+        return c(typename K1::FT(t));
+    }
+
     // drop the boost::detail::variant::void_ generated by the macros
     // from the sequence, transform with the type mapper and throw the
     // new list into a variant
diff --git a/Homogeneous_kernel/include/CGAL/Homogeneous_converter.h b/Homogeneous_kernel/include/CGAL/Homogeneous_converter.h
index d4c3e6a1dd9..25391d6097a 100644
--- a/Homogeneous_kernel/include/CGAL/Homogeneous_converter.h
+++ b/Homogeneous_kernel/include/CGAL/Homogeneous_converter.h
@@ -30,6 +30,8 @@
 #include <CGAL/Bbox_3.h>
 #include <CGAL/Origin.h>
 
+#include <type_traits>
+
 namespace CGAL {
 
 template <class K1, class K2,
@@ -84,6 +86,31 @@ public:
         return fc(a);
     }
 
+    // This intentionally does not check that K1::RT is constructible from T, because otherwise
+    // the function `bool Enum_converter::operator()(bool)` might be called instead, with an implicit
+    // conversion from the fundamental type to bool, which is usually unintended.
+    template <typename T>
+    typename K2::RT
+    operator()(const T& t,
+               typename std::enable_if<std::is_fundamental<T>::value>::type* = nullptr,
+               typename std::enable_if<std::is_constructible<typename K1::RT, T>::value>::type* = nullptr,
+               typename std::enable_if<!std::is_same<T, typename K1::RT>::value>::type* = nullptr,
+               typename std::enable_if<!std::is_same<T, typename K1::FT>::value>::type* = nullptr)
+    {
+        return rc(typename K1::RT(t));
+    }
+
+    template <typename T>
+    typename K2::FT
+    operator()(const T& t,
+               typename std::enable_if<std::is_fundamental<T>::value>::type* = nullptr,
+               typename std::enable_if<!std::is_constructible<typename K1::RT, T>::value>::type* = nullptr,
+               typename std::enable_if<!std::is_same<T, typename K1::FT>::value>::type* = nullptr)
+    {
+        return fc(typename K1::FT(t));
+    }
+
+
     typename K2::Point_2
     operator()(const typename K1::Point_2 &a) const
     {