On Tue, Feb 8, 2022 at 8:30 AM Victor Stinner <vstin...@python.org> wrote:
> 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 <pvikt...@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. > That's why we pointed out what is not controversial and can be done without a PEP. That lets you update the PEP for everything remaining so we can focus on what really need discussion instead on things that are an obvious "yes" to us.
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/TCZDU6QBSHYBZQCVAJZBJMDZQTTI7BIL/ Code of Conduct: http://python.org/psf/codeofconduct/
