Introducing proper deallocation ways for CPPInstance

CMakeLists.txt

@@ -11,8 +11,8 @@ include(GNUInstallDirs)
 # This option won't make a lot of sense since we only ship the shared library in site-packages
 # Perhaps this should permanently be OFF and users can build their own CppInterOp if they want to run the tests?
 option(CPPJIT_ENABLE_CPPINTEROP_TESTS "enable CppInterOp tests" OFF)
-set(CPPINTEROP_GIT_REPOSITORY "https://github.com/compiler-research/CppInterOp.git" CACHE STRING "")
-set(CPPINTEROP_GIT_TAG "9802d61921ad5688ae42e4e628d754fc1192244d" CACHE STRING "")
+set(CPPINTEROP_GIT_REPOSITORY "https://github.com/keremsahn/CppInterOp.git" CACHE STRING "")
+set(CPPINTEROP_GIT_TAG "attr-design" CACHE STRING "")
 set(CPPINTEROP_SOURCE_DIR "" CACHE PATH
     "Override default CppInterOp built by ExternalProject_Add, with a path to local CppInterOp source")
 

python/cppjit/__init__.py

@@ -48,6 +48,7 @@
     "add_library_path",  # add a path to search for libraries
     "add_autoload_map",  # explicitly include an autoload map
     "set_debug",  # enable/disable debug output
+    "use_alloc_analyzer",  # enable/disable memory ownership analyzer
 ]
 
 import ctypes
@@ -397,6 +398,11 @@ def set_debug(enable=True):
     gbl.Cpp.EnableDebugOutput(enable)
 
 
+def use_alloc_analyzer(enable=True):
+    """Enable/disable memory ownership analyzer"""
+    _backend.UseAllocAnalyzer(enable)
+
+
 def _get_name(tt):
     if isinstance(tt, str):
         return tt

src/cpyrt/CPPInstance.cxx

@@ -16,6 +16,7 @@ using namespace cppjit;
 
 // Standard
 #include <algorithm>
+#include <cstdlib>
 #include <sstream>
 
 //- data _____________________________________________________________________
@@ -226,7 +227,17 @@ void cpyrt::op_dealloc_nofree(CPPInstance* pyobj) {
     if (pyobj->fFlags & CPPInstance::kIsValue) {
       interop::CallDestructor(klass, cppobj);
       interop::Deallocate(klass, cppobj);
-    } else
+    } else if (pyobj->fFlags & CPPInstance::kIsMalloc)
+      std::free(cppobj);
+    else if (pyobj->fFlags & CPPInstance::kIsNoConstruct) {
+      if (pyobj->fFlags & CPPInstance::kIsArrayAlloc)
+        ::operator delete[](cppobj);
+      else
+        ::operator delete(cppobj);
+    } else if (pyobj->fFlags & CPPInstance::kIsArrayAlloc)
+      interop::Destruct(klass, cppobj, 1);
+    // Default case: just kIsOwner set in all of memory-ownership flags
+    else
       interop::Destruct(klass, cppobj);
   }
   cppobj = nullptr;
@@ -954,10 +965,30 @@ static int op_setownership(CPPInstance* pyobj, PyObject* value, void*) {
   return 0;
 }
 
+// Added for testing purposes
+//-----------------------------------------------------------------------------
+static PyObject* op_get_array_alloc(CPPInstance* pyobj, void*) {
+  return PyBool_FromLong((long)(pyobj->fFlags & CPPInstance::kIsArrayAlloc));
+}
+//-----------------------------------------------------------------------------
+static PyObject* op_get_no_construct(CPPInstance* pyobj, void*) {
+  return PyBool_FromLong((long)(pyobj->fFlags & CPPInstance::kIsNoConstruct));
+}
+//-----------------------------------------------------------------------------
+static PyObject* op_get_malloc(CPPInstance* pyobj, void*) {
+  return PyBool_FromLong((long)(pyobj->fFlags & CPPInstance::kIsMalloc));
+}
 //-----------------------------------------------------------------------------
 static PyGetSetDef op_getset[] = {
     {(char*)"__python_owns__", (getter)op_getownership, (setter)op_setownership,
      (char*)"If true, python manages the life time of this object", nullptr},
+    {(char*)"__is_array_alloc__", (getter)op_get_array_alloc, nullptr,
+     (char*)"If true, the object was allocated with new[]/operator new[]",
+     nullptr},
+    {(char*)"__is_no_construct__", (getter)op_get_no_construct, nullptr,
+     (char*)"If true, the memory is raw: no constructor was run", nullptr},
+    {(char*)"__is_malloc__", (getter)op_get_malloc, nullptr,
+     (char*)"If true, the object was allocated with malloc", nullptr},
     {(char*)nullptr, nullptr, nullptr, nullptr, nullptr}};
 
 //= cpyrt type number stubs to allow dynamic overrides =====================

src/cpyrt/CPPInstance.h

@@ -27,21 +27,25 @@ typedef std::vector<std::pair<ptrdiff_t, PyObject*>> CI_DatamemberCache_t;
 class CPPInstance {
 public:
   enum EFlags {
-    kDefault = 0x0000,
-    kNoWrapConv = 0x0001,  // use type as-is (eg. no smart ptr wrap)
-    kIsOwner = 0x0002,     // Python instance owns C++ object/memory
-    kIsExtended = 0x0004,  // has extended data
-    kIsValue = 0x0008,     // was created from a by-value return
-    kIsReference = 0x0010, // represents one indirection
-    kIsArray = 0x0020,     // represents an array of objects
-    kIsSmartPtr = 0x0040,  // is or embeds a smart pointer
-    kIsPtrPtr = 0x0080,    // represents two indirections
-    kIsRValue = 0x0100,    // can be used as an r-value
-    kIsLValue = 0x0200,    // can be used as an l-value
-    kNoMemReg = 0x0400,    // do not register with memory regulator
-    kIsRegulated = 0x0800, // is registered with memory regulator
-    kIsActual = 0x1000,    // has been downcasted to actual type
-    kHasLifeLine = 0x2000, // has a life line set
+    kDefault = 0x00000,
+    kNoWrapConv = 0x00001,   // use type as-is (eg. no smart ptr wrap)
+    kIsOwner = 0x00002,      // Python instance owns C++ object/memory
+    kIsExtended = 0x00004,   // has extended data
+    kIsValue = 0x00008,      // was created from a by-value return
+    kIsReference = 0x00010,  // represents one indirection
+    kIsArray = 0x00020,      // represents an array of objects
+    kIsSmartPtr = 0x00040,   // is or embeds a smart pointer
+    kIsPtrPtr = 0x00080,     // represents two indirections
+    kIsRValue = 0x00100,     // can be used as an r-value
+    kIsLValue = 0x00200,     // can be used as an l-value
+    kNoMemReg = 0x00400,     // do not register with memory regulator
+    kIsRegulated = 0x00800,  // is registered with memory regulator
+    kIsActual = 0x01000,     // has been downcasted to actual type
+    kHasLifeLine = 0x02000,  // has a life line set
+    kIsArrayAlloc = 0x04000, // represents a heap allocated array of objects
+    kIsNoConstruct =
+        0x08000, // represents constructor is not called in the allocation
+    kIsMalloc = 0x10000, // is allocated with malloc
   };
 
 public: // public, as the python C-API works with C structs

src/cpyrt/CPPMethod.cxx

@@ -34,6 +34,7 @@ extern PyObject* gBusException;
 extern PyObject* gSegvException;
 extern PyObject* gIllException;
 extern PyObject* gAbrtException;
+extern bool gUseAllocAnalyzer;
 } // namespace cppjit::cpyrt
 
 //- public helper ------------------------------------------------------------
@@ -752,6 +753,19 @@ PyObject* cpyrt::CPPMethod::GetArgDefault(int iarg, bool silent) {
 
 bool cpyrt::CPPMethod::IsConst() { return interop::IsConstMethod(GetMethod()); }
 
+//----------------------------------------------------------------------------
+interop::AllocType cpyrt::CPPMethod::GetAllocBehaviour() {
+  if (fAllocType.has_value())
+    return *fAllocType;
+  interop::AllocType attrResult = interop::IsAllocator(GetMethod());
+  if (attrResult == interop::AllocType::Unknown && gUseAllocAnalyzer) {
+    interop::AllocType analyzeResult = interop::GetAllocType(GetMethod());
+    fAllocType = analyzeResult;
+    return analyzeResult;
+  }
+  fAllocType = attrResult;
+  return attrResult;
+}
 //----------------------------------------------------------------------------
 PyObject* cpyrt::CPPMethod::GetScopeProxy() {
   // Get or build the scope of this method.

src/cpyrt/CPPMethod.h

@@ -5,6 +5,7 @@
 #include "PyCallable.h"
 
 // Standard
+#include <optional>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -62,6 +63,7 @@ class CPPMethod : public PyCallable {
   PyObject* GetCoVarNames() override;
   PyObject* GetArgDefault(int iarg, bool silent = true) override;
   bool IsConst() override;
+  cppjit::interop::AllocType GetAllocBehaviour() override;
 
   PyObject* GetScopeProxy() override;
   interop::TCppFuncAddr_t GetFunctionAddress() override;
@@ -116,6 +118,7 @@ class CPPMethod : public PyCallable {
 protected:
   // cached value that doubles as initialized flag (uninitialized if -1)
   int fArgsRequired;
+  std::optional<cppjit::interop::AllocType> fAllocType;
 };
 
 } // namespace cppjit::cpyrt

src/cpyrt/CPPOverload.cxx

@@ -155,6 +155,12 @@ static inline PyObject* HandleReturn(CPPOverload* pymeth, CPPInstance* im_self,
     CPPInstance* cppres =
         (CPPInstance*)(CPPInstance_Check(result) ? result : nullptr);
 
+    interop::AllocType AT =
+        pymeth->fMethodInfo->fMethods[0]->GetAllocBehaviour();
+    if (AT != interop::AllocType::None && AT != interop::AllocType::Null &&
+        AT != interop::AllocType::Unknown)
+      pymeth->fMethodInfo->fFlags |= CallContext::kIsCreator;
+
     // if this method creates new objects, always take ownership
     if (IsCreator(pymeth->fMethodInfo->fFlags)) {
 
@@ -165,8 +171,28 @@ static inline PyObject* HandleReturn(CPPOverload* pymeth, CPPInstance* im_self,
       }
 
       // ... or be a regular method with an object proxy return value
-      else if (cppres)
+      else if (cppres) {
         cppres->PythonOwns();
+        // After giving ownership, set proper flags to indicate allocation
+        // method/func
+        switch (AT) {
+        case interop::AllocType::Malloc:
+          cppres->fFlags |= CPPInstance::kIsMalloc;
+          break;
+        case interop::AllocType::NewArr:
+          cppres->fFlags |= CPPInstance::kIsArrayAlloc;
+          break;
+        case interop::AllocType::OperatorNew:
+          cppres->fFlags |= CPPInstance::kIsNoConstruct;
+          break;
+        case interop::AllocType::OperatorNewArr:
+          cppres->fFlags |= CPPInstance::kIsArrayAlloc;
+          cppres->fFlags |= CPPInstance::kIsNoConstruct;
+          break;
+        default:
+          break;
+        }
+      }
     }
 
     // if this new object falls inside self, make sure its lifetime is proper

src/cpyrt/PyCallable.h

@@ -37,6 +37,9 @@ class PyCallable {
   virtual PyObject* GetCoVarNames() = 0;
   virtual PyObject* GetArgDefault(int /* iarg */, bool silent = true) = 0;
   virtual bool IsConst() { return false; }
+  virtual cppjit::interop::AllocType GetAllocBehaviour() {
+    return cppjit::interop::AllocType::None;
+  }
 
   virtual PyObject* GetScopeProxy() = 0;
   virtual interop::TCppFuncAddr_t GetFunctionAddress() = 0;

src/cpyrt/cpyrtModule.cxx

@@ -279,6 +279,7 @@ PyObject* gAbrtException = nullptr;
 std::unordered_set<interop::TCppScope_t> gPinnedTypes;
 std::ostringstream gCapturedError;
 std::streambuf* gOldErrorBuffer = nullptr;
+bool gUseAllocAnalyzer = false;
 
 std::unordered_map<std::string, std::vector<PyObject*>>& pythonizations() {
   static std::unordered_map<std::string, std::vector<PyObject*>> pyzMap;
@@ -1012,6 +1013,20 @@ static PyObject* EndCaptureStderr(PyObject*, PyObject*) {
 
   return Py_BuildValue("s", capturedError.c_str());
 }
+
+//----------------------------------------------------------------------------
+static PyObject* UseAllocAnalyzer(PyObject*, PyObject* args) {
+  // Set allocation-analyzer policy, disabled by default
+  // Usage: enabling ->SetUseAllocAnalyzer(True) / SetUseAllocAnalyzer(1)
+  // disabling ->SetUseAllocAnalyzer(False) / SetUseAllocAnalyzer(0)
+  int enable = 0;
+  if (!PyArg_ParseTuple(args, const_cast<char*>("p"), &enable))
+    return nullptr;
+
+  gUseAllocAnalyzer = enable;
+
+  Py_RETURN_NONE;
+}
 } // unnamed namespace
 
 //- data -----------------------------------------------------------------------
@@ -1061,6 +1076,8 @@ static PyMethodDef gcpyrtMethods[] = {
      METH_NOARGS, (char*)"Begin capturing stderr to a in memory buffer."},
     {(char*)"_end_capture_stderr", (PyCFunction)EndCaptureStderr, METH_NOARGS,
      (char*)"End capturing stderr and returns the captured buffer."},
+    {(char*)"UseAllocAnalyzer", (PyCFunction)UseAllocAnalyzer, METH_VARARGS,
+     (char*)"Enable/disable memory-allocation analyzer."},
     {nullptr, nullptr, 0, nullptr}};
 
 struct module_state {

src/interop/cppjit_interop.h

@@ -44,6 +44,7 @@ typedef Cpp::FuncRef TCppMethod_t;
 typedef Cpp::InterpRef TInterp_t;
 typedef size_t TCppIndex_t;
 typedef void* TCppFuncAddr_t;
+typedef Cpp::AllocType AllocType;
 
 // direct interpreter access -------------------------------------------------
 RPY_EXPORTED
@@ -132,7 +133,7 @@ void Deallocate(TCppScope_t scope, TCppObject_t instance);
 RPY_EXPORTED
 TCppObject_t Construct(TCppScope_t scope, void* arena = nullptr);
 RPY_EXPORTED
-void Destruct(TCppScope_t scope, TCppObject_t instance);
+void Destruct(TCppScope_t scope, TCppObject_t instance, size_t count = 0);
 
 // method/function dispatching -----------------------------------------------
 RPY_EXPORTED
@@ -297,6 +298,10 @@ RPY_EXPORTED
 std::string GetDoxygenComment(TCppScope_t scope, bool strip_markers = true);
 RPY_EXPORTED
 bool IsConstMethod(TCppMethod_t);
+RPY_EXPORTED
+AllocType IsAllocator(TCppMethod_t);
+RPY_EXPORTED
+AllocType GetAllocType(TCppMethod_t);
 // Templated method/function reflection information
 // ------------------------------------
 RPY_EXPORTED

src/interop/interop_wrapper.cxx

@@ -817,10 +817,11 @@ interop::TCppObject_t interop::Construct(TCppScope_t scope,
   return Cpp::Construct(scope, arena, /*count=*/1);
 }
 
-void interop::Destruct(TCppScope_t scope, TCppObject_t instance) {
+void interop::Destruct(TCppScope_t scope, TCppObject_t instance,
+                       size_t count /*=0*/) {
   std::lock_guard<std::recursive_mutex> Lock(
       InterOpMutex); // TODO: this shouldn't locks the JIT call
-  Cpp::Destruct(instance, scope, true, /*count=*/0);
+  Cpp::Destruct(instance, scope, true, count);
 }
 
 static inline bool copy_args(Parameter* args, size_t nargs, void** vargs) {
@@ -1200,6 +1201,16 @@ interop::TCppType_t interop::GetMethodReturnType(TCppMethod_t method) {
   return Cpp::GetFunctionReturnType(method);
 }
 
+interop::AllocType interop::IsAllocator(TCppMethod_t method) {
+  std::lock_guard<std::recursive_mutex> Lock(InterOpMutex);
+  return Cpp::IsAllocator(method);
+}
+
+interop::AllocType interop::GetAllocType(TCppMethod_t method) {
+  std::lock_guard<std::recursive_mutex> Lock(InterOpMutex);
+  return Cpp::GetAllocType(method);
+}
+
 std::string interop::GetMethodReturnTypeAsString(TCppMethod_t method) {
   std::lock_guard<std::recursive_mutex> Lock(InterOpMutex);
   return Cpp::GetTypeAsString(

test/Makefile

@@ -10,6 +10,7 @@ dictnames = advancedcpp \
             doc_helper \
             example01 \
             fragile \
+            memory_analysis \
             operators \
             overloads \
             pythonizables \

test/cpp/MemoryOwnership/MemOwnrship.apinotes

@@ -0,0 +1,9 @@
+Name: MemOwnrship
+Functions:
+    - Name: memOwnOperatorNew
+      SwiftReturnOwnership: cppAllocOperatorNew
+Tags:
+    - Name: memOwn
+      Methods:
+        - Name: memOwnAllocator
+          SwiftReturnOwnership: cppAllocNew

test/cpp/MemoryOwnership/memory_analysis_redecl.h

@@ -0,0 +1,10 @@
+#ifndef MEMORY_ANALYSIS_REDECL_H
+#define MEMORY_ANALYSIS_REDECL_H
+#include "../memory_analysis.h"
+
+namespace memory {
+[[clang::annotate("cppAllocNew")]]
+memOwn* allocDefaultMemOwn();
+}
+
+#endif

test/cpp/MemoryOwnership/module.modulemap

@@ -0,0 +1 @@
+module MemOwnrship { header "../memory_analysis.h" }