types: Fix dict iterator dereference values destruction. #SW-7582

Author: nyibbang

src/pytypes.cpp

@@ -595,20 +595,13 @@ class ListInterface : public ObjectInterfaceBase<Storage, qi::ListTypeInterface>
 
     using DefaultImpl = DefaultTypeImplMethods<Iterator, TypeByPointerPOD<Iterator>>;
 
-    void* initializeStorage(void* ptr = nullptr) override
-    {
-      return DefaultImpl::initializeStorage(ptr);
-    }
-
+    void* initializeStorage(void* ptr = nullptr) override { return DefaultImpl::initializeStorage(ptr); }
     void* clone(void* storage) override { return DefaultImpl::clone(storage); }
-    void destroy(void* storage) override
-    {
-      destroyDisownedReferences(storage);
-      return DefaultImpl::destroy(storage);
-    }
+    void destroy(void* storage) override { return DefaultImpl::destroy(storage); }
     const TypeInfo& info() override { return DefaultImpl::info(); }
     void* ptrFromStorage(void** s) override { return DefaultImpl::ptrFromStorage(s); }
     bool less(void* a, void* b) override { return DefaultImpl::less(a, b); }
+
     Iterator* asIterPtr(void** storage) { return static_cast<Iterator*>(ptrFromStorage(storage)); }
     Iterator& asIter(void** storage) { return *asIterPtr(storage); }
   };
@@ -680,34 +673,27 @@ class DictInterface: public ObjectInterfaceBase<Storage, qi::MapTypeInterface>
       std::advance(it, index);
       const auto key = ::py::reinterpret_borrow<::py::object>(it->first);
       const auto element = ::py::reinterpret_borrow<::py::object>(it->second);
-      const auto keyElementPair = std::make_pair(key, element);
-      auto ref = AnyReference::from(keyElementPair).clone();
+      auto keyRef = AnyReference::from(key);
+      auto elementRef = AnyReference::from(element);
+      auto pairRef = makeGenericTuple({keyRef, elementRef});
       // Store the disowned reference with the list as a context instead of the
       // iterator because the reference might outlive the iterator.
-      storeDisownedReference(dictStorage, ref);
-      return ref;
+      storeDisownedReference(dictStorage, pairRef);
+      return pairRef;
     }
 
     void next(void** storage) override { ++asIter(storage).second; }
     bool equals(void* s1, void* s2) override { return asIter(&s1) == asIter(&s2); }
 
     using DefaultImpl = DefaultTypeImplMethods<Iterator, TypeByPointerPOD<Iterator>>;
 
-    void* initializeStorage(void* ptr = nullptr) override
-    {
-      return DefaultImpl::initializeStorage(ptr);
-    }
-
+    void* initializeStorage(void* ptr = nullptr) override { return DefaultImpl::initializeStorage(ptr); }
     void* clone(void* storage) override { return DefaultImpl::clone(storage); }
-    void destroy(void* storage) override
-    {
-      destroyDisownedReferences(storage);
-      return DefaultImpl::destroy(storage);
-    }
-
+    void destroy(void* storage) override { return DefaultImpl::destroy(storage); }
     const TypeInfo& info() override { return DefaultImpl::info(); }
     void* ptrFromStorage(void** s) override { return DefaultImpl::ptrFromStorage(s); }
     bool less(void* a, void* b) override { return DefaultImpl::less(a, b); }
+
     Iterator* asIterPtr(void** storage) { return static_cast<Iterator*>(ptrFromStorage(storage)); }
     Iterator& asIter(void** storage) { return *asIterPtr(storage); }
   };

tests/test_types.cpp

@@ -387,20 +387,40 @@ TEST_F(TypePassing, Recursive)
   }
 }
 
+TEST_F(TypePassing, ReverseList)
+{
+  exec(
+      "class TestService:\n"
+      "    def func(self, list):\n"
+      // Test the iterator interface.
+      "        for value in list:\n"
+      "            assert(isinstance(value, str))\n"
+      "        assert(list == ['hello', 'world'])\n"
+      );
+  registerService();
+  const std::vector<const char*> list {"hello", "world"};
+  getService().call<void>("func", list);
+}
+
+
 TEST_F(TypePassing, ReverseDict)
 {
   exec(
       "class TestService:\n"
       "    def func(self, dict):\n"
-      "        return dict == {'one' : 1, 'two' : 2, 'three' : 3}\n"
+      // Test the iterator interface.
+      "        for key, value in dict.items():\n"
+      "            assert(isinstance(key, str))\n"
+      "            assert(isinstance(value, int))\n"
+      "        assert(dict == {'one' : 1, 'two' : 2, 'three' : 3})\n"
       );
   registerService();
-  const std::map<std::string, int> expected {
+  const std::map<std::string, int> dict {
     {"one", 1},
     {"two", 2},
     {"three", 3},
   };
-  EXPECT_TRUE(getService().call<bool>("func", expected));
+  getService().call<void>("func", dict);
 }
 
 TEST_F(TypePassing, LogLevel)