Hi Petr,

Thanks for the SC review, it's very helpful! I know that it's a big PEP :-)

On Tue, Feb 8, 2022 at 11:33 AM Petr Viktorin <pviktori@redhat.com> wrote:
> *All other things being equal, static inline functions are better than
> macros.*
> Specifically, inline static functions should be preferred over
> function-like macros in new code. Please add a note about this to PEP 7.

Ok, I created: https://github.com/python/peps/pull/2315


> *When there is no backwards compatibility issue, macros can be changed
> to static inline functions.*
> In effect, the SC accepts the first part of the PEP, except cases where:
> a macro is converted to macro and function ("Cast to PyObject*"),

Right now, a large number of macros cast their argument to a type. A
few examples:

#define PyObject_TypeCheck(ob, type)
PyObject_TypeCheck(_PyObject_CAST(ob), type)
#define PyTuple_GET_ITEM(op, i) (_PyTuple_CAST(op)->ob_item[i])
#define PyDict_GET_SIZE(mp)  (assert(PyDict_Check(mp)),((PyDictObject
*)mp)->ma_used)
#define PyWeakref_GET_OBJECT(ref) (... ((PyWeakReference
*)(ref))->wr_object ...)

Does it mean that these macros must remain macros?

PEP 670 proposes adding a macro to still cast their arguments to the
requested type so the PEP doesn't add any new compiler warnings. The
PEP proposes to consider removing these implicit casts later (to
reduce the scope and limit annoyance caused by the PEP).

If the macro is converted to a static inline function without such
convenient macro, C extensions which don't pass the expected types
will get spammed by warnings. Well, it's easy to fix, and once it is
fixed, the code remains compatible with old Python versions.

I'm not sure that I understood the SC statement here: does it mean
that these specific macros (which cast their arguments) must remain
macros, or that it's ok to convert them to static inline functions
(even if it can emit new warnings)?

If the SC is ok to add new compiler warnings, I'm fine with it. Most
functions are documented with an exact type, they are not documented
to cast implicitly their arguments to the expected types.

For example, Py_INCREF() expects a PyObject* type in the documentation:
https://docs.python.org/dev/c-api/refcounting.html#c.Py_INCREF

--

Or is the problem that having a macro to wrap a static inline function
requires to change the function name? Well, technically, it's not
needed... I wrote a PR so the macro and the function have the same
name:
https://github.com/python/cpython/pull/31217

For example, the Py_INCREF() macro now calls Py_INCREF(), instead of
calling _Py_INCREF().

Static inline functions are not part of the ABI since they are
inlined, but it's maybe better to avoid the "_" prefix to avoid
confusion in debuggers and profilers (especially when comparing old
and new Python versions).


> and where the return value is removed.

In this case, I propose to leave these macros unchanged in PEP 670 in
this case, as it already excludes macros which can be used as l-values
(macros changed by PEP 674).

I would prefer to have the whole PEP 670 either accepted or rejected.
I'm not comfortable with a half-accepted status, it's misleading for
readers.