[memory-analysis] Added a field to CPPMethod to store memory-ownershi… - #35
[memory-analysis] Added a field to CPPMethod to store memory-ownershi…#35keremsahn wants to merge 1 commit into
Conversation
…p information and this information effects kIsCreator flag of overload group , currently analyzer is not called, just attribute checker is called
| //---------------------------------------------------------------------------- | ||
| // 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, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
The changes this PR depends on are already part of main in CppInterOp.
|
I am converting to draft for now. Once I am happy with the state of this PR, I will mark it "ready for review" for others. |
| class memAnalysisKlass { | ||
| int val; | ||
| }; | ||
| __attribute__((malloc)) memAnalysisKlass* allocTest(); | ||
| __attribute__((ownership_returns(malloc))) memAnalysisKlass* allocTestReturns(); |
There was a problem hiding this comment.
Put everything in this file into a namespace; otherwise, the names may conflict with things that are JIT compiled or exist in other files.
Vipul-Cariappa
left a comment
Vipul-Cariappa
left a comment
There was a problem hiding this comment.
Some minor comments. LGTM, otherwise.
|
|
||
| obj = cppjit.gbl.allocTestReturns() | ||
| assert type(obj) == cppjit.gbl.memAnalysisKlass | ||
| assert obj.__python_owns__ |
There was a problem hiding this comment.
We also need tests for
assert not obj.__python__owns__| virtual PyObject* GetArgDefault(int /* iarg */, bool silent = true) = 0; | ||
| virtual bool IsConst() { return false; } | ||
| virtual cppjit::interop::AllocType GetAllocBehaviour() { | ||
| return cppjit::interop::AllocType::None; |
There was a problem hiding this comment.
| return cppjit::interop::AllocType::None; | |
| return cppjit::interop::AllocType::Unknown; |
?
There was a problem hiding this comment.
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
3 participants
When overload resolution happens and a suitable candidate found and called, HandleReturn helper is called for every case. In this function I call GetAllocBehaviour function for the first element of overload candidates (I followed design where IsCreator flag belongs to overload group, not unique for every PyCallable, but I think of changing it).
2 important notes: Enum order of AllocType is going to change in CppInterOp after open PRs are merged,
and IsAllocator returns bool in the current CppInterOp but this is also going to change after PRs get merged, that is why I keep AllocType in CPPMethod even though it is not useful now
@Vipul-Cariappa @aaronj0