Skip to content

Commit c31159f

Browse files
authored
[embind] Add pointer policies for creating val objects. (#24175)
Using pointers with val worked inconsistently before where: ``` Foo f; Foo* p = &f; val v1(p); // works fine val v2(&f); // fails ``` The pointer working above was probably a mistake[1] and was caused by TypeID normalizing the types differently than how BindingType does. This patch picks up the work done previously[2] to enforce that types are normalized consistently. In the above example both will now require a pointer policy e.g. `(val v(p, allow_raw_pointers())`. [1]#7292 (comment) [2]https://github.com/yeputons/emscripten/tree/fix-7292-embind-type-normalize
1 parent 1ae691e commit c31159f

13 files changed

+90
-36
lines changed

ChangeLog.md

+2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ See docs/process.md for more on how version tagging works.
3131
network requests. (#24163, #24190)
3232
- Closure arguments can now be used from ports using `settings.CLOSURE_ARGS`
3333
(#24192)
34+
- Embind's `val` now requires a pointer policy when using pointers. e.g.
35+
`(val v(pointer, allow_raw_pointers())`.
3436

3537
4.0.7 - 04/15/25
3638
----------------

system/include/emscripten/bind.h

+10-8
Original file line numberDiff line numberDiff line change
@@ -314,9 +314,11 @@ struct allow_raw_pointer {
314314
struct allow_raw_pointers {
315315
template<typename InputType, int Index>
316316
struct Transform {
317+
// Use decay to handle references to pointers e.g.(T*&)->(T*).
318+
typedef typename std::decay<InputType>::type DecayedType;
317319
typedef typename std::conditional<
318-
std::is_pointer<InputType>::value,
319-
internal::AllowedRawPointer<typename std::remove_pointer<InputType>::type>,
320+
std::is_pointer<DecayedType>::value,
321+
internal::AllowedRawPointer<typename std::remove_pointer<DecayedType>::type>,
320322
InputType
321323
>::type type;
322324
};
@@ -2054,7 +2056,7 @@ struct VectorAccess {
20542056
typename VectorType::size_type index
20552057
) {
20562058
if (index < v.size()) {
2057-
return val(v[index]);
2059+
return val(v[index], allow_raw_pointers());
20582060
} else {
20592061
return val::undefined();
20602062
}
@@ -2085,11 +2087,11 @@ class_<std::vector<T, Allocator>> register_vector(const char* name) {
20852087
size_t (VecType::*size)() const = &VecType::size;
20862088
return class_<std::vector<T>>(name)
20872089
.template constructor<>()
2088-
.function("push_back", push_back)
2089-
.function("resize", resize)
2090+
.function("push_back", push_back, allow_raw_pointers())
2091+
.function("resize", resize, allow_raw_pointers())
20902092
.function("size", size)
2091-
.function("get", &internal::VectorAccess<VecType>::get)
2092-
.function("set", &internal::VectorAccess<VecType>::set)
2093+
.function("get", &internal::VectorAccess<VecType>::get, allow_raw_pointers())
2094+
.function("set", &internal::VectorAccess<VecType>::set, allow_raw_pointers())
20932095
;
20942096
}
20952097

@@ -2183,7 +2185,7 @@ struct BindingType<std::optional<T>> {
21832185
template<typename ReturnPolicy = void>
21842186
static WireType toWireType(std::optional<T> value, rvp::default_tag) {
21852187
if (value) {
2186-
return ValBinding::toWireType(val(*value), rvp::default_tag{});
2188+
return ValBinding::toWireType(val(*value, allow_raw_pointers()), rvp::default_tag{});
21872189
}
21882190
return ValBinding::toWireType(val::undefined(), rvp::default_tag{});
21892191
}

system/include/emscripten/val.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -364,12 +364,12 @@ class EMBIND_VISIBILITY_DEFAULT val {
364364
return val(internal::_emval_get_module_property(name));
365365
}
366366

367-
template<typename T>
368-
explicit val(T&& value) {
367+
template<typename T, typename... Policies>
368+
explicit val(T&& value, Policies...) {
369369
using namespace internal;
370-
370+
typename WithPolicies<Policies...>::template ArgTypeList<T> valueType;
371371
WireTypePack<T> argv(std::forward<T>(value));
372-
new (this) val(_emval_take_value(internal::TypeID<T>::get(), argv));
372+
new (this) val(_emval_take_value(valueType.getTypes()[0], argv));
373373
}
374374

375375
val() : val(EM_VAL(internal::_EMVAL_UNDEFINED)) {}

system/include/emscripten/wire.h

+18-4
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,12 @@ typedef const void* TYPEID;
4646
// just need a unique identifier per type and polymorphic type
4747
// identification.
4848

49+
template<typename T>
50+
static inline constexpr bool IsCanonicalized = std::is_same<T, typename std::decay<T>::type>::value;
51+
4952
template<typename T>
5053
struct CanonicalizedID {
54+
static_assert(IsCanonicalized<T>, "T should not be a reference or cv-qualified");
5155
static char c;
5256
static constexpr TYPEID get() {
5357
return &c;
@@ -65,6 +69,7 @@ struct Canonicalized {
6569
template<typename T>
6670
struct LightTypeID {
6771
static constexpr TYPEID get() {
72+
static_assert(IsCanonicalized<T>, "T should not be a reference or cv-qualified");
6873
if (has_unbound_type_names) {
6974
#if __has_feature(cxx_rtti)
7075
return &typeid(T);
@@ -83,6 +88,7 @@ struct LightTypeID {
8388

8489
template<typename T>
8590
constexpr TYPEID getLightTypeID(const T& value) {
91+
static_assert(IsCanonicalized<T>, "T should not be a reference or cv-qualified");
8692
if (has_unbound_type_names) {
8793
#if __has_feature(cxx_rtti)
8894
return &typeid(value);
@@ -137,6 +143,18 @@ struct TypeID<AllowedRawPointer<T>> {
137143
}
138144
};
139145

146+
template<typename T>
147+
struct TypeID<const T> : TypeID<T> {
148+
};
149+
150+
template<typename T>
151+
struct TypeID<T&> : TypeID<T> {
152+
};
153+
154+
template<typename T>
155+
struct TypeID<T&&> : TypeID<T> {
156+
};
157+
140158
// ExecutePolicies<>
141159

142160
template<typename... Policies>
@@ -323,10 +341,6 @@ template<typename T>
323341
struct BindingType<T&> : public BindingType<T> {
324342
};
325343

326-
template<typename T>
327-
struct BindingType<const T&> : public BindingType<T> {
328-
};
329-
330344
template<typename T>
331345
struct BindingType<T&&> {
332346
typedef typename BindingType<T>::WireType WireType;

test/embind/embind_test.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ static DummyForPointer emval_pointer_dummy(42);
5959

6060
val emval_test_instance_pointer() {
6161
DummyForPointer* p = &emval_pointer_dummy;
62-
return val(p);
62+
return val(p, allow_raw_pointers());
6363
}
6464

6565
int emval_test_value_from_instance_pointer(val v) {

test/embind/test_custom_marshal.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,8 @@ using IntWrapperIntermediate = int;
3838
// Specify custom (un)marshalling for all types satisfying IsIntWrapper.
3939
namespace emscripten {
4040
namespace internal {
41-
// remove_cv/remove_reference are required for TypeID, but not BindingType, see https://github.com/emscripten-core/emscripten/issues/7292
4241
template<typename T>
43-
struct TypeID<T, typename std::enable_if<IsIntWrapper<typename std::remove_cv<typename std::remove_reference<T>::type>::type>::value, void>::type> {
42+
struct TypeID<T, typename std::enable_if<IsIntWrapper<T>::value, void>::type> {
4443
static constexpr TYPEID get() {
4544
return TypeID<IntWrapperIntermediate>::get();
4645
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#include <emscripten/bind.h>
2+
#include <emscripten/val.h>
3+
int main() {
4+
int x = 0;
5+
emscripten::val v(&x); // int*
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#include <emscripten/bind.h>
2+
#include <emscripten/val.h>
3+
int main() {
4+
void* x = nullptr;
5+
emscripten::val v(x); // void*&
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#include <emscripten/bind.h>
2+
#include <emscripten/val.h>
3+
int main() {
4+
void* const x = nullptr;
5+
emscripten::val v(x); // void* const
6+
}
+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#include <stdio.h>
2+
#include <emscripten/val.h>
3+
4+
using namespace emscripten;
5+
6+
int main() {
7+
val Math = val::global("Math");
8+
9+
// two ways to call Math.abs
10+
printf("abs(-10): %d\n", Math.call<int>("abs", -10));
11+
printf("abs(-11): %d\n", Math["abs"](-11).as<int>());
12+
13+
// Const-qualification and references should not matter.
14+
int x = -12;
15+
const int y = -13;
16+
printf("abs(%d): %d\n", x, Math.call<int>("abs", x)); // x is deduced to int&
17+
printf("abs(%d): %d\n", y, Math.call<int>("abs", y)); // y is deduced to const int&
18+
printf("abs(-14): %d\n", Math.call<const int>("abs", -14));
19+
20+
return 0;
21+
}
+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
abs(-10): 10
2+
abs(-11): 11
3+
abs(-12): 12
4+
abs(-13): 13
5+
abs(-14): 14

test/test_core.py

+1-17
Original file line numberDiff line numberDiff line change
@@ -7296,23 +7296,7 @@ def test2():
72967296
})
72977297
def test_embind_val_basics(self, args):
72987298
self.maybe_closure()
7299-
create_file('test.cpp', r'''
7300-
#include <stdio.h>
7301-
#include <emscripten/val.h>
7302-
7303-
using namespace emscripten;
7304-
7305-
int main() {
7306-
val Math = val::global("Math");
7307-
7308-
// two ways to call Math.abs
7309-
printf("abs(-10): %d\n", Math.call<int>("abs", -10));
7310-
printf("abs(-11): %d\n", Math["abs"](-11).as<int>());
7311-
7312-
return 0;
7313-
}
7314-
''')
7315-
self.do_runf('test.cpp', 'abs(-10): 10\nabs(-11): 11', emcc_args=args)
7299+
self.do_run_in_out_file_test('embind/test_embind_val_basics.cpp', emcc_args=args)
73167300

73177301
@node_pthreads
73187302
def test_embind_basics(self):

test/test_other.py

+9
Original file line numberDiff line numberDiff line change
@@ -3308,6 +3308,15 @@ def test_embind_closure_no_dynamic_execution(self):
33083308
self.do_runf('main.cpp', '10\nok\n',
33093309
emcc_args=['--no-entry', '-lembind', '-O2', '--closure=1', '--minify=0', '--post-js=post.js'])
33103310

3311+
@parameterized({
3312+
'val_1': ['embind/test_embind_no_raw_pointers_val_1.cpp'],
3313+
'val_2': ['embind/test_embind_no_raw_pointers_val_2.cpp'],
3314+
'val_3': ['embind/test_embind_no_raw_pointers_val_3.cpp'],
3315+
})
3316+
def test_embind_no_raw_pointers(self, filename):
3317+
stderr = self.expect_fail([EMCC, '-lembind', test_file(filename)])
3318+
self.assertContained('Implicitly binding raw pointers is illegal.', stderr)
3319+
33113320
@is_slow_test
33123321
@parameterized({
33133322
'': [],

0 commit comments

Comments
 (0)