Support for T&& function parameters and the special move constructor case

[t-bltg]

In

wrapit/src/FunctionWrapper.cpp

Lines 570 to 579 in 3cb7ace

	if(rvalueref_arg){
	//The code generated for a function with an argument passed as a r-value
	//reference does not compile.
	//Until, it is fixed, skipped these functions
	std::cerr << "Warning: no wrapper will be produced for function '"
	<< name_cxx << "(" << short_arg_list_cxx << ") " << cv
	<< "' because it contains an argument passed by r-value "
	<< "which is not supported yet.\n";
	return false;
	}

, it is stated that move constructors (taking a rvalue) are unsupported for the moment.

The generated code does not compile, right, but where should this be fixed ?
Is it a limitation in wrapit, CxxWrap or libcxxwrap-julia ?

I'd like to try to fix this, but I will need some pointers to start, so any comment would be very helpful.

[grasph]

I don't remember where the issue comes from. The easiest way to start with to address it, is to comment this part of code and run wrapit on a function that takes a pass-by-r-value argument.

[t-bltg]

Thanks, well the generated code errors in libcxxwrap-julia, here is my toy code for the problem:

circle.hh

#include <iostream>
#include <utility>

using namespace std;

class Circle {
public:
    Circle(int r)
    {
        _radius = r;
        cout << "Standard constructor. Radius: " << _radius << endl;
    }

    Circle(const Circle& c)
    {
        _radius = c.radius();
        cout << "Copy constructor with lvalue reference. Radius: " << _radius << endl;
    }

    Circle(Circle&& c)
    {
        _radius = c.radius();
        cout << "Move constructor with rvalue reference. Radius: " << _radius << endl;
    }

    int radius() const {
        return _radius;
    }

private:
    int _radius;
};

void test()
{
    cout << "== from c++ ==" << endl;
    Circle c1(2);
    Circle c2(c1);
    Circle c3(std::move(c2));
}

conf.toml

module_name = "Circ"
input = ["circle.hh"]

wrapit generated jlCirc.cxx

Details

#include <type_traits>
#include "jlcxx/jlcxx.hpp"
#include "jlcxx/functions.hpp"
#include "jlcxx/stl.hpp"

#include "jlCirc.h"

#ifdef VERBOSE_IMPORT
#  define DEBUG_MSG(a) std::cerr << a << "\n"
#else
#  define DEBUG_MSG(a)
#endif
#define __HERE__  __FILE__ ":" QUOTE2(__LINE__)
#define QUOTE(arg) #arg
#define QUOTE2(arg) QUOTE(arg)

namespace jlcxx {
  template<> struct IsMirroredType<Circle> : std::false_type { };
  template<> struct DefaultConstructible<Circle> : std::false_type { };
}

JLCXX_MODULE define_julia_module(jlcxx::Module& types){

  DEBUG_MSG("Adding wrapper for type Circle (" __HERE__ ")");
  // defined in ./circle.hh:6:7
  auto t0 = types.add_type<Circle>("Circle");

  /**********************************************************************/
  /* Wrappers for the methods of class Circle
   */


  DEBUG_MSG("Adding wrapper for void Circle::Circle(int) (" __HERE__ ")");
  // defined in ./circle.hh:8:5
  t0.constructor<int>(/*finalize=*/true);


  DEBUG_MSG("Adding wrapper for void Circle::Circle(const Circle &) (" __HERE__ ")");
  // defined in ./circle.hh:14:5
  t0.constructor<const Circle &>(/*finalize=*/true);


  DEBUG_MSG("Adding wrapper for void Circle::Circle(Circle &&) (" __HERE__ ")");
  // defined in ./circle.hh:20:5
  t0.constructor<Circle &&>(/*finalize=*/true);

  DEBUG_MSG("Adding wrapper for int Circle::radius() (" __HERE__ ")");
  // signature to use in the veto list: int Circle::radius()
  // defined in ./circle.hh:26:9
  t0.method("radius", static_cast<int (Circle::*)()  const>(&Circle::radius));

  /* End of Circle class method wrappers
   **********************************************************************/


  /**********************************************************************
   * Wrappers for global functions and variables including
   * class static members
   */

  DEBUG_MSG("Adding wrapper for void test() (" __HERE__ ")");
  // signature to use in the veto list: void test()
  // defined in ./circle.hh:34:6
  types.method("test", static_cast<void (*)() >(&test));

  /* End of global function wrappers
   **********************************************************************/

  DEBUG_MSG("End of wrapper definitions");

}

error log

Details

In file included from jlCirc.cxx:2:
In file included from [...]/include/jlcxx/jlcxx.hpp:15:
[...]/include/jlcxx/module.hpp:72:14: error: no matching function for call to object of type 'ReturnTypeAdapter<jlcxx::BoxedValue<Circle>, Circle &&>'
      return ReturnTypeAdapter<R, Args...>()(functor, args...);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[...]/include/jlcxx/module.hpp:209:69: note: in instantiation of member function 'jlcxx::detail::CallFunctor<jlcxx::BoxedValue<Circle>, Circle &&>::apply' requested here
    return reinterpret_cast<void*>(detail::CallFunctor<R, Args...>::apply);
                                                                    ^
[...]/include/jlcxx/module.hpp:196:3: note: in instantiation of member function 'jlcxx::FunctionWrapper<jlcxx::BoxedValue<Circle>, Circle &&>::pointer' requested here
  FunctionWrapper(Module* mod, const functor_t &function) : FunctionWrapperBase(mod, julia_return_type<R>()), m_function(function)
  ^
[...]/include/jlcxx/module.hpp:526:29: note: in instantiation of member function 'jlcxx::FunctionWrapper<jlcxx::BoxedValue<Circle>, Circle &&>::FunctionWrapper' requested here
    auto* new_wrapper = new FunctionWrapper<R, Args...>(this, f);
                            ^
[...]/include/jlcxx/module.hpp:700:12: note: in instantiation of function template specialization 'jlcxx::Module::method<jlcxx::BoxedValue<Circle>, Circle &&>' requested here
    return method(name, std::function<R(ArgsT...)>(std::forward<LambdaT>(lambda)));
           ^
[...]/include/jlcxx/module.hpp:555:12: note: in instantiation of function template specialization 'jlcxx::Module::add_lambda<jlcxx::BoxedValue<Circle>, (lambda at [...]/include/jlcxx/module.hpp:562:67), Circle &&>' requested here
    return add_lambda(name, std::forward<LambdaT>(lambda), &LambdaT::operator());
           ^
[...]/include/jlcxx/module.hpp:562:51: note: in instantiation of function template specialization 'jlcxx::Module::method<(lambda at [...]/include/jlcxx/module.hpp:562:67)>' requested here
    FunctionWrapperBase &new_wrapper = finalize ? method("dummy", [](ArgsT... args) { return create<T, true>(args...); }) : method("dummy", [](ArgsT... args) { return create<T, false>(args...); });
                                                  ^
[...]/include/jlcxx/module.hpp:986:16: note: in instantiation of function template specialization 'jlcxx::Module::constructor<Circle, Circle &&>' requested here
      m_module.constructor<T, ArgsT...>(m_dt, finalize);
               ^
jlCirc.cxx:45:6: note: in instantiation of function template specialization 'jlcxx::TypeWrapper<Circle>::constructor<Circle &&>' requested here
  t0.constructor<Circle &&>(/*finalize=*/true);
     ^
[...]/include/jlcxx/module.hpp:43:22: note: candidate function not viable: expects an rvalue for 2nd argument
  inline return_type operator()(const void* functor, static_julia_type<Args>... args)
                     ^
1 error generated.

[grasph]

It's a CxxWrap limitation. When do you want the move constructor to be called? It is not clear to me how you'd like to map the copy and move constructors to Julia.

[t-bltg]

Yes, it's unclear to me conceptually too how a move constructor would be called from the julia side, so it might be a dead end.

However, I'm wrapping a library that I'm in no position of modifying its sources.
There, a class is declared where only a move constructor is defined so I'm kinda stuck here in my wrapping attempts.

[grasph]

I see. I'm adding @barche to get its opinion.

It's not clear to me know the move-only class should be handled, and more general pass-by-rvalue function parameters. The std::uniq_ptr is one notorious example of move-only class. As @t-bltg highlighted, Wrapit currently skip move-constructor and any function with a T&& parameter.

@t-bltg, if you give more insights on the class you want to wrap, this might help to understand use cases and how mapping to Julia should be done.

Philippe.

[t-bltg]

Thanks Philippe,

I cannot disclose the sources, but here is a reduced example of what is intended to be wrapped (you were dead-on right about std::unique_ptr usage):

#include <memory>

struct Options;

enum class State
{
  IDLE = 0,
  RUNNING = 1,
};

class CurrentState
{
public:

  explicit CurrentState(std::unique_ptr<Options> && Options);

  ~CurrentState();

  CurrentState(CurrentState const &) = delete;
  CurrentState(CurrentState &&) = delete;
  CurrentState & operator=(CurrentState const &) = delete;
  CurrentState & operator=(CurrentState &&) = delete;

  State getState() const
  { return m_state; }

private:
  State m_state;

  std::unique_ptr<Options> m_Options;
};

[grasph]

Let's wait from @barche's inputs to see how to handle this case.

If you need an immediate solution, here is a workaround, you can use in the meantime.

Define a function in a header file you add to the WrapIt input list,

CurrentState createCurrentState(const Option& o){ 
  return CurrentState(std::make_unique<Option>(o)); 
}

Then, either require calling this function instead of the usual constructor or, for a more transparent use, define a constructor inside the julia module:

CurrentState(o::Option) = createCurrentState(o)

To customize the julia module, you can use the pattern of ex002-ROOT: export list is generated in a separate file and the generated master file (limited to few lines) is discarded and replaced by a custom file. This will also allow you customizing the shared library path and solve at the same time the Issue #18.

Philippe.

[t-bltg]

That's a nice workaround !
I had to heap allocate and return a pointer, since the copy constructor was marked deleted in my example (hence failing to compile on return), but this is a detail unrelated to the initial issue.

Regarding #18, I ended up writing my own module, however from a user point of view (a beginner using wrapit for the first time), I believe it might be better to use @__DIR__ since the generated module and the shared library reside in the same location.

[barche]

I think @grasph 's workaround is the only way for now. There is currently no way to directly wrap the move constructor, but it seems like it would be a nice feature to have. I think it should be possible to also provide a move function in CxxWrap to go along with it, to make sure the move constructor is called.