Regression: lambdas can't be used as non-static member functions

[rubenlg]

According to #157, @pmed had already implemented this, which is why I identify this as a regression. Here is an example:

C++

class Klass {};
v8pp::class_<Klass> Klass_class(isolate);
Klass_class.ctor();
Klass_class.function("myMethod", [](const Klass &k) { return 2; });
addon.class_("Klass", Klass_class);

Javascript

const instance = new myModule.Klass();
console.log(instance);
console.log(instance.myMethod()); // Fails, should work
console.log(instance.myMethod(instance)); // Works, should fail
console.log(myModule.Klass.myMethod(instance)); // Also works, maybe it should fail?

I sent PR #194 to help troubleshoot such cases, since it wasn't clear at first why the arguments didn't match. As it turned out, myMethod is interpreted as static, and so I need to pass the instance as an argument.

This was very confusing, because in the case of .property, lambdas work as methods and interpret the first argument as the instance, not as an additional argument to be passed when calling JS.

I looked in the test directory and there is no example of using lambdas as non-static methods, which is probably the reason for this regression.

[pmed]

Hi Rubén,

thanks for sharing! The JavaScript test cases, that you have provided, look reasonable, so it's a bug. There are some sophisticated meta-functions for lambda support in .property binding, I think they could be also re-used in .function for behaviour consistency.

[rubenlg]

I tried to reuse the code from .property and thought it worked, even commented here, but I deleted the comment at the end because it was only working in some basic cases.

Thinking about it a bit more, I realized the current API is ambiguous when it comes to lambdas. With actual methods of classes, it's clear whether the method is static or not. However, with a lambda, there is no way to tell. Even if the first argument of the lambda is the class itself, without knowing the intent of the developer, there is no way to tell if it' is meant to mimic a static method that takes a class instance as its first parameter, or a non-static method and the first parameter is meant to represent this.

Here is an example in which the first argument would be the class itself, but the intent could be to have a static method:

PhysicsObject_class.function("detectCollision", [](const PhysicsObject &a, const PhysicsObject& b) { 
  /* ...  */
});

Perhaps it would be best to split .function into two separate APIs (e.g. .function and .static_function), so that developers can make their intent clear?

Klass_class.function("myMethod", [](const Klass &k) { return k.something(); });
Klass_class.static_function("myStaticMethod", [](const Klass &k) { return 2; });

Another alternative is to wrap lambdas and free functions into something that annotates the intent, e.g:

Klass_class.function("myMethod", v8pp::non_static_fn([](const Klass &k) { return k.something(); });
Klass_class.function("myStaticMethod", v8pp::static_fn([](const Klass &k) { return 2; }));

Just a couple of ideas, not sure which one fits better with the overall design.