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

Does not support classes or structs which are contained within template classes #14

Open
finger563 opened this issue Jul 31, 2024 · 2 comments

Comments

@finger563
Copy link
Contributor

I have c++ such as:

template <typename T> class Bezier {
public:
  /**
   * @brief Unweighted cubic bezier configuration for 4 control points.
   */
  struct Config {
    std::array<T, 4> control_points; ///< Array of 4 control points
  };

  /**
   * @brief Weighted cubic bezier configuration for 4 control points with
   *        individual weights.
   */
  struct WeightedConfig {
    std::array<T, 4> control_points; ///< Array of 4 control points
    std::array<float, 4> weights = {1.0f, 1.0f, 1.0f,
                                    1.0f}; ///< Array of 4 weights, default is array of 1.0f
  };

  /**
   * @brief Construct an unweighted cubic bezier curve for evaluation.
   * @param config Unweighted Config structure containing the control points.
   */
  explicit Bezier(const Config &config)
      : weighted_(false)
      , control_points_(config.control_points) {}

  /**
   * @brief Construct a rational / weighted cubic bezier curve for evaluation.
   * @param config Rational / weighted WeightedConfig structure containing the
   *        control points and their weights.
   */
  explicit Bezier(const WeightedConfig &config)
      : weighted_(true)
      , control_points_(config.control_points)
      , weights_(config.weights) {}

And when I run litgen on it (specializing Bezier using the config), I get output of the form:

  ////////////////////    <generated_from:bezier.hpp>    ////////////////////
  auto pyClassBezier_espp_Vector2f =
      py::class_<espp::Bezier<espp::Vector2f>>
          (m, "Bezier_espp_Vector2f", py::dynamic_attr(), "*\n * @brief Implements rational / weighted and unweighted cubic bezier curves\n *        between control points.\n * @note See https://pomax.github.io/bezierinfo/ for information on bezier\n *       curves.\n * @note Template class which can be used individually on floating point\n *       values directly or on containers such as Vector2<float>.\n * @tparam T The type of the control points, e.g. float or Vector2<float>.\n * @note The bezier curve is defined by 4 control points, P0, P1, P2, P3.\n *      The curve is defined by the equation:\n *      \\f$B(t) = (1-t)^3 * P0 + 3 * (1-t)^2 * t * P1 + 3 * (1-t) * t^2 * P2 + t^3 * P3\\f$\n *      where t is the evaluation parameter, [0, 1].\n *\n * @note The weighted bezier curve is defined by 4 control points, P0, P1, P2, P3\n *      and 4 weights, W0, W1, W2, W3.\n *      The curve is defined by the equation:\n *      \\f$B(t) = (W0 * (1-t)^3 * P0 + W1 * 3 * (1-t)^2 * t * P1 + W2 * 3 * (1-t) * t^2 * P2 + W3 *\n * t^3 * P3) / (W0 + W1 + W2 + W3)\\f$ where t is the evaluation parameter, [0, 1].\n *\n * \\section bezier_ex1 Example\n * \\snippet math_example.cpp bezier example\n");

  { // inner classes & enums of Bezier_espp_Vector2f
      auto pyClassBezier_ClassConfig =
          py::class_<espp::Bezier::Config>
              (pyClassBezier, "Config", py::dynamic_attr(), "*\n   * @brief Unweighted cubic bezier configuration for 4 control points.\n")
          .def(py::init<>()) // implicit default constructor
          .def_readwrite("control_points", &espp::Bezier::Config::control_points, "/< Array of 4 control points")
          ;
      auto pyClassBezier_ClassWeightedConfig =
          py::class_<espp::Bezier::WeightedConfig>
              (pyClassBezier, "WeightedConfig", py::dynamic_attr(), "*\n   * @brief Weighted cubic bezier configuration for 4 control points with\n   *        individual weights.\n")
          .def(py::init<>()) // implicit default constructor
          .def_readwrite("control_points", &espp::Bezier::WeightedConfig::control_points, "/< Array of 4 control points")
          .def_readwrite("weights", &espp::Bezier::WeightedConfig::weights, "/< Array of 4 weights, default is array of 1.0")
          ;
  } // end of inner classes & enums of Bezier_espp_Vector2f

  pyClassBezier_espp_Vector2f
      .def(py::init<>()) // implicit default constructor
      .def("__call__",
          &espp::Bezier<espp::Vector2f>::operator(),
          py::arg("t"),
          "*\n   * @brief Evaluate the bezier at \\p t.\n   * @note Convienience wrapper around the at() method.\n   * @param t The evaluation parameter, [0, 1].\n   * @return The bezier evaluated at \\p t.\n")
      ;
  ////////////////////    </generated_from:bezier.hpp>    ////////////////////

Where the template class is appropriately specialized, but the contained config structs are not.

Looking through the code for adapted_types/adapted_class.py, I see that it only checks that the cpp_element().is_template_partially_specialized() which does not check whether the classes which may contain the class in question (e.g. up the scope chain) may themselves be template classes and specialized. I've been looking through it some to figure out how I might add such a check, but I'm not very familiar with srcml / srcmlcpp so I figured I'd go ahead and open the issue while I work through it to see if you or anyone else has any ideas.

@finger563
Copy link
Contributor Author

As a note, this also affects the binding in another way as you can see the Config and WeightedConfig are bound to the non-existent pyClassBezier instead of the correct pyClassBezier_espp_Vector2f.

@pthom
Copy link
Owner

pthom commented Jul 31, 2024

Hello,

Looking through the code for adapted_types/adapted_class.py, I see that it only checks that the cpp_element().is_template_partially_specialized() which does not check whether the classes which may contain the class in question (e.g. up the scope chain) may themselves be template classes and specialized. I've been looking through it some to figure out how I might add such a check, but I'm not very familiar with srcml / srcmlcpp so I figured I'd go ahead and open the issue while I work through it to see if you or anyone else has any ideas.

You are on the right track. Adding support for inner template classes is a bit complex, since it is buried in the machinery.
Adding support for inner classes would require adding recursion, I guess. I'm on holiday right now, so that I do not have a lot of time for this.

However, I created a branch for the work on this with an initial commit. IMHO, the correct way to work on this is to add an integration test.

See the related commit that creates the branch:

And here are some explanations:
image

  • src/litgen/integration_tests: some tests that are compiled and part of the C++ tests. (you need to compile litgen with CMake)
  • I added `template_class_inner.h". Then, "template_class_inner.pyi", and "template_class_inner.pydef.cpp" are autogenerated (and compiled: the compilation fails for the moment)
  • mylib.h: I temporarily only included "template_class_inner.h"

I simplified the C++ test code as much as possible:

template<typename DataType>
struct Pair
{
    struct DataContainer
    {
        DataType value;
    };
    DataContainer first, second;
};

If you want to give it a try, this branch is a good starting point, as you can test directly from within the litgen repo.

If it is too complex, my advice would be to not use inner template classes, at least for the moment.

Cheers

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

No branches or pull requests

2 participants