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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>;
Expand Down
8 changes: 8 additions & 0 deletions test/embind/embind.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions test/embind/embind_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
Expand Down