From 42427a56d03f282dd8d57ca1d97c4b99194e8f8d Mon Sep 17 00:00:00 2001
From: Stephane Del Pino <stephane.delpino44@gmail.com>
Date: Sat, 23 Jul 2022 16:34:21 +0200
Subject: [PATCH] Refactor slightly BuiltinFunctionEmbedder and check types

Now the arguments and return values types are checked at
construction. One checks if the C++ type corresponds to a pugs type.

Closes #29
---
 src/language/modules/BuiltinModule.hpp        |  16 +-
 .../utils/BuiltinFunctionEmbedder.hpp         | 298 +++++++++---------
 tests/test_BuiltinFunctionEmbedder.cpp        |  76 ++++-
 3 files changed, 236 insertions(+), 154 deletions(-)

diff --git a/src/language/modules/BuiltinModule.hpp b/src/language/modules/BuiltinModule.hpp
index ae36c3510..d2a1bcb85 100644
--- a/src/language/modules/BuiltinModule.hpp
+++ b/src/language/modules/BuiltinModule.hpp
@@ -4,6 +4,10 @@
 #include <language/modules/IModule.hpp>
 #include <language/utils/ASTNodeDataType.hpp>
 
+#include <utils/Exceptions.hpp>
+
+#include <sstream>
+
 class IBuiltinFunctionEmbedder;
 class TypeDescriptor;
 class ValueDescriptor;
@@ -22,8 +26,16 @@ class BuiltinModule : public IModule
   void
   _addBuiltinFunction(const std::string& name, std::function<FX(Args...)>&& f)
   {
-    this->_addBuiltinFunction(name, std::make_shared<BuiltinFunctionEmbedder<FX(Args...)>>(
-                                      std::forward<std::function<FX(Args...)>>(f)));
+    try {
+      this->_addBuiltinFunction(name, std::make_shared<BuiltinFunctionEmbedder<FX(Args...)>>(
+                                        std::forward<std::function<FX(Args...)>>(f)));
+    }
+    catch (std::invalid_argument& e) {
+      std::ostringstream os;
+      os << "while defining user function '" << rang::fgB::yellow << name << rang::fg::reset << "'\n";
+      os << e.what();
+      throw UnexpectedError(os.str());
+    }
   }
 
  private:
diff --git a/src/language/utils/BuiltinFunctionEmbedder.hpp b/src/language/utils/BuiltinFunctionEmbedder.hpp
index ac3ba21fe..c3f015c5b 100644
--- a/src/language/utils/BuiltinFunctionEmbedder.hpp
+++ b/src/language/utils/BuiltinFunctionEmbedder.hpp
@@ -33,6 +33,122 @@ class IBuiltinFunctionEmbedder
   virtual ~IBuiltinFunctionEmbedder() = default;
 };
 
+template <typename FX, typename... Args>
+class BuiltinFunctionEmbedderBase;
+
+template <typename FX, typename... Args>
+class BuiltinFunctionEmbedderBase<FX(Args...)> : public IBuiltinFunctionEmbedder
+{
+ protected:
+  template <size_t I>
+  PUGS_INLINE void constexpr _check_value() const
+  {
+    using ValueN_T = std::tuple_element_t<I, FX>;
+    if constexpr (std::is_lvalue_reference_v<ValueN_T>) {
+      static_assert(std::is_const_v<std::remove_reference_t<ValueN_T>>,
+                    "builtin function return values are non mutable use 'const' when passing references");
+    }
+
+    if constexpr (is_std_ptr_v<ValueN_T>) {
+      static_assert(std::is_const_v<typename ValueN_T::element_type>,
+                    "builtin function return values are non mutable. For instance use std::shared_ptr<const T>");
+    }
+
+    if (ast_node_data_type_from<std::remove_cv_t<std::remove_reference_t<ValueN_T>>> == ASTNodeDataType::undefined_t) {
+      throw std::invalid_argument(std::string{"cannot bind C++ to language.\nnote: return value number "} +
+                                  std::to_string(I + 1) + std::string{" has no associated language type: "} +
+                                  demangle<ValueN_T>());
+    }
+  }
+
+  template <size_t... I>
+  PUGS_INLINE void constexpr _check_tuple_value(std::index_sequence<I...>) const
+  {
+    (_check_value<I>(), ...);
+  }
+
+  PUGS_INLINE void constexpr _check_return_type()
+  {
+    if constexpr (is_std_tuple_v<FX>) {
+      constexpr size_t N  = std::tuple_size_v<FX>;
+      using IndexSequence = std::make_index_sequence<N>;
+      _check_tuple_value(IndexSequence{});
+    } else {
+      if (ast_node_data_type_from<std::remove_cv_t<std::remove_reference_t<FX>>> == ASTNodeDataType::undefined_t) {
+        throw std::invalid_argument(
+          std::string{"cannot bind C++ to language.\nnote: return value has no associated language type: "} +
+          demangle<FX>());
+      }
+    }
+  }
+
+  template <typename T>
+  PUGS_INLINE ASTNodeDataType
+  _getDataType() const
+  {
+    Assert(ast_node_data_type_from<T> != ASTNodeDataType::undefined_t);
+    return ast_node_data_type_from<T>;
+  }
+
+  template <typename TupleT, size_t I>
+  PUGS_INLINE ASTNodeDataType
+  _getOneElementDataType() const
+  {
+    using ArgN_T = std::decay_t<decltype(std::get<I>(TupleT{}))>;
+    return this->template _getDataType<ArgN_T>();
+  }
+
+  template <size_t... I>
+  PUGS_INLINE std::vector<std::shared_ptr<const ASTNodeDataType>>
+  _getCompoundDataTypes(std::index_sequence<I...>) const
+  {
+    std::vector<std::shared_ptr<const ASTNodeDataType>> compound_type_list;
+    (compound_type_list.push_back(std::make_shared<ASTNodeDataType>(this->_getOneElementDataType<FX, I>())), ...);
+    return compound_type_list;
+  }
+
+  template <typename T>
+  PUGS_INLINE std::shared_ptr<IDataHandler>
+  _createHandler(std::shared_ptr<T> data) const
+  {
+    return std::make_shared<DataHandler<T>>(data);
+  }
+
+  template <typename ResultT>
+  PUGS_INLINE DataVariant
+  _resultToDataVariant(ResultT&& result) const
+  {
+    if constexpr (is_data_variant_v<std::decay_t<ResultT>>) {
+      return std::move(result);
+    } else {
+      return EmbeddedData(_createHandler(std::move(result)));
+    }
+  }
+
+ public:
+  PUGS_INLINE ASTNodeDataType
+  getReturnDataType() const final
+  {
+    if constexpr (is_std_tuple_v<FX>) {
+      constexpr size_t N  = std::tuple_size_v<FX>;
+      using IndexSequence = std::make_index_sequence<N>;
+      return ASTNodeDataType::build<ASTNodeDataType::list_t>(this->_getCompoundDataTypes(IndexSequence{}));
+    } else {
+      return this->_getDataType<FX>();
+    }
+  }
+
+  BuiltinFunctionEmbedderBase()
+  {
+    this->_check_return_type();
+  }
+
+  BuiltinFunctionEmbedderBase(const BuiltinFunctionEmbedderBase&) = delete;
+  BuiltinFunctionEmbedderBase(BuiltinFunctionEmbedderBase&&)      = delete;
+
+  virtual ~BuiltinFunctionEmbedderBase() = default;
+};
+
 template <typename FX, typename... Args>
 class BuiltinFunctionEmbedder
 {
@@ -45,12 +161,39 @@ inline constexpr bool is_const_ref_or_non_ref = (std::is_const_v<T> and std::is_
                                                 (not std::is_lvalue_reference_v<T>);
 
 template <typename FX, typename... Args>
-class BuiltinFunctionEmbedder<FX(Args...)> : public IBuiltinFunctionEmbedder
+class BuiltinFunctionEmbedder<FX(Args...)> : public BuiltinFunctionEmbedderBase<FX(Args...)>
 {
  private:
   std::function<FX(Args...)> m_f;
   using ArgsTuple = std::tuple<std::decay_t<Args>...>;
 
+  template <size_t I>
+  PUGS_INLINE void constexpr _check_arg() const
+  {
+    using ArgN_T = std::tuple_element_t<I, std::tuple<Args...>>;
+    if constexpr (std::is_lvalue_reference_v<ArgN_T>) {
+      static_assert(std::is_const_v<std::remove_reference_t<ArgN_T>>,
+                    "builtin function arguments are non mutable use 'const' when passing references");
+    }
+
+    if constexpr (is_std_ptr_v<ArgN_T>) {
+      static_assert(std::is_const_v<typename ArgN_T::element_type>,
+                    "builtin function arguments are non mutable. For instance use std::shared_ptr<const T>");
+    }
+
+    if (ast_node_data_type_from<std::remove_cv_t<std::remove_reference_t<ArgN_T>>> == ASTNodeDataType::undefined_t) {
+      throw std::invalid_argument(std::string{"cannot bind C++ to language.\nnote: argument number "} +
+                                  std::to_string(I + 1) + std::string{" has no associated language type: "} +
+                                  demangle<ArgN_T>());
+    }
+  }
+
+  template <size_t... I>
+  PUGS_INLINE void constexpr _check_arg_list(std::index_sequence<I...>) const
+  {
+    (_check_arg<I>(), ...);
+  }
+
   template <size_t I>
   PUGS_INLINE void
   _copyValue(ArgsTuple& t, const std::vector<DataVariant>& v) const
@@ -122,77 +265,15 @@ class BuiltinFunctionEmbedder<FX(Args...)> : public IBuiltinFunctionEmbedder
     (_copyValue<I>(t, v), ...);
   }
 
-  template <typename T>
-  PUGS_INLINE ASTNodeDataType
-  _getDataType() const
-  {
-    Assert(ast_node_data_type_from<T> != ASTNodeDataType::undefined_t);
-    return ast_node_data_type_from<T>;
-  }
-
-  template <typename TupleT, size_t I>
-  PUGS_INLINE ASTNodeDataType
-  _getOneParameterDataType() const
-  {
-    using ArgN_T = std::decay_t<decltype(std::get<I>(TupleT{}))>;
-    return _getDataType<ArgN_T>();
-  }
-
   template <size_t... I>
-  PUGS_INLINE std::vector<ASTNodeDataType> _getParameterDataTypes(std::index_sequence<I...>) const
+  PUGS_INLINE std::vector<ASTNodeDataType>
+  _getParameterDataTypes(std::index_sequence<I...>) const
   {
     std::vector<ASTNodeDataType> parameter_type_list;
-    (parameter_type_list.push_back(this->_getOneParameterDataType<ArgsTuple, I>()), ...);
+    (parameter_type_list.push_back(this->template _getOneElementDataType<ArgsTuple, I>()), ...);
     return parameter_type_list;
   }
 
-  template <size_t... I>
-  PUGS_INLINE std::vector<std::shared_ptr<const ASTNodeDataType>> _getCompoundDataTypes(std::index_sequence<I...>) const
-  {
-    std::vector<std::shared_ptr<const ASTNodeDataType>> compound_type_list;
-    (compound_type_list.push_back(std::make_shared<ASTNodeDataType>(this->_getOneParameterDataType<FX, I>())), ...);
-    return compound_type_list;
-  }
-
-  template <typename T>
-  PUGS_INLINE std::shared_ptr<IDataHandler>
-  _createHandler(std::shared_ptr<T> data) const
-  {
-    return std::make_shared<DataHandler<T>>(data);
-  }
-
-  template <size_t I>
-  PUGS_INLINE void constexpr _check_arg() const
-  {
-    using ArgN_T = std::tuple_element_t<I, std::tuple<Args...>>;
-    if constexpr (std::is_lvalue_reference_v<ArgN_T>) {
-      static_assert(std::is_const_v<std::remove_reference_t<ArgN_T>>,
-                    "builtin function arguments are non mutable use 'const' when passing references");
-    }
-
-    if constexpr (is_std_ptr_v<ArgN_T>) {
-      static_assert(std::is_const_v<typename ArgN_T::element_type>,
-                    "builtin function arguments are non mutable. For instance use std::shared_ptr<const T>");
-    }
-  }
-
-  template <size_t... I>
-  PUGS_INLINE void constexpr _check_arg_list(std::index_sequence<I...>) const
-  {
-    (_check_arg<I>(), ...);
-  }
-
-  template <typename ResultT>
-  PUGS_INLINE DataVariant
-  _resultToDataVariant(ResultT&& result) const
-  {
-    if constexpr (is_data_variant_v<std::decay_t<ResultT>>) {
-      return std::move(result);
-    } else {
-      return EmbeddedData(_createHandler(std::move(result)));
-    }
-  }
-
   PUGS_INLINE
   AggregateDataVariant
   _applyToAggregate(const ArgsTuple& t) const
@@ -201,26 +282,14 @@ class BuiltinFunctionEmbedder<FX(Args...)> : public IBuiltinFunctionEmbedder
     std::vector<DataVariant> vector_result;
     vector_result.reserve(std::tuple_size_v<decltype(tuple_result)>);
 
-    std::apply([&](auto&&... result) { ((vector_result.emplace_back(_resultToDataVariant(result))), ...); },
-               tuple_result);
+    std::
+      apply([&](auto&&... result) { ((vector_result.emplace_back(this->template _resultToDataVariant(result))), ...); },
+            tuple_result);
 
     return AggregateDataVariant{std::move(vector_result)};
   }
 
  public:
-  PUGS_INLINE ASTNodeDataType
-  getReturnDataType() const final
-  {
-    if constexpr (is_std_tuple_v<FX>) {
-      constexpr size_t N  = std::tuple_size_v<FX>;
-      using IndexSequence = std::make_index_sequence<N>;
-
-      return ASTNodeDataType::build<ASTNodeDataType::list_t>(this->_getCompoundDataTypes(IndexSequence{}));
-    } else {
-      return this->_getDataType<FX>();
-    }
-  }
-
   PUGS_INLINE std::vector<ASTNodeDataType>
   getParameterDataTypes() const final
   {
@@ -253,7 +322,7 @@ class BuiltinFunctionEmbedder<FX(Args...)> : public IBuiltinFunctionEmbedder
       std::apply(m_f, t);
       return {};
     } else {
-      return EmbeddedData(_createHandler(std::apply(m_f, t)));
+      return EmbeddedData(this->template _createHandler(std::apply(m_f, t)));
     }
   }
 
@@ -261,8 +330,6 @@ class BuiltinFunctionEmbedder<FX(Args...)> : public IBuiltinFunctionEmbedder
   {
     using IndexSequence = std::make_index_sequence<std::tuple_size_v<ArgsTuple>>;
     this->_check_arg_list(IndexSequence{});
-
-    static_assert((std::is_same_v<Args, Args> and ...));
   }
 
   BuiltinFunctionEmbedder(const BuiltinFunctionEmbedder&) = delete;
@@ -279,53 +346,11 @@ class BuiltinFunctionEmbedder<FX, void>
 };
 
 template <typename FX>
-class BuiltinFunctionEmbedder<FX(void)> : public IBuiltinFunctionEmbedder
+class BuiltinFunctionEmbedder<FX(void)> : public BuiltinFunctionEmbedderBase<FX(void)>
 {
  private:
   std::function<FX(void)> m_f;
 
-  template <typename TupleT, size_t I>
-  PUGS_INLINE ASTNodeDataType
-  _getOneParameterDataType() const
-  {
-    using ArgN_T = std::decay_t<decltype(std::get<I>(TupleT{}))>;
-    return _getDataType<ArgN_T>();
-  }
-
-  template <size_t... I>
-  PUGS_INLINE std::vector<std::shared_ptr<const ASTNodeDataType>> _getCompoundDataTypes(std::index_sequence<I...>) const
-  {
-    std::vector<std::shared_ptr<const ASTNodeDataType>> compound_type_list;
-    (compound_type_list.push_back(std::make_shared<ASTNodeDataType>(this->_getOneParameterDataType<FX, I>())), ...);
-    return compound_type_list;
-  }
-
-  template <typename T>
-  PUGS_INLINE std::shared_ptr<IDataHandler>
-  _createHandler(std::shared_ptr<T> data) const
-  {
-    return std::make_shared<DataHandler<T>>(data);
-  }
-
-  template <typename T>
-  PUGS_INLINE ASTNodeDataType
-  _getDataType() const
-  {
-    Assert(ast_node_data_type_from<T> != ASTNodeDataType::undefined_t);
-    return ast_node_data_type_from<T>;
-  }
-
-  template <typename ResultT>
-  PUGS_INLINE DataVariant
-  _resultToDataVariant(ResultT&& result) const
-  {
-    if constexpr (is_data_variant_v<std::decay_t<ResultT>>) {
-      return std::move(result);
-    } else {
-      return EmbeddedData(_createHandler(std::move(result)));
-    }
-  }
-
   PUGS_INLINE
   AggregateDataVariant
   _applyToAggregate() const
@@ -334,25 +359,14 @@ class BuiltinFunctionEmbedder<FX(void)> : public IBuiltinFunctionEmbedder
     std::vector<DataVariant> vector_result;
     vector_result.reserve(std::tuple_size_v<decltype(tuple_result)>);
 
-    std::apply([&](auto&&... result) { ((vector_result.emplace_back(_resultToDataVariant(result))), ...); },
-               tuple_result);
+    std::
+      apply([&](auto&&... result) { ((vector_result.emplace_back(this->template _resultToDataVariant(result))), ...); },
+            tuple_result);
 
     return AggregateDataVariant{std::move(vector_result)};
   }
 
  public:
-  PUGS_INLINE ASTNodeDataType
-  getReturnDataType() const final
-  {
-    if constexpr (is_std_tuple_v<FX>) {
-      constexpr size_t N  = std::tuple_size_v<FX>;
-      using IndexSequence = std::make_index_sequence<N>;
-      return ASTNodeDataType::build<ASTNodeDataType::list_t>(this->_getCompoundDataTypes(IndexSequence{}));
-    } else {
-      return this->_getDataType<FX>();
-    }
-  }
-
   PUGS_INLINE std::vector<ASTNodeDataType>
   getParameterDataTypes() const final
   {
@@ -377,7 +391,7 @@ class BuiltinFunctionEmbedder<FX(void)> : public IBuiltinFunctionEmbedder
       m_f();
       return {};
     } else {
-      return EmbeddedData(_createHandler(m_f()));
+      return EmbeddedData(this->template _createHandler(m_f()));
     }
   }
 
diff --git a/tests/test_BuiltinFunctionEmbedder.cpp b/tests/test_BuiltinFunctionEmbedder.cpp
index 3e9b5192e..7fd481e16 100644
--- a/tests/test_BuiltinFunctionEmbedder.cpp
+++ b/tests/test_BuiltinFunctionEmbedder.cpp
@@ -375,15 +375,71 @@ TEST_CASE("BuiltinFunctionEmbedder", "[language]")
 
   SECTION("error")
   {
-    std::function positive = [](double x) -> bool { return x >= 0; };
-
-    BuiltinFunctionEmbedder<bool(double)> embedded_positive{positive};
-
-    std::string arg = std::string{"2.3"};
-
-    std::vector<DataVariant> args;
-    args.push_back(arg);
-
-    REQUIRE_THROWS(embedded_positive.apply(args));
+    SECTION("invalid cast")
+    {
+      std::function positive = [](double x) -> bool { return x >= 0; };
+
+      BuiltinFunctionEmbedder<bool(double)> embedded_positive{positive};
+
+      std::string arg = std::string{"2.3"};
+
+      std::vector<DataVariant> args;
+      args.push_back(arg);
+
+      std::ostringstream error_msg;
+      error_msg << "unexpected error: unexpected argument types while casting \"" << demangle<std::string>()
+                << "\" to \"" << demangle<double>() << "\"";
+
+      REQUIRE_THROWS_WITH(embedded_positive.apply(args), error_msg.str());
+    }
+
+    SECTION("invalid arg type")
+    {
+      std::ostringstream error_msg;
+      error_msg << "cannot bind C++ to language.\n"
+                << "note: argument number 1 has no associated language type: ";
+      error_msg << demangle<int>();
+      REQUIRE_THROWS_WITH(BuiltinFunctionEmbedder<bool(int)>{[](int) -> bool { return 1; }}, error_msg.str());
+    }
+
+    SECTION("invalid arg type")
+    {
+      std::ostringstream error_msg;
+      error_msg << "cannot bind C++ to language.\n"
+                << "note: argument number 2 has no associated language type: ";
+      error_msg << demangle<int>();
+      REQUIRE_THROWS_WITH(BuiltinFunctionEmbedder<bool(double, int)>{[](int, int) -> bool { return 1; }},
+                          error_msg.str());
+    }
+
+    SECTION("invalid return type")
+    {
+      std::ostringstream error_msg;
+      error_msg << "cannot bind C++ to language.\n"
+                << "note: return value has no associated language type: ";
+      error_msg << demangle<std::shared_ptr<const int>>();
+
+      std::function f = [](double) -> std::shared_ptr<const int> { return std::make_shared<int>(1); };
+
+      REQUIRE_THROWS_WITH(BuiltinFunctionEmbedder<std::shared_ptr<const int>(double)>(f), error_msg.str());
+    }
+
+    SECTION("invalid return type in compound")
+    {
+      std::ostringstream error_msg;
+      error_msg << "cannot bind C++ to language.\n"
+                << "note: return value number 2 has no associated language type: ";
+      error_msg << demangle<std::shared_ptr<const int>>();
+
+      std::function f = [](double) -> std::tuple<double, std::shared_ptr<const int>> {
+        return std::make_tuple(double{1.3}, std::make_shared<const int>(1));
+      };
+
+      REQUIRE_THROWS_WITH((BuiltinFunctionEmbedder<std::tuple<double, std::shared_ptr<const int>>(double)>{
+                            [](double) -> std::tuple<double, std::shared_ptr<const int>> {
+                              return std::make_tuple(2.3, std::make_shared<const int>(1));
+                            }}),
+                          error_msg.str());
+    }
   }
 }
-- 
GitLab