Merge commit 'refs/integration/team/platform/merge-naoqi-libqi-301'

Author: opennao

cmake/set_libqi_dependency.cmake

@@ -1,4 +1,4 @@
-set(LIBQI_VERSION 1.8.3
+set(LIBQI_VERSION 1.8.6
     CACHE STRING
     "Version of LibQi to use. If not empty, it will be checked against the \
 version in the `package.xml` file in the libqi sources.")

qi/_version.py

@@ -1 +1 @@
-__version__ = '2.0.0'
+__version__ = '2.0.1'

qipython/common.hpp

@@ -146,7 +146,7 @@ namespace detail
                                [&] {
                                  currentObject =
                                    reinterpret_borrow<object>(src);
-                                 return qi::py::unwrapAsRef(&currentObject);
+                                 return qi::py::unwrapAsRef(currentObject);
                                });
       return value.isValid();
     }

qipython/pytypes.hpp

@@ -49,8 +49,8 @@ pybind11::object unwrapValue(AnyReference val);
 /// introspected and a `qi::py::Object` is constructed out of it by executing
 /// `qi::py::toObject`.
 ///
-/// @pre `obj != nullptr && *obj`
-AnyReference unwrapAsRef(pybind11::object* obj);
+/// @pre `obj`
+AnyReference unwrapAsRef(pybind11::object& obj);
 
 void registerTypes();
 

src/pyproperty.cpp

@@ -133,7 +133,7 @@ void exportProperty(::py::module& m)
          call_guard<gil_scoped_release>(), arg(asyncArgName) = false)
     .def("setValue",
          [](detail::ProxyProperty& prop, object pyValue, bool async) {
-           AnyValue value(unwrapAsRef(&pyValue));
+           AnyValue value(unwrapAsRef(pyValue));
            gil_scoped_release unlock;
            const auto fut =
              toFuture(prop.object.setProperty(prop.propertyId, std::move(value))

src/pytypes.cpp

@@ -258,19 +258,23 @@ T* instance(Args&&... args)
   return &it->second;
 }
 
-using DanglingReferencesStorage =
+/// A 'disowned' reference would be a reference that is not expected to be automatically destroyed,
+/// meaning we would have to manually call destroy on it. Such a storage enables us to associate
+/// such references to a type-erased value so that we can later retrieve them and destroy them
+/// manually.
+using DisownedReferencesStorage =
   boost::synchronized_value<std::map<void*, std::vector<AnyReference>>>;
 
-void storeDanglingReference(void* context, AnyReference ref) noexcept
+void storeDisownedReference(void* context, AnyReference ref) noexcept
 {
-  auto* storage = instance<DanglingReferencesStorage>();
+  auto* storage = instance<DisownedReferencesStorage>();
   auto syncStorage = storage->synchronize();
   (*syncStorage)[context].push_back(ref);
 }
 
-std::vector<AnyReference> unstoreDanglingReferences(void* context) noexcept
+std::vector<AnyReference> unstoreDisownedReferences(void* context) noexcept
 {
-  auto* storage = instance<DanglingReferencesStorage>();
+  auto* storage = instance<DisownedReferencesStorage>();
   auto syncStorage = storage->synchronize();
   auto it = syncStorage->find(context);
   if (it == syncStorage->end())
@@ -282,14 +286,40 @@ std::vector<AnyReference> unstoreDanglingReferences(void* context) noexcept
   return res;
 }
 
-std::size_t destroyDanglingReferences(void* context) noexcept
+std::size_t destroyDisownedReferences(void* context) noexcept
 {
-  const auto refs = unstoreDanglingReferences(context);
+  const auto refs = unstoreDisownedReferences(context);
   for (auto ref : refs)
     ref.destroy();
   return refs.size();
 }
 
+/// Associates a value to a Python object, so that it shares its lifetime.
+template<typename T>
+AnyReference associateValueToObj(::py::object& obj, T value)
+{
+  const auto res = AnyValue::from(std::move(value)).release();
+  auto pybindObjPtr = &obj;
+  storeDisownedReference(pybindObjPtr, res);
+
+  // Create a weak reference to the target Python object that will track its lifetime and execute a
+  // callback once it is destroyed.
+  //
+  // The callback ensures that all the references associated to the target object are destroyed, so
+  // that we can leak them here and still ensure that they will be properly destroyed once needed.
+  //
+  // The callback also takes a handle to the weakref Python object itself. We need to make sure that
+  // this weakref object lives as long as the target Python object so that the callback stays
+  // attached to the lifetime of the target object. To do that, we leak the weakref Python object
+  // by releasing it, and we manually decrement the reference count in the callback.
+  auto weakref = ::py::weakref(obj, ::py::cpp_function([=](::py::handle weakref) {
+    destroyDisownedReferences(pybindObjPtr);
+    weakref.dec_ref();
+  }));
+  weakref.release();
+  return res;
+}
+
 } // namespace
 
 ::py::object unwrapValue(AnyReference val)
@@ -337,7 +367,7 @@ class ObjectInterfaceBase : public Interface
 
   void destroy(void* storage) override
   {
-    destroyDanglingReferences(storage);
+    destroyDisownedReferences(storage);
     ::py::gil_scoped_acquire lock;
     delete asObjectPtr(&storage);
   }
@@ -363,7 +393,7 @@ class DynamicInterface : public ObjectInterfaceBase<Storage, qi::DynamicTypeInte
   {
     ::py::gil_scoped_acquire lock;
     const auto obj = this->asObjectPtr(&storage);
-    return unwrapAsRef(obj);
+    return unwrapAsRef(*obj);
   }
 
   void set(void** storage, AnyReference src) override
@@ -559,9 +589,9 @@ class ListInterface : public ObjectInterfaceBase<Storage, qi::ListTypeInterface>
       const ::py::object element = list[index];
 
       auto ref = AnyReference::from(element).clone();
-      // Store the dangling reference with the list as a context instead of the
+      // Store the disowned reference with the list as a context instead of the
       // iterator because the reference might outlive the iterator.
-      storeDanglingReference(listStorage, ref);
+      storeDisownedReference(listStorage, ref);
       return ref;
     }
 
@@ -578,7 +608,7 @@ class ListInterface : public ObjectInterfaceBase<Storage, qi::ListTypeInterface>
     void* clone(void* storage) override { return DefaultImpl::clone(storage); }
     void destroy(void* storage) override
     {
-      destroyDanglingReferences(storage);
+      destroyDisownedReferences(storage);
       return DefaultImpl::destroy(storage);
     }
     const TypeInfo& info() override { return DefaultImpl::info(); }
@@ -657,9 +687,9 @@ class DictInterface: public ObjectInterfaceBase<Storage, qi::MapTypeInterface>
       const auto element = ::py::reinterpret_borrow<::py::object>(it->second);
       const auto keyElementPair = std::make_pair(key, element);
       auto ref = AnyReference::from(keyElementPair).clone();
-      // Store the dangling reference with the list as a context instead of the
+      // Store the disowned reference with the list as a context instead of the
       // iterator because the reference might outlive the iterator.
-      storeDanglingReference(dictStorage, ref);
+      storeDisownedReference(dictStorage, ref);
       return ref;
     }
 
@@ -676,7 +706,7 @@ class DictInterface: public ObjectInterfaceBase<Storage, qi::MapTypeInterface>
     void* clone(void* storage) override { return DefaultImpl::clone(storage); }
     void destroy(void* storage) override
     {
-      destroyDanglingReferences(storage);
+      destroyDisownedReferences(storage);
       return DefaultImpl::destroy(storage);
     }
 
@@ -749,78 +779,87 @@ class DictInterface: public ObjectInterfaceBase<Storage, qi::MapTypeInterface>
     }
 
     auto ref = AnyReference::from(value).clone();
-    // Store the dangling reference with the list as a context instead of the
+    // Store the disowned reference with the list as a context instead of the
     // iterator because the reference might outlive the iterator.
-    storeDanglingReference(storage, ref);
+    storeDisownedReference(storage, ref);
     return ref;
   }
 };
 
 } // namespace types
 
-AnyReference unwrapAsRef(pybind11::object* obj)
+AnyReference unwrapAsRef(pybind11::object& obj)
 {
-  QI_ASSERT_NOT_NULL(obj);
-  QI_ASSERT_TRUE(*obj);
+  QI_ASSERT_TRUE(obj);
 
   ::py::gil_scoped_acquire lock;
 
-  if (obj->is_none())
+  if (obj.is_none())
     // The "void" value in AnyValue has no storage, so we can just release it
     // without risk of a leak.
     return AnyValue::makeVoid().release();
 
   if (   ::py::isinstance<::py::ellipsis>(*obj)
-      || PyComplex_CheckExact(obj->ptr())
+      || PyComplex_CheckExact(obj.ptr())
       || ::py::isinstance<::py::memoryview>(*obj)
       || ::py::isinstance<::py::slice>(*obj)
       || ::py::isinstance<::py::module>(*obj))
   {
     throw std::runtime_error("The Python type " +
-                             std::string(::py::str(obj->get_type())) +
+                             std::string(::py::str(obj.get_type())) +
                              " is not handled");
   }
 
-  if (PyLong_CheckExact(obj->ptr()))
-    return AnyReference(instance<types::IntInterface<::py::object>>(), obj);
+  // Pointer to the libpython C library python object.
+  const auto pyObjPtr = obj.ptr();
+
+  // Pointer to the pybind C++ library python object.
+  const auto pybindObjPtr = &obj;
+
+  if (PyLong_CheckExact(pyObjPtr))
+    return AnyReference(instance<types::IntInterface<::py::object>>(), pybindObjPtr);
 
-  if (PyFloat_CheckExact(obj->ptr()))
-    return AnyReference(instance<types::FloatInterface<::py::object>>(), obj);
+  if (PyFloat_CheckExact(pyObjPtr))
+    return AnyReference(instance<types::FloatInterface<::py::object>>(), pybindObjPtr);
 
-  if (PyBool_Check(obj->ptr()))
-    return AnyReference(instance<types::BoolInterface<::py::object>>(), obj);
+  if (PyBool_Check(pyObjPtr))
+    return AnyReference(instance<types::BoolInterface<::py::object>>(), pybindObjPtr);
 
-  if (PyUnicode_CheckExact(obj->ptr()))
-    return AnyReference(instance<types::StrInterface<::py::object>>(), obj);
+  if (PyUnicode_CheckExact(pyObjPtr))
+    return AnyReference(instance<types::StrInterface<::py::object>>(), pybindObjPtr);
 
-  if (PyBytes_CheckExact(obj->ptr()) || PyByteArray_CheckExact(obj->ptr()))
-    return AnyReference(instance<types::StringBufferInterface<::py::object>>(), obj);
+  if (PyBytes_CheckExact(pyObjPtr) || PyByteArray_CheckExact(pyObjPtr))
+    return AnyReference(instance<types::StringBufferInterface<::py::object>>(), pybindObjPtr);
 
-  if (PyTuple_CheckExact(obj->ptr()))
+  if (PyTuple_CheckExact(pyObjPtr))
     return AnyReference(instance<types::StructuredIterableInterface<::py::object>>(
-                          ::py::tuple(*obj).size()),
-                        obj);
+                          ::py::tuple(obj).size()),
+                        pybindObjPtr);
 
   // Checks if it is AnySet, meaning it can be a set or a frozenset.
-  if (PyAnySet_CheckExact(obj->ptr()))
+  if (PyAnySet_CheckExact(pyObjPtr))
     return AnyReference(instance<types::StructuredIterableInterface<::py::object>>(
-                          ::py::set(*obj).size()),
-                        obj);
-
-  if (PyList_CheckExact(obj->ptr()))
-    return AnyReference(instance<types::ListInterface<::py::object, ::py::list>>(), obj);
-
-  if (PyDict_CheckExact(obj->ptr()))
-    return AnyReference(instance<types::DictInterface<::py::object>>(), obj);
-
-  auto qiObj = py::toObject(*obj);
-  const auto res = AnyValue::from(std::move(qiObj)).release();
-  storeDanglingReference(obj, res);
-  ::py::weakref(*obj, ::py::cpp_function([=](::py::handle ref) {
-    destroyDanglingReferences(obj);
-    ref.dec_ref(); // release the reference to the weak reference itself.
-  })).release(); // leak the weak reference, it will be released by the callback.
-  return res;
+                          ::py::set(obj).size()),
+                        pybindObjPtr);
+
+  if (PyList_CheckExact(pyObjPtr) || PyDictViewSet_Check(pyObjPtr) || PyDictValues_Check(pyObjPtr))
+    return AnyReference(instance<types::ListInterface<::py::object, ::py::list>>(), pybindObjPtr);
+
+  if (PyDict_CheckExact(pyObjPtr))
+    return AnyReference(instance<types::DictInterface<::py::object>>(), pybindObjPtr);
+
+  // At the moment in libqi, the `LogLevel` type is not registered in the qi type system. If we use
+  // `AnyValue::from` with a `LogLevel` value, we get a value with a dummy type that cannot be set
+  // or read. Furthermore, when registered with the `QI_TYPE_ENUM` macro, enumeration types are
+  // treated as `int` values. This forces us to cast it here as an `int` to try to stay compatible.
+  if (::py::isinstance<LogLevel>(obj))
+  {
+    const auto logLevel = obj.cast<LogLevel>();
+    const auto logLevelValue = static_cast<int>(logLevel);
+    return associateValueToObj(obj, logLevelValue);
+  }
+
+  return associateValueToObj(obj, py::toObject(obj));
 }
 
 void registerTypes()

tests/test_types.cpp

@@ -401,3 +401,50 @@ TEST_F(TypePassing, ReverseDict)
   };
   EXPECT_TRUE(getService().call<bool>("func", expected));
 }
+
+TEST_F(TypePassing, LogLevel)
+{
+  exec(
+      "class TestService:\n"
+      "    def func(self):\n"
+      "        return qi.LogLevel.Info\n"
+      );
+  registerService();
+  EXPECT_EQ(qi::LogLevel_Info, getService().call<int>("func"));
+}
+
+TEST_F(TypePassing, DictViews)
+{
+  exec(
+      "class TestService:\n"
+      "    def func(self, kind):\n"
+      "        d = dict(one=1, two=2)\n"
+      "        if kind == 0:\n"
+      "            return d.keys()\n"
+      "        if kind == 1:\n"
+      "            return d.items()\n"
+      "        return d.values()\n"
+      );
+  registerService();
+
+  { // Keys
+    using Keys = std::set<std::string>;
+    const Keys expected{ "one", "two" };
+    EXPECT_EQ(expected, getService().call<Keys>("func", 0));
+  }
+
+  { // Items
+    using Items = std::set<std::pair<std::string, int>>;
+    const Items expected{ { "one", 1 }, { "two", 2 } };
+    EXPECT_EQ(expected, getService().call<Items>("func", 1));
+  }
+
+
+  { // Values
+    using Values = std::vector<int>;
+    const Values expected{ 1, 2 };
+    auto values = getService().call<Values>("func", 2);
+    std::sort(values.begin(), values.end());
+    EXPECT_EQ(expected, values);
+  }
+}