[Python-checkins] bpo-33738: Fix macros which contradict PEP 384 (GH-7477)
Ned Deily
webhook-mailer at python.org
Sat Jun 9 14:32:28 EDT 2018
https://github.com/python/cpython/commit/ea62ce7f4fefc66bc0adba16bcd7666d5bbd5b44
commit: ea62ce7f4fefc66bc0adba16bcd7666d5bbd5b44
branch: master
author: Christian Tismer <tismer at stackless.com>
committer: Ned Deily <nad at python.org>
date: 2018-06-09T14:32:25-04:00
summary:
bpo-33738: Fix macros which contradict PEP 384 (GH-7477)
During development of the limited API support for PySide,
we saw an error in a macro that accessed a type field.
This patch fixes the 7 errors in the Python headers.
Macros which were not written as capitals were implemented
as function.
To do the necessary analysis again, a script was included that
parses all headers and looks for "->tp_" in serctions which can
be reached with active limited API.
It is easily possible to call this script as a test.
Error listing:
../../Include/objimpl.h:243
#define PyObject_IS_GC(o) (PyType_IS_GC(Py_TYPE(o)) && \
(Py_TYPE(o)->tp_is_gc == NULL || Py_TYPE(o)->tp_is_gc(o)))
Action: commented only
../../Include/objimpl.h:362
#define PyType_SUPPORTS_WEAKREFS(t) ((t)->tp_weaklistoffset > 0)
Action: commented only
../../Include/objimpl.h:364
#define PyObject_GET_WEAKREFS_LISTPTR(o) \
((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset))
Action: commented only
../../Include/pyerrors.h:143
#define PyExceptionClass_Name(x) \
((char *)(((PyTypeObject*)(x))->tp_name))
Action: implemented function
../../Include/abstract.h:593
#define PyIter_Check(obj) \
((obj)->ob_type->tp_iternext != NULL && \
(obj)->ob_type->tp_iternext != &_PyObject_NextNotImplemented)
Action: implemented function
../../Include/abstract.h:713
#define PyIndex_Check(obj) \
((obj)->ob_type->tp_as_number != NULL && \
(obj)->ob_type->tp_as_number->nb_index != NULL)
Action: implemented function
../../Include/abstract.h:924
#define PySequence_ITEM(o, i)\
( Py_TYPE(o)->tp_as_sequence->sq_item(o, i) )
Action: commented only
files:
A Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst
A Tools/scripts/pep384_macrocheck.py
M Include/abstract.h
M Include/objimpl.h
M Include/pyerrors.h
M Objects/abstract.c
M Objects/exceptions.c
M PC/python3.def
diff --git a/Include/abstract.h b/Include/abstract.h
index 4088f75ff3c7..e7bc2d24bc22 100644
--- a/Include/abstract.h
+++ b/Include/abstract.h
@@ -590,9 +590,16 @@ PyAPI_FUNC(PyObject *) PyObject_Format(PyObject *obj,
returns itself. */
PyAPI_FUNC(PyObject *) PyObject_GetIter(PyObject *);
+/* Returns 1 if the object 'obj' provides iterator protocols, and 0 otherwise.
+
+ This function always succeeds. */
+#ifndef Py_LIMITED_API
#define PyIter_Check(obj) \
((obj)->ob_type->tp_iternext != NULL && \
(obj)->ob_type->tp_iternext != &_PyObject_NextNotImplemented)
+#else
+PyAPI_FUNC(int) PyIter_Check(PyObject*);
+#endif
/* Takes an iterator object and calls its tp_iternext slot,
returning the next value.
@@ -710,9 +717,15 @@ PyAPI_FUNC(PyObject *) PyNumber_Xor(PyObject *o1, PyObject *o2);
This is the equivalent of the Python expression: o1 | o2. */
PyAPI_FUNC(PyObject *) PyNumber_Or(PyObject *o1, PyObject *o2);
+/* Returns 1 if obj is an index integer (has the nb_index slot of the
+ tp_as_number structure filled in), and 0 otherwise. */
+#ifndef Py_LIMITED_API
#define PyIndex_Check(obj) \
((obj)->ob_type->tp_as_number != NULL && \
(obj)->ob_type->tp_as_number->nb_index != NULL)
+#else
+PyAPI_FUNC(int) PyIndex_Check(PyObject *);
+#endif
/* Returns the object 'o' converted to a Python int, or NULL with an exception
raised on failure. */
@@ -921,8 +934,10 @@ PyAPI_FUNC(PyObject *) PySequence_Fast(PyObject *o, const char* m);
/* Assume tp_as_sequence and sq_item exist and that 'i' does not
need to be corrected for a negative index. */
+#ifndef Py_LIMITED_API
#define PySequence_ITEM(o, i)\
( Py_TYPE(o)->tp_as_sequence->sq_item(o, i) )
+#endif
/* Return a pointer to the underlying item array for
an object retured by PySequence_Fast */
diff --git a/Include/objimpl.h b/Include/objimpl.h
index 057bb50cbda9..a38906c7dc8b 100644
--- a/Include/objimpl.h
+++ b/Include/objimpl.h
@@ -240,8 +240,10 @@ PyAPI_FUNC(Py_ssize_t) _PyGC_CollectIfEnabled(void);
#define PyType_IS_GC(t) PyType_HasFeature((t), Py_TPFLAGS_HAVE_GC)
/* Test if an object has a GC head */
+#ifndef Py_LIMITED_API
#define PyObject_IS_GC(o) (PyType_IS_GC(Py_TYPE(o)) && \
(Py_TYPE(o)->tp_is_gc == NULL || Py_TYPE(o)->tp_is_gc(o)))
+#endif
PyAPI_FUNC(PyVarObject *) _PyObject_GC_Resize(PyVarObject *, Py_ssize_t);
#define PyObject_GC_Resize(type, op, n) \
@@ -359,10 +361,12 @@ PyAPI_FUNC(void) PyObject_GC_Del(void *);
/* Test if a type supports weak references */
+#ifndef Py_LIMITED_API
#define PyType_SUPPORTS_WEAKREFS(t) ((t)->tp_weaklistoffset > 0)
#define PyObject_GET_WEAKREFS_LISTPTR(o) \
((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset))
+#endif
#ifdef __cplusplus
}
diff --git a/Include/pyerrors.h b/Include/pyerrors.h
index f289471be20c..a9929f5f19f9 100644
--- a/Include/pyerrors.h
+++ b/Include/pyerrors.h
@@ -140,8 +140,12 @@ PyAPI_FUNC(void) _PyErr_ChainExceptions(PyObject *, PyObject *, PyObject *);
#define PyExceptionInstance_Check(x) \
PyType_FastSubclass((x)->ob_type, Py_TPFLAGS_BASE_EXC_SUBCLASS)
+#ifndef Py_LIMITED_API
#define PyExceptionClass_Name(x) \
((char *)(((PyTypeObject*)(x))->tp_name))
+#else
+ PyAPI_FUNC(char *) PyExceptionClass_Name(PyObject*);
+#endif
#define PyExceptionInstance_Class(x) ((PyObject*)((x)->ob_type))
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst b/Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst
new file mode 100644
index 000000000000..0d66c4b1f191
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst
@@ -0,0 +1,3 @@
+Seven macro incompatibilities with the Limited API were fixed, and the
+macros PyIter_Check, PyIndex_Check and PyExceptionClass_Name were added as
+functions. A script for automatic macro checks was added.
diff --git a/Objects/abstract.c b/Objects/abstract.c
index 93eda62a2745..e2700e3f80d7 100644
--- a/Objects/abstract.c
+++ b/Objects/abstract.c
@@ -1244,6 +1244,14 @@ PyNumber_Absolute(PyObject *o)
return type_error("bad operand type for abs(): '%.200s'", o);
}
+#undef PyIndex_Check
+int
+PyIndex_Check(PyObject *obj)
+{
+ return obj->ob_type->tp_as_number != NULL &&
+ obj->ob_type->tp_as_number->nb_index != NULL;
+}
+
/* Return a Python int from the object item.
Raise TypeError if the result is not an int
or if the object cannot be interpreted as an index.
@@ -2535,6 +2543,13 @@ PyObject_GetIter(PyObject *o)
}
}
+#undef PyIter_Check
+int PyIter_Check(PyObject *obj)
+{
+ return obj->ob_type->tp_iternext != NULL &&
+ obj->ob_type->tp_iternext != &_PyObject_NextNotImplemented;
+}
+
/* Return next item.
* If an error occurs, return NULL. PyErr_Occurred() will be true.
* If the iteration terminates normally, return NULL and clear the
diff --git a/Objects/exceptions.c b/Objects/exceptions.c
index bfc818f7b4ca..bcb1fd5d07c5 100644
--- a/Objects/exceptions.c
+++ b/Objects/exceptions.c
@@ -342,6 +342,12 @@ PyException_SetContext(PyObject *self, PyObject *context)
Py_XSETREF(((PyBaseExceptionObject *)self)->context, context);
}
+#undef PyExceptionClass_Name
+char *
+PyExceptionClass_Name(PyObject *ob)
+{
+ return ((PyTypeObject*)ob)->tp_name;
+}
static struct PyMemberDef BaseException_members[] = {
{"__suppress_context__", T_BOOL,
diff --git a/PC/python3.def b/PC/python3.def
index 48a2d07aab13..5d93c18af87e 100644
--- a/PC/python3.def
+++ b/PC/python3.def
@@ -248,6 +248,7 @@ EXPORTS
PyExc_Warning=python38.PyExc_Warning DATA
PyExc_WindowsError=python38.PyExc_WindowsError DATA
PyExc_ZeroDivisionError=python38.PyExc_ZeroDivisionError DATA
+ PyExceptionClass_Name=python38.PyExceptionClass_Name
PyException_GetCause=python38.PyException_GetCause
PyException_GetContext=python38.PyException_GetContext
PyException_GetTraceback=python38.PyException_GetTraceback
@@ -294,9 +295,11 @@ EXPORTS
PyImport_ImportModuleLevelObject=python38.PyImport_ImportModuleLevelObject
PyImport_ImportModuleNoBlock=python38.PyImport_ImportModuleNoBlock
PyImport_ReloadModule=python38.PyImport_ReloadModule
+ PyIndex_Check=python38.PyIndex_Check
PyInterpreterState_Clear=python38.PyInterpreterState_Clear
PyInterpreterState_Delete=python38.PyInterpreterState_Delete
PyInterpreterState_New=python38.PyInterpreterState_New
+ PyIter_Check=python38.PyIter_Check
PyIter_Next=python38.PyIter_Next
PyListIter_Type=python38.PyListIter_Type DATA
PyListRevIter_Type=python38.PyListRevIter_Type DATA
diff --git a/Tools/scripts/pep384_macrocheck.py b/Tools/scripts/pep384_macrocheck.py
new file mode 100644
index 000000000000..142d248dd2fa
--- /dev/null
+++ b/Tools/scripts/pep384_macrocheck.py
@@ -0,0 +1,148 @@
+"""
+pep384_macrocheck.py
+
+This programm tries to locate errors in the relevant Python header
+files where macros access type fields when they are reachable from
+the limided API.
+
+The idea is to search macros with the string "->tp_" in it.
+When the macro name does not begin with an underscore,
+then we have found a dormant error.
+
+Christian Tismer
+2018-06-02
+"""
+
+import sys
+import os
+import re
+
+
+DEBUG = False
+
+def dprint(*args, **kw):
+ if DEBUG:
+ print(*args, **kw)
+
+def parse_headerfiles(startpath):
+ """
+ Scan all header files which are reachable fronm Python.h
+ """
+ search = "Python.h"
+ name = os.path.join(startpath, search)
+ if not os.path.exists(name):
+ raise ValueError("file {} was not found in {}\n"
+ "Please give the path to Python's include directory."
+ .format(search, startpath))
+ errors = 0
+ with open(name) as python_h:
+ while True:
+ line = python_h.readline()
+ if not line:
+ break
+ found = re.match(r'^\s*#\s*include\s*"(\w+\.h)"', line)
+ if not found:
+ continue
+ include = found.group(1)
+ dprint("Scanning", include)
+ name = os.path.join(startpath, include)
+ if not os.path.exists(name):
+ name = os.path.join(startpath, "../PC", include)
+ errors += parse_file(name)
+ return errors
+
+def ifdef_level_gen():
+ """
+ Scan lines for #ifdef and track the level.
+ """
+ level = 0
+ ifdef_pattern = r"^\s*#\s*if" # covers ifdef and ifndef as well
+ endif_pattern = r"^\s*#\s*endif"
+ while True:
+ line = yield level
+ if re.match(ifdef_pattern, line):
+ level += 1
+ elif re.match(endif_pattern, line):
+ level -= 1
+
+def limited_gen():
+ """
+ Scan lines for Py_LIMITED_API yes(1) no(-1) or nothing (0)
+ """
+ limited = [0] # nothing
+ unlimited_pattern = r"^\s*#\s*ifndef\s+Py_LIMITED_API"
+ limited_pattern = "|".join([
+ r"^\s*#\s*ifdef\s+Py_LIMITED_API",
+ r"^\s*#\s*(el)?if\s+!\s*defined\s*\(\s*Py_LIMITED_API\s*\)\s*\|\|",
+ r"^\s*#\s*(el)?if\s+defined\s*\(\s*Py_LIMITED_API"
+ ])
+ else_pattern = r"^\s*#\s*else"
+ ifdef_level = ifdef_level_gen()
+ status = next(ifdef_level)
+ wait_for = -1
+ while True:
+ line = yield limited[-1]
+ new_status = ifdef_level.send(line)
+ dir = new_status - status
+ status = new_status
+ if dir == 1:
+ if re.match(unlimited_pattern, line):
+ limited.append(-1)
+ wait_for = status - 1
+ elif re.match(limited_pattern, line):
+ limited.append(1)
+ wait_for = status - 1
+ elif dir == -1:
+ # this must have been an endif
+ if status == wait_for:
+ limited.pop()
+ wait_for = -1
+ else:
+ # it could be that we have an elif
+ if re.match(limited_pattern, line):
+ limited.append(1)
+ wait_for = status - 1
+ elif re.match(else_pattern, line):
+ limited.append(-limited.pop()) # negate top
+
+def parse_file(fname):
+ errors = 0
+ with open(fname) as f:
+ lines = f.readlines()
+ type_pattern = r"^.*?->\s*tp_"
+ define_pattern = r"^\s*#\s*define\s+(\w+)"
+ limited = limited_gen()
+ status = next(limited)
+ for nr, line in enumerate(lines):
+ status = limited.send(line)
+ line = line.rstrip()
+ dprint(fname, nr, status, line)
+ if status != -1:
+ if re.match(define_pattern, line):
+ name = re.match(define_pattern, line).group(1)
+ if not name.startswith("_"):
+ # found a candidate, check it!
+ macro = line + "\n"
+ idx = nr
+ while line.endswith("\\"):
+ idx += 1
+ line = lines[idx].rstrip()
+ macro += line + "\n"
+ if re.match(type_pattern, macro, re.DOTALL):
+ # this type field can reach the limited API
+ report(fname, nr + 1, macro)
+ errors += 1
+ return errors
+
+def report(fname, nr, macro):
+ f = sys.stderr
+ print(fname + ":" + str(nr), file=f)
+ print(macro, file=f)
+
+if __name__ == "__main__":
+ p = sys.argv[1] if sys.argv[1:] else "../../Include"
+ errors = parse_headerfiles(p)
+ if errors:
+ # somehow it makes sense to raise a TypeError :-)
+ raise TypeError("These {} locations contradict the limited API."
+ .format(errors))
More information about the Python-checkins
mailing list