[embind] Fix return value policy for constructors. (#21981)

Use the `take_ownership` policy by default for constructors. This handles
the case where an external constructor returns by value and has no copy
constructor. This seems to be a sane default, since a constructor binding
should imply that JS is taking ownership when it's called.

Author: brendandahl

system/include/emscripten/bind.h

@@ -1388,7 +1388,7 @@ struct RegisterClassConstructor<ReturnType (*)(Args...)> {
     template <typename ClassType, typename... Policies>
     static void invoke(ReturnType (*factory)(Args...)) {
         typename WithPolicies<allow_raw_pointers, Policies...>::template ArgTypeList<ReturnType, Args...> args;
-        using ReturnPolicy = rvp::default_tag;
+        using ReturnPolicy = rvp::take_ownership;
         auto invoke = &Invoker<ReturnPolicy, ReturnType, Args...>::invoke;
         _embind_register_class_constructor(
             TypeID<ClassType>::get(),
@@ -1406,7 +1406,7 @@ struct RegisterClassConstructor<std::function<ReturnType (Args...)>> {
     template <typename ClassType, typename... Policies>
     static void invoke(std::function<ReturnType (Args...)> factory) {
         typename WithPolicies<Policies...>::template ArgTypeList<ReturnType, Args...> args;
-        using ReturnPolicy = rvp::default_tag;
+        using ReturnPolicy = rvp::take_ownership;
         auto invoke = &FunctorInvoker<ReturnPolicy, decltype(factory), ReturnType, Args...>::invoke;
         _embind_register_class_constructor(
             TypeID<ClassType>::get(),
@@ -1423,7 +1423,7 @@ struct RegisterClassConstructor<ReturnType (Args...)> {
     template <typename ClassType, typename Callable, typename... Policies>
     static void invoke(Callable& factory) {
         typename WithPolicies<Policies...>::template ArgTypeList<ReturnType, Args...> args;
-        using ReturnPolicy = rvp::default_tag;
+        using ReturnPolicy = rvp::take_ownership;
         auto invoke = &FunctorInvoker<ReturnPolicy, decltype(factory), ReturnType, Args...>::invoke;
         _embind_register_class_constructor(
             TypeID<ClassType>::get(),

test/embind/embind.test.js

@@ -1827,6 +1827,13 @@ module({
             assert.equal("foo", e.getString());
             e.delete();
         });
+
+        test("can construct class with external constructor with no copy constructor", function() {
+            var e = new cm.HasExternalConstructorNoCopy(42);
+            assert.instanceof(e, cm.HasExternalConstructorNoCopy);
+            assert.equal(42, e.getInt());
+            e.delete();
+        });
     });
 
     BaseFixture.extend("const", function() {

test/embind/embind_test.cpp

@@ -1609,6 +1609,22 @@ HasExternalConstructor* createHasExternalConstructor(const std::string& str) {
     return new HasExternalConstructor(str);
 }
 
+class HasExternalConstructorNoCopy {
+private:
+    HasExternalConstructorNoCopy(int i) : m(i) {}
+    int m;
+public:
+    HasExternalConstructorNoCopy(HasExternalConstructorNoCopy&) = delete;
+    HasExternalConstructorNoCopy(HasExternalConstructorNoCopy&&) = default;
+    int getInt() {
+        return m;
+    }
+    static HasExternalConstructorNoCopy create(int i) {
+        HasExternalConstructorNoCopy obj(i);
+        return obj;
+    }
+};
+
 template<typename T>
 class CustomSmartPtr {
 public:
@@ -2411,6 +2427,11 @@ EMSCRIPTEN_BINDINGS(tests) {
         .function("getString", &HasExternalConstructor::getString)
         ;
 
+    class_<HasExternalConstructorNoCopy>("HasExternalConstructorNoCopy")
+        .constructor(&HasExternalConstructorNoCopy::create)
+        .function("getInt", &HasExternalConstructorNoCopy::getInt)
+        ;
+
     auto HeldBySmartPtr_class = class_<HeldBySmartPtr>("HeldBySmartPtr");
     HeldBySmartPtr_class
         .smart_ptr<CustomSmartPtr<HeldBySmartPtr>>("CustomSmartPtr<HeldBySmartPtr>")