pmed / v8pp

Bind C++ functions and classes into V8 JavaScript engine
http://pmed.github.io/v8pp/
Other
898 stars 120 forks source link

Question: overloaded ctor #41

Closed jcmonnin closed 7 years ago

jcmonnin commented 7 years ago

Hi,

I need to provide an overloaded ctor in the JavaScript binding of my class. When calling multiple times X_class.ctor<...>() with different template parameters, it compiles. However it seems to consider only the last call.

I've also looked at v8pp::factory documentation, but I've not found a way to handled overloads in the ctor so far.

Is there a way to achieve that?

pmed commented 7 years ago

Currently binding of overloaded functions is not allowed, but this feature in my TODO list.

As a workaround I used a generic signature for C++ functions with v8::FunctionCallbackInfo and manual management depending on the function arguments passed from JavaScript:

void f(v8::FunctionCallbackInfo<v8::Value> const& args)
{
    if (args[0]->IsString()) f_string(args);
    else if (args[0]->IsObject() f_object(args);
    else thow std::runtime_error("unsupported arguments");
}

v8pp::factory<T> template is intended to specialize create/destroy operations for a wrapped class T (and default factory implementation uses new/delete).

jcmonnin commented 7 years ago

Thanks for your help. Generic function signature for ctor would be good. I don't have too may overloaded constructors, so doing them 'by hand' is not a big problem. However, how do I assign that function to be used as ctor?

Please consider following sample:

struct X
{
    bool b;
    X() : b(true) {}
    X(bool b) : b(b) {}
};

v8::Handle<v8::Object> Xctor(v8::FunctionCallbackInfo<v8::Value> const& args)
{
    v8::Isolate* isolate = args.GetIsolate();
    if (args.Length() == 0) {
        return v8pp::class_<X>::import_external(isolate, new X());
    } else if (args.Length() == 1 && args[0]->IsBoolean()) {
        bool b = v8pp::from_v8<bool>(isolate, args[0]);
        return v8pp::class_<X>::import_external(isolate, new X(b));
    } else {
        throw std::runtime_error("unsupported arguments");
    }
}

void v8ppTestBind(v8::Isolate* isolate)
{
    v8pp::class_<X> X_class(isolate);
    X_class
        .set("b", &X::b);

    v8pp::module module(isolate);
    module
        .set("X", X_class)
        .set("Xctor", Xctor);

    isolate->GetCurrentContext()->Global()->Set(v8::String::NewFromUtf8(isolate, "module"), module.new_instance());
}

From a functionality point of view it does what I want when calling var x = module.Xctor(false); or var x = module.Xctor();, however I would like to have them named as a real constructor (ie. var x = module.X(false); or var x = module.X();)

pmed commented 7 years ago

You can add a constructor with v8::FunctionCallbackInfo argument and bind it with ctor() function:

struct X
{
    bool b;
    X() : b(true) {}
    X(bool b) : b(b) {}
    X(v8::FunctionCallbackInfo<v8::Value> const& args)
    {
         // convert 1st argument to bool, otherwise use true for default value
        b = v8pp::from_v8<bool>(args.GetIsolate(), args[0], true);
    }
};

v8pp::class_<X> X_class(isolate);
    X_class
        .ctor<v8::FunctionCallbackInfo<v8::Value> const&>() // use V8 arguments constructor
        .set("b", &X::b);

In this case X can be created from JavaScript:

var x = new module.X(); // x.b == true
var x2 = new module.X(false); // x2.b == false
var x3 = new module.X(1, "any", { "other" : ["arguments"] }); // x3.b == true
jcmonnin commented 7 years ago

Thanks. Unfortunately, in the real code, the class X has a fixed ABI (external lib) and I can't add a v8pp specific ctor inside the class. I've come up with the solution below. It requires a small code change on the v8pp side (see ad8b0fa213f35374002d472ba5db632272fe5f6b). Comments welcome.

struct X
{
    bool b;
    X() : b(true) {}
    X(bool b) : b(b) {}
};

X* Xctor(v8::FunctionCallbackInfo<v8::Value> const& args)
{
    v8::Isolate* isolate = args.GetIsolate();
    if (args.Length() == 0) {
        return v8pp::factory<X>::create(isolate);
    } else if (args.Length() == 1 && args[0]->IsBoolean()) {
        bool b = v8pp::from_v8<bool>(isolate, args[0]);
        return v8pp::factory<X>::create(isolate, b);
    } else {
        throw std::runtime_error("unsupported arguments");
    }
}

void v8ppTestBind(v8::Isolate* isolate)
{
    std::function<X* (v8::FunctionCallbackInfo<v8::Value> const&)> test = Xctor;

    v8pp::class_<X> X_class(isolate);
    X_class
        .ctor(Xctor)
        .set("b", &X::b);

    v8pp::module module(isolate);
    module
        .set("X", X_class);

    isolate->GetCurrentContext()->Global()->Set(v8::String::NewFromUtf8(isolate, "module"), module.new_instance());
}
pmed commented 7 years ago

Hi, Jean-Claude

It seems I was wrong, and specialization of v8pp::factory::create can be used as an external constructor, like (haven't tested the code below):

v8pp::factory<X>::create(v8::Isolate* isolate, v8::FunctionCallbackInfo<v8::Value> const& args)
{
    assert(isolate == args.GetIsolate());
    if (args.Length() == 0) {
        return new X();
    } else if (args.Length() == 1 && args[0]->IsBoolean()) {
        bool b = v8pp::from_v8<bool>(isolate, args[0]);
        return new X(b);
    } else {
        throw std::runtime_error("unsupported arguments");
    }
}

v8pp::class_<X> X_class(isolate);
    X_class
        .ctor<v8::FunctionCallbackInfo<v8::Value> const&>() // use V8 arguments constructor
        .set("b", &X::b);

This way doesn't require to add a constructor X::X(v8::FunctionCallbackInfo<v8::Value> const&) into the existing class X.

Anyway, your pull request #42 allows to set a custom function for the class constructor in a simple way and I like this idea.

I'm wondering is it possible to have one function class_<T>::ctor(f = &factory<T>::create) instead of class_::ctor() overloading. I will check the idea tomorrow, after the weekend.

pmed commented 7 years ago

I tried to use pointer to a function in class_::ctor() with default value of &factory::create:

template<typename T>
v8pp::class_
{
    template<typename ...Args>
    static T* factory_create(v8::FunctionCallbackInfo<v8::Value> const& args)
    {
        using ctor_type = T* (*)(v8::Isolate* isolate, Args...);
        return detail::call_from_v8(static_cast<ctor_type>(&factory<T>::create), args);
    }

public:
    /// Set class constructor signature
    template<typename ...Args>
    class_& ctor(T* (*create)(v8::FunctionCallbackInfo<v8::Value> const&)
        = &class_::template factory_create<Args...>)
    {
        class_singleton_.ctor(create);
        return *this;
    }
...
};

This code doesn't work with GCC 4.8 on Args... expansion at line &class_::template factory_create<Args...>, with GCC 4.9 it's ok.

I would like to stay with GCC 4.8 version, since it may be a default compiler in Linux distros with long term support, say Ubuntu.

As I mentioned above, such a custom constructor function duplicates v8pp::factory::create, and I think there should be ability to set a custom destructor similar to v8pp::factory::destroy.

Next my thought: why to have v8pp::factory at all in the library, if we could have 2 pointers to create/destroy functions (and they actually are in v8pp::detail::class_singleton) and to use default implementations for these functions. But removing of v8pp::factory may be unacceptable for some library users, because it would break existing code.

jcmonnin commented 7 years ago

I'm not really in a position to judge if replacing v8pp::factory is a good thing for the library. Still learning v8pp. I understand that GCC 4.8 must be supported.

Even if overloads would be implemented in a future version of v8pp, I think it's still useful to be able to bind a simple function as ctor. I see following use case: I have some classes to bind with fairly complicated constructors using structs. The most usable interface for JavaScript is to provide an object as constructor parameter, which can be handled in this ctor function (calling the appropriate C++ overload depending on the object content). Trying to make a 1:1 binding of the C++ code would not give the most friendly JavaScript interface.

What I'm trying to say is that it would be good to have an officially supported way to have a simple ctor function. I don't care too much how this is achieved.

pmed commented 7 years ago

Fixed