Skip to content

Commit 09b765f

Browse files
brendandahlbrendandahl
authored
[embind] Return value policy support for properties. (#21935)
All the return polices now also work with `property` bindings. Using `return_value_policy::reference()` is especially helpful for binding child objects of a class so they don't always create copies and potentially leak. Fixes #6402, #17573
1 parent b1422fc commit 09b765f

File tree

4 files changed

+289
-46
lines changed

4 files changed

+289
-46
lines changed

‎ChangeLog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ See docs/process.md for more on how version tagging works.
2929
upstream LLVM change (https://github.com/llvm/llvm-project/pull/93261).
3030
- Emscripten now uses `strftime` from musl rather than using a custom
3131
JavaScript implementation. (#21379)
32+
- Embind now supports return value policies for properties.
3233

3334
3.1.61 - 05/31/24
3435
-----------------

‎site/source/docs/porting/connecting_cpp_and_javascript/embind.rst

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,8 @@ moves between languages. To make object ownership more explicit, *embind*
360360
supports smart pointers and return value policies. Return value
361361
polices dictate what happens to a C++ object when it is returned to JavaScript.
362362

363-
To use a return value policy, pass the desired policy into function or method
364-
bindings. For example:
363+
To use a return value policy, pass the desired policy into function, method, or
364+
property bindings. For example:
365365

366366
.. code:: cpp
367367
@@ -826,6 +826,71 @@ To expose a C++ :cpp:func:`constant` to JavaScript, simply write:
826826

827827
.. _embind-memory-view:
828828

829+
Class Properties
830+
================
831+
832+
.. warning:: By default ``property()`` bindings to objects use
833+
``return_value_policy::copy`` which can very easily lead to memory leaks
834+
since each access to the property will create a new object that must be
835+
deleted. Alternatively, use ``return_value_policy::reference``, so a new
836+
object is not allocated and changes to the object will be reflected in the
837+
original object.
838+
839+
Class properties can be defined several ways as seen below.
840+
841+
.. code:: cpp
842+
843+
struct Point {
844+
float x;
845+
float y;
846+
};
847+
848+
struct Person {
849+
Point location;
850+
Point getLocation() const { // Note: const is required on getters
851+
return location;
852+
}
853+
void setLocation(Point p) {
854+
location = p;
855+
}
856+
};
857+
858+
EMSCRIPTEN_BINDINGS(xxx) {
859+
class_<Person>("Person")
860+
.constructor<>()
861+
// Bind directly to a class member with automatically generated getters/setters using a
862+
// reference return policy so the object does not need to be deleted JS.
863+
.property("location", &Person::location, return_value_policy::reference())
864+
// Same as above, but this will return a copy and the object must be deleted or it will
865+
// leak!
866+
.property("locationCopy", &Person::location)
867+
// Bind using a only getter method for read only access.
868+
.property("readOnlyLocation", &Person::getLocation, return_value_policy::reference())
869+
// Bind using a getter and setter method.
870+
.property("getterAndSetterLocation", &Person::getLocation, &Person::setLocation,
871+
return_value_policy::reference());
872+
class_<Point>("Point")
873+
.property("x", &Point::x)
874+
.property("y", &Point::y);
875+
}
876+
877+
int main() {
878+
EM_ASM(
879+
let person = new Module.Person();
880+
person.location.x = 42;
881+
console.log(person.location.x); // 42
882+
let locationCopy = person.locationCopy;
883+
// This is a copy so the original person's location will not be updated.
884+
locationCopy.x = 99;
885+
console.log(locationCopy.x); // 99
886+
// Important: delete any copies!
887+
locationCopy.delete();
888+
console.log(person.readOnlyLocation.x); // 42
889+
console.log(person.getterAndSetterLocation.x); // 42
890+
person.delete();
891+
);
892+
}
893+
829894
Memory views
830895
============
831896

‎system/include/emscripten/bind.h

Lines changed: 127 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,13 @@ struct async {
331331
};
332332
};
333333

334+
struct pure_virtual {
335+
template<typename InputType, int Index>
336+
struct Transform {
337+
typedef InputType type;
338+
};
339+
};
340+
334341
namespace return_value_policy {
335342

336343
struct take_ownership : public allow_raw_pointers {};
@@ -340,6 +347,49 @@ struct reference : public allow_raw_pointers {};
340347

341348
namespace internal {
342349

350+
template<typename... Policies>
351+
struct isPolicy;
352+
353+
template<typename... Rest>
354+
struct isPolicy<return_value_policy::take_ownership, Rest...> {
355+
static constexpr bool value = true;
356+
};
357+
358+
template<typename... Rest>
359+
struct isPolicy<return_value_policy::reference, Rest...> {
360+
static constexpr bool value = true;
361+
};
362+
363+
template<typename... Rest>
364+
struct isPolicy<emscripten::async, Rest...> {
365+
static constexpr bool value = true;
366+
};
367+
368+
template <typename T, typename... Rest>
369+
struct isPolicy<emscripten::allow_raw_pointer<T>, Rest...> {
370+
static constexpr bool value = true;
371+
};
372+
373+
template<typename... Rest>
374+
struct isPolicy<allow_raw_pointers, Rest...> {
375+
static constexpr bool value = true;
376+
};
377+
378+
template<typename... Rest>
379+
struct isPolicy<emscripten::pure_virtual, Rest...> {
380+
static constexpr bool value = true;
381+
};
382+
383+
template<typename T, typename... Rest>
384+
struct isPolicy<T, Rest...> {
385+
static constexpr bool value = isPolicy<Rest...>::value;
386+
};
387+
388+
template<>
389+
struct isPolicy<> {
390+
static constexpr bool value = false;
391+
};
392+
343393
template<typename ReturnType, typename... Rest>
344394
struct GetReturnValuePolicy {
345395
using tag = rvp::default_tag;
@@ -691,12 +741,12 @@ struct MemberAccess {
691741
typedef internal::BindingType<MemberType> MemberBinding;
692742
typedef typename MemberBinding::WireType WireType;
693743

694-
template<typename ClassType>
744+
template<typename ClassType, typename ReturnPolicy = rvp::default_tag>
695745
static WireType getWire(
696746
const MemberPointer& field,
697-
const ClassType& ptr
747+
ClassType& ptr
698748
) {
699-
return MemberBinding::toWireType(ptr.*field, rvp::default_tag{});
749+
return MemberBinding::toWireType(ptr.*field, ReturnPolicy{});
700750
}
701751

702752
template<typename ClassType>
@@ -748,9 +798,9 @@ struct GetterPolicy<GetterReturnType (GetterThisType::*)() const> {
748798
typedef internal::BindingType<ReturnType> Binding;
749799
typedef typename Binding::WireType WireType;
750800

751-
template<typename ClassType>
801+
template<typename ClassType, typename ReturnPolicy>
752802
static WireType get(const Context& context, const ClassType& ptr) {
753-
return Binding::toWireType((ptr.*context)(), rvp::default_tag{});
803+
return Binding::toWireType((ptr.*context)(), ReturnPolicy{});
754804
}
755805

756806
static void* getContext(Context context) {
@@ -772,9 +822,9 @@ struct GetterPolicy<GetterReturnType (*)(const GetterThisType&)> {
772822
typedef internal::BindingType<ReturnType> Binding;
773823
typedef typename Binding::WireType WireType;
774824

775-
template<typename ClassType>
825+
template<typename ClassType, typename ReturnPolicy>
776826
static WireType get(const Context& context, const ClassType& ptr) {
777-
return Binding::toWireType(context(ptr), rvp::default_tag{});
827+
return Binding::toWireType(context(ptr), ReturnPolicy{});
778828
}
779829

780830
static void* getContext(Context context) {
@@ -790,9 +840,9 @@ struct GetterPolicy<std::function<GetterReturnType(const GetterThisType&)>> {
790840
typedef internal::BindingType<ReturnType> Binding;
791841
typedef typename Binding::WireType WireType;
792842

793-
template<typename ClassType>
843+
template<typename ClassType, typename ReturnPolicy>
794844
static WireType get(const Context& context, const ClassType& ptr) {
795-
return Binding::toWireType(context(ptr), rvp::default_tag{});
845+
return Binding::toWireType(context(ptr), ReturnPolicy{});
796846
}
797847

798848
static void* getContext(const Context& context) {
@@ -808,9 +858,9 @@ struct GetterPolicy<PropertyTag<Getter, GetterReturnType>> {
808858
typedef internal::BindingType<ReturnType> Binding;
809859
typedef typename Binding::WireType WireType;
810860

811-
template<typename ClassType>
861+
template<typename ClassType, typename ReturnPolicy>
812862
static WireType get(const Context& context, const ClassType& ptr) {
813-
return Binding::toWireType(context(ptr), rvp::default_tag{});
863+
return Binding::toWireType(context(ptr), ReturnPolicy{});
814864
}
815865

816866
static void* getContext(const Context& context) {
@@ -899,6 +949,18 @@ struct SetterPolicy<PropertyTag<Setter, SetterArgumentType>> {
899949
}
900950
};
901951

952+
// Helper available in C++14.
953+
template <bool _Test, class _T1, class _T2>
954+
using conditional_t = typename std::conditional<_Test, _T1, _T2>::type;
955+
956+
// Conjunction is available in C++17
957+
template<class...> struct conjunction : std::true_type {};
958+
template<class B1> struct conjunction<B1> : B1 {};
959+
template<class B1, class... Bn>
960+
struct conjunction<B1, Bn...>
961+
: conditional_t<bool(B1::value), conjunction<Bn...>, B1> {};
962+
963+
902964
class noncopyable {
903965
protected:
904966
noncopyable() {}
@@ -980,7 +1042,7 @@ class value_array : public internal::noncopyable {
9801042
typedef GetterPolicy<Getter> GP;
9811043
typedef SetterPolicy<Setter> SP;
9821044

983-
auto g = &GP::template get<ClassType>;
1045+
auto g = &GP::template get<ClassType, rvp::default_tag>;
9841046
auto s = &SP::template set<ClassType>;
9851047

9861048
_embind_register_value_array_element(
@@ -1106,7 +1168,7 @@ class value_object : public internal::noncopyable {
11061168
typedef GetterPolicy<Getter> GP;
11071169
typedef SetterPolicy<Setter> SP;
11081170

1109-
auto g = &GP::template get<ClassType>;
1171+
auto g = &GP::template get<ClassType, rvp::default_tag>;
11101172
auto s = &SP::template set<ClassType>;
11111173

11121174
_embind_register_value_object_field(
@@ -1346,13 +1408,6 @@ val wrapped_extend(const std::string& name, const val& properties) {
13461408

13471409
} // end namespace internal
13481410

1349-
struct pure_virtual {
1350-
template<typename InputType, int Index>
1351-
struct Transform {
1352-
typedef InputType type;
1353-
};
1354-
};
1355-
13561411
namespace internal {
13571412

13581413
template<typename... Policies>
@@ -1741,15 +1796,24 @@ class class_ {
17411796
return *this;
17421797
}
17431798

1744-
template<typename FieldType, typename = typename std::enable_if<!std::is_function<FieldType>::value>::type>
1745-
EMSCRIPTEN_ALWAYS_INLINE const class_& property(const char* fieldName, const FieldType ClassType::*field) const {
1799+
template<
1800+
typename FieldType,
1801+
typename... Policies,
1802+
// Prevent the template from wrongly matching the getter function
1803+
// overload.
1804+
typename = typename std::enable_if<
1805+
!std::is_function<FieldType>::value &&
1806+
internal::conjunction<internal::isPolicy<Policies>...>::value>::type>
1807+
EMSCRIPTEN_ALWAYS_INLINE const class_& property(const char* fieldName, const FieldType ClassType::*field, Policies...) const {
17461808
using namespace internal;
1809+
using ReturnPolicy = GetReturnValuePolicy<FieldType, Policies...>::tag;
1810+
typename WithPolicies<Policies...>::template ArgTypeList<FieldType> returnType;
17471811

1748-
auto getter = &MemberAccess<ClassType, FieldType>::template getWire<ClassType>;
1812+
auto getter = &MemberAccess<ClassType, FieldType>::template getWire<ClassType, ReturnPolicy>;
17491813
_embind_register_class_property(
17501814
TypeID<ClassType>::get(),
17511815
fieldName,
1752-
TypeID<FieldType>::get(),
1816+
returnType.getTypes()[0],
17531817
getSignature(getter),
17541818
reinterpret_cast<GenericFunction>(getter),
17551819
getContext(field),
@@ -1760,40 +1824,57 @@ class class_ {
17601824
return *this;
17611825
}
17621826

1763-
template<typename FieldType, typename = typename std::enable_if<!std::is_function<FieldType>::value>::type>
1764-
EMSCRIPTEN_ALWAYS_INLINE const class_& property(const char* fieldName, FieldType ClassType::*field) const {
1827+
template<
1828+
typename FieldType,
1829+
typename... Policies,
1830+
// Prevent the template from wrongly matching the getter function
1831+
// overload.
1832+
typename = typename std::enable_if<
1833+
!std::is_function<FieldType>::value &&
1834+
internal::conjunction<internal::isPolicy<Policies>...>::value>::type>
1835+
EMSCRIPTEN_ALWAYS_INLINE const class_& property(const char* fieldName, FieldType ClassType::*field, Policies...) const {
17651836
using namespace internal;
1837+
using ReturnPolicy = GetReturnValuePolicy<FieldType, Policies...>::tag;
1838+
typename WithPolicies<Policies...>::template ArgTypeList<FieldType> returnType;
17661839

1767-
auto getter = &MemberAccess<ClassType, FieldType>::template getWire<ClassType>;
1840+
auto getter = &MemberAccess<ClassType, FieldType>::template getWire<ClassType, ReturnPolicy>;
17681841
auto setter = &MemberAccess<ClassType, FieldType>::template setWire<ClassType>;
17691842
_embind_register_class_property(
17701843
TypeID<ClassType>::get(),
17711844
fieldName,
1772-
TypeID<FieldType>::get(),
1845+
returnType.getTypes()[0],
17731846
getSignature(getter),
17741847
reinterpret_cast<GenericFunction>(getter),
17751848
getContext(field),
1776-
TypeID<FieldType>::get(),
1849+
returnType.getTypes()[0],
17771850
getSignature(setter),
17781851
reinterpret_cast<GenericFunction>(setter),
17791852
getContext(field));
17801853
return *this;
17811854
}
17821855

1783-
template<typename PropertyType = internal::DeduceArgumentsTag, typename Getter>
1784-
EMSCRIPTEN_ALWAYS_INLINE const class_& property(const char* fieldName, Getter getter) const {
1856+
template<
1857+
typename PropertyType = internal::DeduceArgumentsTag,
1858+
typename Getter,
1859+
typename... Policies,
1860+
// Prevent the template from wrongly matching the getter/setter overload
1861+
// of this function.
1862+
typename = typename std::enable_if<
1863+
internal::conjunction<internal::isPolicy<Policies>...>::value>::type>
1864+
EMSCRIPTEN_ALWAYS_INLINE const class_& property(const char* fieldName, Getter getter, Policies...) const {
17851865
using namespace internal;
17861866

17871867
typedef GetterPolicy<
17881868
typename std::conditional<std::is_same<PropertyType, internal::DeduceArgumentsTag>::value,
17891869
Getter,
17901870
PropertyTag<Getter, PropertyType>>::type> GP;
1791-
1792-
auto gter = &GP::template get<ClassType>;
1871+
using ReturnPolicy = GetReturnValuePolicy<typename GP::ReturnType, Policies...>::tag;
1872+
auto gter = &GP::template get<ClassType, ReturnPolicy>;
1873+
typename WithPolicies<Policies...>::template ArgTypeList<typename GP::ReturnType> returnType;
17931874
_embind_register_class_property(
17941875
TypeID<ClassType>::get(),
17951876
fieldName,
1796-
TypeID<typename GP::ReturnType>::get(),
1877+
returnType.getTypes()[0],
17971878
getSignature(gter),
17981879
reinterpret_cast<GenericFunction>(gter),
17991880
GP::getContext(getter),
@@ -1804,9 +1885,18 @@ class class_ {
18041885
return *this;
18051886
}
18061887

1807-
template<typename PropertyType = internal::DeduceArgumentsTag, typename Getter, typename Setter>
1808-
EMSCRIPTEN_ALWAYS_INLINE const class_& property(const char* fieldName, Getter getter, Setter setter) const {
1888+
template<
1889+
typename PropertyType = internal::DeduceArgumentsTag,
1890+
typename Getter,
1891+
typename Setter,
1892+
typename... Policies,
1893+
// Similar to the other variadic property overloads this can greedily
1894+
// match the wrong overload so we need to ensure the setter is not a
1895+
// policy argument.
1896+
typename = typename std::enable_if<!internal::isPolicy<Setter>::value>::type>
1897+
EMSCRIPTEN_ALWAYS_INLINE const class_& property(const char* fieldName, Getter getter, Setter setter, Policies...) const {
18091898
using namespace internal;
1899+
using ReturnPolicy = GetReturnValuePolicy<PropertyType, Policies...>::tag;
18101900

18111901
typedef GetterPolicy<
18121902
typename std::conditional<std::is_same<PropertyType, internal::DeduceArgumentsTag>::value,
@@ -1818,7 +1908,7 @@ class class_ {
18181908
PropertyTag<Setter, PropertyType>>::type> SP;
18191909

18201910

1821-
auto gter = &GP::template get<ClassType>;
1911+
auto gter = &GP::template get<ClassType, ReturnPolicy>;
18221912
auto ster = &SP::template set<ClassType>;
18231913

18241914
_embind_register_class_property(

0 commit comments

Comments
 (0)