[embind] Fix misindexed template parameters which prevented policies from being applied to class functions or constructors (#24053)

Hi, trying to `allow_raw_pointers()` in a class method bound with embind
when a custom signature was given as a template argument to
`.class_function` failed. It's easy to see why by comparing the
definition of either `RegisterClassMethod` or `RegisterClassConstructor`
with their usage:

```cpp
template<typename ReturnType, typename ThisType, typename... Args>
struct RegisterClassMethod<ReturnType (ThisType, Args...)> {

    template <typename ClassType, typename Callable, typename... Policies>
    static void invoke(const char* methodName,
...
```

and its invocation in `class_function`:

```cpp
using invoker = internal::RegisterClassConstructor<
            typename std::conditional<std::is_same<Signature, internal::DeduceArgumentsTag>::value,
                                      Callable,
                                      Signature>::type>;

invoker::template invoke<ClassType, Policies...>(callable);
```

which will skip the `Callable` template argument in some specializations
(and `Callable` will become the return value policy). To fix this, I
used the same approach used by the getters/setter `PropertyTag`s (c.f.
diff). Please let me know if anything is off.

Author: canislupaster

system/include/emscripten/bind.h

@@ -820,8 +820,8 @@ inline T* getContext(const T& t) {
     return ret;
 }
 
-template<typename Accessor, typename ValueType>
-struct PropertyTag {};
+template<typename Func, typename ValueTypeOrSignature>
+struct FunctionTag {};
 
 template<typename T>
 struct GetterPolicy;
@@ -887,7 +887,7 @@ struct GetterPolicy<std::function<GetterReturnType(const GetterThisType&)>> {
 };
 
 template<typename Getter, typename GetterReturnType>
-struct GetterPolicy<PropertyTag<Getter, GetterReturnType>> {
+struct GetterPolicy<FunctionTag<Getter, GetterReturnType>> {
     typedef GetterReturnType ReturnType;
     typedef Getter Context;
 
@@ -968,7 +968,7 @@ struct SetterPolicy<std::function<SetterReturnType(SetterThisType&, SetterArgume
 };
 
 template<typename Setter, typename SetterArgumentType>
-struct SetterPolicy<PropertyTag<Setter, SetterArgumentType>> {
+struct SetterPolicy<FunctionTag<Setter, SetterArgumentType>> {
     typedef SetterArgumentType ArgumentType;
     typedef Setter Context;
 
@@ -1497,9 +1497,9 @@ struct RegisterClassConstructor<std::function<ReturnType (Args...)>> {
     }
 };
 
-template<typename ReturnType, typename... Args>
-struct RegisterClassConstructor<ReturnType (Args...)> {
-    template <typename ClassType, typename Callable, typename... Policies>
+template<typename Callable, typename ReturnType, typename... Args>
+struct RegisterClassConstructor<FunctionTag<Callable, ReturnType (Args...)>> {
+    template <typename ClassType, typename... Policies>
     static void invoke(Callable& factory) {
         typename WithPolicies<Policies...>::template ArgTypeList<ReturnType, Args...> args;
         using ReturnPolicy = rvp::take_ownership;
@@ -1633,10 +1633,10 @@ struct RegisterClassMethod<std::function<ReturnType (ThisType, Args...)>> {
     }
 };
 
-template<typename ReturnType, typename ThisType, typename... Args>
-struct RegisterClassMethod<ReturnType (ThisType, Args...)> {
+template<typename Callable, typename ReturnType, typename ThisType, typename... Args>
+struct RegisterClassMethod<FunctionTag<Callable, ReturnType (ThisType, Args...)>> {
 
-    template <typename ClassType, typename Callable, typename... Policies>
+    template <typename ClassType, typename... Policies>
     static void invoke(const char* methodName,
                        Callable& callable) {
         typename WithPolicies<Policies...>::template ArgTypeList<ReturnType, ThisType, Args...> args;
@@ -1739,7 +1739,7 @@ class class_ {
         using invoker = internal::RegisterClassConstructor<
             typename std::conditional<std::is_same<Signature, internal::DeduceArgumentsTag>::value,
                                       Callable,
-                                      Signature>::type>;
+                                      internal::FunctionTag<Callable, Signature>>::type>;
 
         invoker::template invoke<ClassType, Policies...>(callable);
         return *this;
@@ -1819,7 +1819,7 @@ class class_ {
         using invoker = internal::RegisterClassMethod<
             typename std::conditional<std::is_same<Signature, internal::DeduceArgumentsTag>::value,
                                       Callable,
-                                      Signature>::type>;
+                                      internal::FunctionTag<Callable, Signature>>::type>;
 
         invoker::template invoke<ClassType, Policies...>(methodName, callable);
         return *this;
@@ -1896,7 +1896,7 @@ class class_ {
         typedef GetterPolicy<
             typename std::conditional<std::is_same<PropertyType, internal::DeduceArgumentsTag>::value,
                                                    Getter,
-                                                   PropertyTag<Getter, PropertyType>>::type> GP;
+                                                   FunctionTag<Getter, PropertyType>>::type> GP;
         using ReturnPolicy = GetReturnValuePolicy<typename GP::ReturnType, Policies...>::tag;
         auto gter = &GP::template get<ClassType, ReturnPolicy>;
         typename WithPolicies<Policies...>::template ArgTypeList<typename GP::ReturnType> returnType;
@@ -1930,11 +1930,11 @@ class class_ {
         typedef GetterPolicy<
             typename std::conditional<std::is_same<PropertyType, internal::DeduceArgumentsTag>::value,
                                                    Getter,
-                                                   PropertyTag<Getter, PropertyType>>::type> GP;
+                                                   FunctionTag<Getter, PropertyType>>::type> GP;
         typedef SetterPolicy<
             typename std::conditional<std::is_same<PropertyType, internal::DeduceArgumentsTag>::value,
                                                    Setter,
-                                                   PropertyTag<Setter, PropertyType>>::type> SP;
+                                                   FunctionTag<Setter, PropertyType>>::type> SP;
 
 
         auto gter = &GP::template get<ClassType, ReturnPolicy>;

test/embind/embind.test.js

@@ -1461,6 +1461,9 @@ module({
             assert.equal("foo", b.getValFunction());
             b.setValFunction("bar");
 
+            // get & set with templated signature
+            assert.equal("bar", b.getThisPointer().getVal());
+
             // get & set via 'callable'
             assert.equal("bar", b.getValFunctor());
             b.setValFunctor("baz");
@@ -1834,6 +1837,11 @@ module({
             assert.equal("AbstractClass has no accessible constructor", e.message);
         });
 
+        test("can construct class with external constructor with custom signature", function() {
+            const valHolder = new cm.ValHolder(1,2);
+            assert.equal(valHolder.getVal(), 3);
+        });
+
         test("can construct class with external constructor", function() {
             var e = new cm.HasExternalConstructor("foo");
             assert.instanceof(e, cm.HasExternalConstructor);

test/embind/embind_test.cpp

@@ -327,10 +327,18 @@ ValHolder emval_test_return_ValHolder() {
   return val::object();
 }
 
+ValHolder valholder_from_sum(int x, int y) {
+  return val(x+y);
+}
+
 val valholder_get_value_mixin(const ValHolder& target) {
   return target.getVal();
 }
 
+ValHolder* valholder_get_this_ptr(ValHolder& target) {
+  return ⌖
+}
+
 void valholder_set_value_mixin(ValHolder& target, const val& value) {
   target.setVal(value);
 }
@@ -2005,13 +2013,15 @@ EMSCRIPTEN_BINDINGS(tests) {
   class_<ValHolder>("ValHolder")
     .smart_ptr<std::shared_ptr<ValHolder>>("std::shared_ptr<ValHolder>")
     .constructor<val>()
+    .constructor<ValHolder(int, int)>(&valholder_from_sum, allow_raw_pointers()) // custom signature with policy
     .function("getVal", &ValHolder::getVal)
     .function("getValNonConst", &ValHolder::getValNonConst)
     .function("getConstVal", &ValHolder::getConstVal)
     .function("getValConstRef", &ValHolder::getValConstRef)
     .function("setVal", &ValHolder::setVal)
     .function("getValFunction", std::function<val(const ValHolder&)>(&valholder_get_value_mixin))
     .function("setValFunction", std::function<void(ValHolder&, const val&)>(&valholder_set_value_mixin))
+    .function<ValHolder*(ValHolder&)>("getThisPointer", std::function<ValHolder*(ValHolder&)>(&valholder_get_this_ptr), allow_raw_pointer<ret_val>())
     .function<val(const ValHolder&)>("getValFunctor", std::bind(&valholder_get_value_mixin, _1))
     .function<void(ValHolder&, const val&)>("setValFunctor", std::bind(&valholder_set_value_mixin, _1, _2))
     .property("val", &ValHolder::getVal, &ValHolder::setVal)