Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/cpyrt/CPPMethod.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,23 @@ PyObject* cpyrt::CPPMethod::GetArgDefault(int iarg, bool silent) {

bool cpyrt::CPPMethod::IsConst() { return interop::IsConstMethod(GetMethod()); }

//----------------------------------------------------------------------------
// FIXME: For now every allocation is assumed to be done with `new`
// will be fixed soon. Also the reason function returns an AllocType
// instead of bool, IsAllocator function from CppInterOp changed a little,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can test these changes on top of your CppInterOp branches by updating the remote and ref here: https://github.com/compiler-research/cppjit/blob/main/CMakeLists.txt#L14-L15

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes this PR depends on are already part of main in CppInterOp.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant for changes that have not been merged yet (referred to in the FIXME comment on IsAllocator). iiuc the API on the CppInterOp side will change pending some work being merged, so if you want to coverage test the cppjit patch, this could help

// but these changes did not merged to main yet, another reason is next PR
// will add user-optional analyzer so the function needs to return allocation
// way
interop::AllocType cpyrt::CPPMethod::GetAllocBehaviour() {
if (fAllocType.has_value())
return *fAllocType;
if (interop::IsAllocator(GetMethod())) {
fAllocType = interop::AllocType::New;
return interop::AllocType::New;
}
fAllocType = interop::AllocType::None;
return interop::AllocType::None;
}
//----------------------------------------------------------------------------
PyObject* cpyrt::CPPMethod::GetScopeProxy() {
// Get or build the scope of this method.
Expand Down
3 changes: 3 additions & 0 deletions src/cpyrt/CPPMethod.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "PyCallable.h"

// Standard
#include <optional>
#include <string>
#include <unordered_map>
#include <vector>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/cpyrt/CPPOverload.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {

Expand Down
3 changes: 3 additions & 0 deletions src/cpyrt/PyCallable.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return cppjit::interop::AllocType::None;
return cppjit::interop::AllocType::Unknown;

?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only class does not override GetAllocBehaviour is TPythonCallBack, and from my understanding this class is for some sort of python function, not C/C++, therefore I thought it would make sense to return None

}

virtual PyObject* GetScopeProxy() = 0;
virtual interop::TCppFuncAddr_t GetFunctionAddress() = 0;
Expand Down
15 changes: 15 additions & 0 deletions src/cpyrt/cppjit_interop.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ struct ObjectRef {
friend bool operator==(ObjectRef a, ObjectRef b) { return a.data == b.data; }
friend bool operator!=(ObjectRef a, ObjectRef b) { return !(a == b); }
};

enum class AllocType : unsigned char {
None,
New,
NewArr,
Malloc,
Unknown,
CustomAlloc,
Null,
OperatorNew,
OperatorNewArr
};
} // namespace Cpp

template <> struct std::hash<Cpp::DeclRef> {
Expand Down Expand Up @@ -122,6 +134,7 @@ typedef Cpp::ObjectRef TCppObject_t;
typedef Cpp::FuncRef TCppMethod_t;
typedef size_t TCppIndex_t;
typedef void* TCppFuncAddr_t;
typedef Cpp::AllocType AllocType;

// direct interpreter access -------------------------------------------------
CPPJIT_IMPORT
Expand Down Expand Up @@ -368,6 +381,8 @@ CPPJIT_IMPORT
std::string GetDoxygenComment(TCppScope_t scope, bool strip_markers = true);
CPPJIT_IMPORT
bool IsConstMethod(TCppMethod_t);
CPPJIT_IMPORT
bool IsAllocator(TCppMethod_t);
// Templated method/function reflection information
// ------------------------------------
CPPJIT_IMPORT
Expand Down
3 changes: 3 additions & 0 deletions src/interop/cpp_cppjit.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,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
Expand Down Expand Up @@ -306,6 +307,8 @@ RPY_EXPORTED
std::string GetDoxygenComment(TCppScope_t scope, bool strip_markers = true);
RPY_EXPORTED
bool IsConstMethod(TCppMethod_t);
RPY_EXPORTED
bool IsAllocator(TCppMethod_t);
// Templated method/function reflection information
// ------------------------------------
RPY_EXPORTED
Expand Down
5 changes: 5 additions & 0 deletions src/interop/interop_wrapper.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,11 @@ interop::TCppType_t interop::GetMethodReturnType(TCppMethod_t method) {
return Cpp::GetFunctionReturnType(method);
}

bool interop::IsAllocator(TCppMethod_t method) {
std::lock_guard<std::recursive_mutex> Lock(InterOpMutex);
return Cpp::IsAllocator(method);
}

std::string interop::GetMethodReturnTypeAsString(TCppMethod_t method) {
std::lock_guard<std::recursive_mutex> Lock(InterOpMutex);
return Cpp::GetTypeAsString(
Expand Down
1 change: 1 addition & 0 deletions test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ dictnames = advancedcpp \
doc_helper \
example01 \
fragile \
memory_analysis \
operators \
overloads \
pythonizables \
Expand Down
15 changes: 15 additions & 0 deletions test/cpp/memory_analysis.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#include "memory_analysis.h"
__attribute__((malloc)) memory::memAnalysisKlass* memory::mallocAttr() {
return new memory::memAnalysisKlass;
}

__attribute__((ownership_returns(malloc))) memory::memAnalysisKlass*
memory::ownershipReturnsAttr() {
return new memory::memAnalysisKlass;
}

// Expected to not return ownership when analysis is off, and there is just
// attr-check
memory::memAnalysisKlass* memory::noAttr() {
return new memory::memAnalysisKlass;
}
19 changes: 19 additions & 0 deletions test/cpp/memory_analysis.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#ifndef MEMORY_ANALYSIS_H
#define MEMORY_ANALYSIS_H

#include <new>
#include <stdlib.h>
namespace memory {

class memAnalysisKlass {
public:
int val;
};
__attribute__((malloc)) memAnalysisKlass* mallocAttr();
__attribute__((ownership_returns(malloc))) memAnalysisKlass*
ownershipReturnsAttr();
memAnalysisKlass* noAttr();

} // namespace memory

#endif // MEMORY_ANALYSIS_H
39 changes: 39 additions & 0 deletions test/test_memoryanalysis.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import py
from support import setup_make

currpath = py.path.local(__file__).dirpath()
test_dct = str(currpath.join("cpp/memory_analysisDict"))


def setup_module(mod):
setup_make("memory_analysis")


class TestMEMORYANALYSIS:
def setup_class(cls):
cls.test_dct = test_dct
import cppjit

cls.memory_analysis = cppjit.load_reflection_info(cls.test_dct)

def test01_malloc_attr(self):
import cppjit

obj = cppjit.gbl.memory.mallocAttr()
assert type(obj) == cppjit.gbl.memory.memAnalysisKlass
assert obj.__python_owns__

def test02_ownership_returns_attr(self):
import cppjit

obj = cppjit.gbl.memory.ownershipReturnsAttr()
assert type(obj) == cppjit.gbl.memory.memAnalysisKlass
assert obj.__python_owns__

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need tests for

assert not obj.__python__owns__


def test03_no_attr(self):
import cppjit

obj = cppjit.gbl.memory.noAttr()
assert type(obj) == cppjit.gbl.memory.memAnalysisKlass
assert not obj.__python_owns__
obj.__python_owns__ = True
Loading