Skip to content

[embind] Add pointer policies for creating val objects. #24175

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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
2 changes: 2 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ See docs/process.md for more on how version tagging works.

4.0.8 (in development)
----------------------
- Embind's `val` now requires a pointer policy when using pointers. e.g.
`(val v(pointer, allow_raw_pointers())`.

4.0.7 - 04/15/25
----------------
Expand Down
18 changes: 10 additions & 8 deletions system/include/emscripten/bind.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,11 @@ struct allow_raw_pointer {
struct allow_raw_pointers {
template<typename InputType, int Index>
struct Transform {
// Use decay to handle references to pointers e.g.(T*&)->(T*).
typedef typename std::decay<InputType>::type DecayedType;
typedef typename std::conditional<
std::is_pointer<InputType>::value,
internal::AllowedRawPointer<typename std::remove_pointer<InputType>::type>,
std::is_pointer<DecayedType>::value,
internal::AllowedRawPointer<typename std::remove_pointer<DecayedType>::type>,
InputType
>::type type;
};
Expand Down Expand Up @@ -2054,7 +2056,7 @@ struct VectorAccess {
typename VectorType::size_type index
) {
if (index < v.size()) {
return val(v[index]);
return val(v[index], allow_raw_pointers());
} else {
return val::undefined();
}
Expand Down Expand Up @@ -2085,11 +2087,11 @@ class_<std::vector<T, Allocator>> register_vector(const char* name) {
size_t (VecType::*size)() const = &VecType::size;
return class_<std::vector<T>>(name)
.template constructor<>()
.function("push_back", push_back)
.function("resize", resize)
.function("push_back", push_back, allow_raw_pointers())
.function("resize", resize, allow_raw_pointers())
.function("size", size)
.function("get", &internal::VectorAccess<VecType>::get)
.function("set", &internal::VectorAccess<VecType>::set)
.function("get", &internal::VectorAccess<VecType>::get, allow_raw_pointers())
.function("set", &internal::VectorAccess<VecType>::set, allow_raw_pointers())
;
}

Expand Down Expand Up @@ -2183,7 +2185,7 @@ struct BindingType<std::optional<T>> {
template<typename ReturnPolicy = void>
static WireType toWireType(std::optional<T> value, rvp::default_tag) {
if (value) {
return ValBinding::toWireType(val(*value), rvp::default_tag{});
return ValBinding::toWireType(val(*value, allow_raw_pointers()), rvp::default_tag{});
}
return ValBinding::toWireType(val::undefined(), rvp::default_tag{});
}
Expand Down
8 changes: 4 additions & 4 deletions system/include/emscripten/val.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,12 +364,12 @@ class EMBIND_VISIBILITY_DEFAULT val {
return val(internal::_emval_get_module_property(name));
}

template<typename T>
explicit val(T&& value) {
template<typename T, typename... Policies>
explicit val(T&& value, Policies...) {
using namespace internal;

typename WithPolicies<Policies...>::template ArgTypeList<T> valueType;
WireTypePack<T> argv(std::forward<T>(value));
new (this) val(_emval_take_value(internal::TypeID<T>::get(), argv));
new (this) val(_emval_take_value(valueType.getTypes()[0], argv));
}

val() : val(EM_VAL(internal::_EMVAL_UNDEFINED)) {}
Expand Down
22 changes: 18 additions & 4 deletions system/include/emscripten/wire.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,12 @@ typedef const void* TYPEID;
// just need a unique identifier per type and polymorphic type
// identification.

template<typename T>
static inline constexpr bool IsCanonicalized = std::is_same<T, typename std::decay<T>::type>::value;

template<typename T>
struct CanonicalizedID {
static_assert(IsCanonicalized<T>, "T should not be a reference or cv-qualified");
static char c;
static constexpr TYPEID get() {
return &c;
Expand All @@ -65,6 +69,7 @@ struct Canonicalized {
template<typename T>
struct LightTypeID {
static constexpr TYPEID get() {
static_assert(IsCanonicalized<T>, "T should not be a reference or cv-qualified");
if (has_unbound_type_names) {
#if __has_feature(cxx_rtti)
return &typeid(T);
Expand All @@ -83,6 +88,7 @@ struct LightTypeID {

template<typename T>
constexpr TYPEID getLightTypeID(const T& value) {
static_assert(IsCanonicalized<T>, "T should not be a reference or cv-qualified");
if (has_unbound_type_names) {
#if __has_feature(cxx_rtti)
return &typeid(value);
Expand Down Expand Up @@ -137,6 +143,18 @@ struct TypeID<AllowedRawPointer<T>> {
}
};

template<typename T>
struct TypeID<const T> : TypeID<T> {
};

template<typename T>
struct TypeID<T&> : TypeID<T> {
};

template<typename T>
struct TypeID<T&&> : TypeID<T> {
};

// ExecutePolicies<>

template<typename... Policies>
Expand Down Expand Up @@ -323,10 +341,6 @@ template<typename T>
struct BindingType<T&> : public BindingType<T> {
};

template<typename T>
struct BindingType<const T&> : public BindingType<T> {
};

template<typename T>
struct BindingType<T&&> {
typedef typename BindingType<T>::WireType WireType;
Expand Down
2 changes: 1 addition & 1 deletion test/embind/embind_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ static DummyForPointer emval_pointer_dummy(42);

val emval_test_instance_pointer() {
DummyForPointer* p = &emval_pointer_dummy;
return val(p);
return val(p, allow_raw_pointers());
}

int emval_test_value_from_instance_pointer(val v) {
Expand Down
3 changes: 1 addition & 2 deletions test/embind/test_custom_marshal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ using IntWrapperIntermediate = int;
// Specify custom (un)marshalling for all types satisfying IsIntWrapper.
namespace emscripten {
namespace internal {
// remove_cv/remove_reference are required for TypeID, but not BindingType, see https://github.com/emscripten-core/emscripten/issues/7292
template<typename T>
struct TypeID<T, typename std::enable_if<IsIntWrapper<typename std::remove_cv<typename std::remove_reference<T>::type>::type>::value, void>::type> {
struct TypeID<T, typename std::enable_if<IsIntWrapper<T>::value, void>::type> {
static constexpr TYPEID get() {
return TypeID<IntWrapperIntermediate>::get();
}
Expand Down
6 changes: 6 additions & 0 deletions test/embind/test_embind_no_raw_pointers_val_1.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <emscripten/bind.h>
#include <emscripten/val.h>
int main() {
int x = 0;
emscripten::val v(&x); // int*
}
6 changes: 6 additions & 0 deletions test/embind/test_embind_no_raw_pointers_val_2.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <emscripten/bind.h>
#include <emscripten/val.h>
int main() {
void* x = nullptr;
emscripten::val v(x); // void*&
}
6 changes: 6 additions & 0 deletions test/embind/test_embind_no_raw_pointers_val_3.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <emscripten/bind.h>
#include <emscripten/val.h>
int main() {
void* const x = nullptr;
emscripten::val v(x); // void* const
}
21 changes: 21 additions & 0 deletions test/embind/test_embind_val_basics.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#include <stdio.h>
#include <emscripten/val.h>

using namespace emscripten;

int main() {
val Math = val::global("Math");

// two ways to call Math.abs
printf("abs(-10): %d\n", Math.call<int>("abs", -10));
printf("abs(-11): %d\n", Math["abs"](-11).as<int>());

// Const-qualification and references should not matter.
int x = -12;
const int y = -13;
printf("abs(%d): %d\n", x, Math.call<int>("abs", x)); // x is deduced to int&
printf("abs(%d): %d\n", y, Math.call<int>("abs", y)); // y is deduced to const int&
printf("abs(-14): %d\n", Math.call<const int>("abs", -14));

return 0;
}
5 changes: 5 additions & 0 deletions test/embind/test_embind_val_basics.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
abs(-10): 10
abs(-11): 11
abs(-12): 12
abs(-13): 13
abs(-14): 14
18 changes: 1 addition & 17 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -7296,23 +7296,7 @@ def test2():
})
def test_embind_val_basics(self, args):
self.maybe_closure()
create_file('test.cpp', r'''
#include <stdio.h>
#include <emscripten/val.h>

using namespace emscripten;

int main() {
val Math = val::global("Math");

// two ways to call Math.abs
printf("abs(-10): %d\n", Math.call<int>("abs", -10));
printf("abs(-11): %d\n", Math["abs"](-11).as<int>());

return 0;
}
''')
self.do_runf('test.cpp', 'abs(-10): 10\nabs(-11): 11', emcc_args=args)
self.do_run_in_out_file_test('embind/test_embind_val_basics.cpp', emcc_args=args)

@node_pthreads
def test_embind_basics(self):
Expand Down
9 changes: 9 additions & 0 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -3308,6 +3308,15 @@ def test_embind_closure_no_dynamic_execution(self):
self.do_runf('main.cpp', '10\nok\n',
emcc_args=['--no-entry', '-lembind', '-O2', '--closure=1', '--minify=0', '--post-js=post.js'])

@parameterized({
'val_1': ['embind/test_embind_no_raw_pointers_val_1.cpp'],
'val_2': ['embind/test_embind_no_raw_pointers_val_2.cpp'],
'val_3': ['embind/test_embind_no_raw_pointers_val_3.cpp'],
})
def test_embind_no_raw_pointers(self, filename):
stderr = self.expect_fail([EMCC, '-lembind', test_file(filename)])
self.assertContained('Implicitly binding raw pointers is illegal.', stderr)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm almost tempted to say you should just write 3 separate tests here (since each one is only two lines), but I'm not sure it makes any difference really.


@is_slow_test
@parameterized({
'': [],
Expand Down