I'm working on a script to migrate old C extension modules to the latest flavor of the Python C API:
https://github.com/pythoncapi/upgrade_pythoncapi

I would like to replace frame->f_code with PyFrame_GetCode(frame). The problem is that frame->f_code is a borrowed reference, whereas PyFrame_GetCode() returns a strong reference. Having a Py_Borrow() function would help to automate migration to PyFrame_GetCode():

static inline PyObject* Py_Borrow(PyObject *obj)
{
    Py_XDECREF(obj);
    return obj;
}

So frame->f_code can be replaced with Py_Borrow(PyFrame_GetCode(frame)).

Py_Borrow() is similar to Py_XDECREF() but can be used as an expression:

    PyObject *code = Py_Borrow(PyFrame_GetCode(frame));

Py_Borrow() is the opposite of Py_XNewRef(). For example, Py_Borrow(Py_XNewRef(obj)) leaves the reference count unchanged (+1 and then -1).


My pratical problem is that it's not easy not add Py_XDECREF() call when converting C code to the new C API.

Example 1:

    PyObject *code = frame->f_code;

This one is easy and can be written as:

    PyObject *code = PyFrame_GetCode(frame); Py_XDECREF(code);


Example 2:

    func(frame->f_code);

This one is more tricky. For example, the following code using a macro is wrong:

    #define Py_BORROW(obj) (Py_XDECREF(obj), obj)
    func(Py_BORROW(PyFrame_GetCode(frame->f_code)));

since it calls PyFrame_GetCode() twice when proceed by the C preprocessor and so leaks a reference:

    func(Py_XDECREF(PyFrame_GetCode(frame->f_code)), PyFrame_GetCode(frame->f_code));


Attached PR implements Py_Borrow() function as a static inline function.

I'm -1 on adding this API as it is inherently unsafe and is bound to be used in locations where its contract is not honoured (especially outside of the core). 

Note that the pre-condition on the argument is that you now only own a reference to the object, but that there's also someone else that owns a reference.

It is better to bite the bullet and go for correct reference count updates when converting to the new PyFrame_GetCode API.

Ronald Oussoren: "I'm -1 on adding this API as it is inherently unsafe and is bound to be used in locations where its contract is not honoured (especially outside of the core)."

I'm trying to update old C extensions which already rely on borrowed references.

I expect that only my script will use this function. Developers should not call it directly, but use strong references. I explained it in the function documentation.

Obviously, using strong references is safer, but it's not possible to automate the conversion of borrowed references to strong references. Maybe it's possible on simple functions, but not in the general code (long functions with loops, function calls, goto, etc.).


> Note that the pre-condition on the argument is that you now only own a reference to the object, but that there's also someone else that owns a reference.

Py_Borrow() is unsafe by design and should not be used :-)

Replacing frame->f_code with Py_Borrow(PyFrame_GetCode(frame)) doesn't make the code worse nor better in term of reference counting.


> It is better to bite the bullet and go for correct reference count updates when converting to the new PyFrame_GetCode API.

My goal is to automate conversion to new C API. Using strong references is really challenging and I expect that it would be done manually.

--

Once a code base is updated to use Py_Borrow(), it might become possible to replace automatically some simple patterns to use strong references.

My proposed function doesn't have to be public. I updated the issue and my PR to propose to add a *private* function to clarify that it should not be used unless you understand what you do.

In the meanwhile, I modified my upgrade_pythoncapi.py tool to include pythoncapi_compat.h, and I added _Py_Borrow() and _Py_XBorrow() to pythoncapi_compat.h:

* https://github.com/pythoncapi/upgrade_pythoncapi
* https://github.com/pythoncapi/pythoncapi_compat

A private function is less problematic, but still far from ideal. Any use of the function is IMHO a code smell that should be refactored. 

I'm primarily against this function in the public API because it will be used where it isn't appropriate because it looks convenient (such as ``somefuntion(Py_Borrow(PyUnicode_FromUTF8(...)))``).

BTW. Have you considered using coccinelle <https://coccinelle.gitlabpages.inria.fr/website/> instead of a python script?

I have no experience with the tool, but it is meant to be used for code transformation ("semantic patches").  I only know of the tool because of articles on LWN.net, I haven't tried using it yet.

> BTW. Have you considered using coccinelle <https://coccinelle.gitlabpages.inria.fr/website/> instead of a python script?

I wrote Python code just because I like Python. There are likely other more advanced tools like coccinelle, but Python users are more used to install Python scripts and Python is portable.

I totally understand, I would have done the same.  Coccinelle looks like an interesting tool for refactorings like this, but I prefer to write Python scripts (and often just refactor manually).

I find it confusing that the function is called Py_Borrow() but steals the reference.

I had more time to think about this issue.

First, I decided to always include "pythoncapi_compat.h" and so there is no strict requirement to add new C API functions to Python.

Second, the _Py_Borrow() function looked nice, but there is a practical issue: its return type is always PyObject* which makes it not convenient to "convert" a function returning a different type.

For example, I replaced "frame->f_code" with "((PyCodeObject *)_Py_Borrow(PyFrame_GetCode(frame)))", but this expression requires 3 levels of parenthesis, it's long and it's hard to read :-( The outter parenthesis is needed if the modified expresion is used to get a code member, like: "frame->f_code->co_name".

I chose another approach: add a "Borrow" variant of the functions that I need.

For example, add _PyFrameGetCodeBorrow() variant of PyFrameGetCode(). This function is only added to pythoncapi_compat.h, and so Python doesn't have to be modified.

I reject my issue :-)