Skip to content

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

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

Merged
merged 1 commit into from
Apr 28, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions system/include/emscripten/bind.h
Original file line number Diff line number Diff line change
@@ -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>;
8 changes: 8 additions & 0 deletions test/embind/embind.test.js
Original file line number Diff line number Diff line change
@@ -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);
10 changes: 10 additions & 0 deletions test/embind/embind_test.cpp
Original file line number Diff line number Diff line change
@@ -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 &target;
}

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)