PEP 670 amendments (#2157)

Author: Erlend Egeberg Aasland

pep-0670.rst

@@ -60,13 +60,13 @@ The `GCC documentation
 <https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html>`_ lists several
 common macro pitfalls:
 
-- Misnesting;
-- Operator precedence problems;
-- Swallowing the semicolon;
-- Duplication of side effects;
-- Self-referential macros;
-- Argument prescan;
-- Newlines in arguments.
+- Misnesting
+- Operator precedence problems
+- Swallowing the semicolon
+- Duplication of side effects
+- Self-referential macros
+- Argument prescan
+- Newlines in arguments
 
 
 Performance and inlining
@@ -91,7 +91,7 @@ performances and reliable benchmarks. PGO helps the compiler to decide
 if function should be inlined or not.
 
 ``./configure --with-pydebug`` uses the ``-Og`` compiler option if it's
-supported by the compiler (GCC and LLVM clang support it): optimize
+supported by the compiler (GCC and LLVM Clang support it): optimize
 debugging experience. Otherwise, the ``-O0`` compiler option is used:
 disable most optimizations.
 
@@ -119,11 +119,10 @@ The ``Py_ALWAYS_INLINE`` macro can be used to force inlining. This macro
 uses ``__attribute__((always_inline))`` with GCC and Clang, and
 ``__forceinline`` with MSC.
 
-So far, previous attempts to use ``Py_ALWAYS_INLINE`` didn't show any
-benefit and were abandoned. See for example: `bpo-45094
-<https://bugs.python.org/issue45094>`_: "Consider using
-``__forceinline`` and ``__attribute__((always_inline))`` on static
-inline functions (``Py_INCREF``, ``Py_TYPE``) for debug build".
+Previous attempts to use ``Py_ALWAYS_INLINE`` didn't show any benefit, and were
+abandoned. See for example: `bpo-45094 <https://bugs.python.org/issue45094>`_:
+"Consider using ``__forceinline`` and ``__attribute__((always_inline))`` on
+static inline functions (``Py_INCREF``, ``Py_TYPE``) for debug build".
 
 When the ``Py_INCREF()`` macro was converted to a static inline
 functions in 2018 (`commit
@@ -140,8 +139,8 @@ Disable inlining
 ----------------
 
 On the other side, the ``Py_NO_INLINE`` macro can be used to disable
-inlining.  It is useful to reduce the stack memory usage. It is
-especially useful on a LTO+PGO build which is more aggressive to inline
+inlining.  It can be used to reduce the stack memory usage, or to prevent
+inlining on LTO+PGO builds, which are generally more aggressive to inline
 code: see `bpo-33720 <https://bugs.python.org/issue33720>`_. The
 ``Py_NO_INLINE`` macro uses ``__attribute__ ((noinline))`` with GCC and
 Clang, and ``__declspec(noinline)`` with MSC.
@@ -164,6 +163,7 @@ The following macros should not be converted:
 * Compatibility layer for different C compilers, C language extensions,
   or recent C features.
   Example: ``#define Py_ALWAYS_INLINE __attribute__((always_inline))``.
+* Macros that need the stringification or concatenation feature of the C preprocessor.
 
 
 Convert static inline functions to regular functions
@@ -189,7 +189,7 @@ Cast to PyObject*
 When a macro is converted to a function and the macro casts its
 arguments to ``PyObject*``, the new function comes with a new macro
 which cast arguments to ``PyObject*`` to prevent emitting new compiler
-warnings. So the converted functions still accept pointers to other
+warnings.  This implies that a converted function will accept pointers to
 structures inheriting from ``PyObject`` (ex: ``PyTupleObject``).
 
 For example, the ``Py_TYPE(obj)`` macro casts its ``obj`` argument to
@@ -212,14 +212,10 @@ Remove the return value
 -----------------------
 
 When a macro is implemented as an expression, it has an implicit return
-value. In some cases, the macro must not have a return value and can be
-misused in third party C extensions. See `bpo-30459
-<https://bugs.python.org/issue30459>`_ for the example of
-``PyList_SET_ITEM()`` and ``PyCell_SET()`` macros. It is not easy to
-notice this issue while reviewing macro code.
-
-These macros are converted to functions using the ``void`` return type
-to remove their return value. Removing the return value aids detecting
+value.  This macro pitfall can be misused in third party C extensions.  See
+`bpo-30459 <https://bugs.python.org/issue30459>`_ regarding the misuse of the
+``PyList_SET_ITEM()`` and ``PyCell_SET()`` macros.  Such pitfalls are hard to
+catch while reviewing macro code.  Removing the return value aids detecting
 bugs in C extensions when the C API is misused.
 
 
@@ -249,9 +245,9 @@ the macro.
 People using macros should be considered "consenting adults". People who
 feel unsafe with macros should simply not use them.
 
-The idea was rejected because macros are error prone and it is too easy
-to miss a macro pitfall when writing a macro. Moreover, macros are
-harder to read and to maintain than functions.
+These ideas are rejected because macros _are_ error prone, and it is too easy
+to miss a macro pitfall when writing and reviewing macro code. Moreover, macros
+are harder to read and maintain than functions.
 
 
 Examples of hard to read macros
@@ -318,6 +314,44 @@ Python 3.8 function (simplified code)::
     }
 
 
+PyUnicode_READ_CHAR()
+---------------------
+
+This macro reuses arguments, and possibly calls ``PyUnicode_KIND`` multiple
+times::
+
+    #define PyUnicode_READ_CHAR(unicode, index) \
+    (assert(PyUnicode_Check(unicode)),          \
+     assert(PyUnicode_IS_READY(unicode)),       \
+     (Py_UCS4)                                  \
+        (PyUnicode_KIND((unicode)) == PyUnicode_1BYTE_KIND ? \
+            ((const Py_UCS1 *)(PyUnicode_DATA((unicode))))[(index)] : \
+            (PyUnicode_KIND((unicode)) == PyUnicode_2BYTE_KIND ? \
+                ((const Py_UCS2 *)(PyUnicode_DATA((unicode))))[(index)] : \
+                ((const Py_UCS4 *)(PyUnicode_DATA((unicode))))[(index)] \
+            ) \
+        ))
+
+Possible implementation as a static inlined function::
+
+    static inline Py_UCS4
+    PyUnicode_READ_CHAR(PyObject *unicode, Py_ssize_t index)
+    {
+        assert(PyUnicode_Check(unicode));
+        assert(PyUnicode_IS_READY(unicode));
+
+        switch (PyUnicode_KIND(unicode)) {
+        case PyUnicode_1BYTE_KIND:
+            return (Py_UCS4)((const Py_UCS1 *)(PyUnicode_DATA(unicode)))[index];
+        case PyUnicode_2BYTE_KIND:
+            return (Py_UCS4)((const Py_UCS2 *)(PyUnicode_DATA(unicode)))[index];
+        case PyUnicode_4BYTE_KIND:
+        default:
+            return (Py_UCS4)((const Py_UCS4 *)(PyUnicode_DATA(unicode)))[index];
+        }
+    }
+
+
 Macros converted to functions since Python 3.8
 ==============================================