Skip to content

[3.8] bpo-37250: put back tp_print for backwards compatibility #14193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 25, 2019
Merged

[3.8] bpo-37250: put back tp_print for backwards compatibility #14193

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d886024
[3.8] bpo-37250: put back tp_print for backwards compatibility
jdemeyer Jun 12, 2019
35f4388
Fix sizeof test
jdemeyer Jun 18, 2019
99f11a1
Use Py_DEPRECATED(3.8) macro but not in _testcapimodule.c
jdemeyer Jun 19, 2019
101163c
Re-enable deprecation warning during compilation of _testcapimodule
jdemeyer Jun 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Include/cpython/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ typedef struct _typeobject {
destructor tp_finalize;
vectorcallfunc tp_vectorcall;

/* Unused, kept for backwards compatibility in CPython 3.8 only */
int (*tp_print)(PyObject *, FILE *, int);
/* bpo-37250: kept for backwards compatibility in CPython 3.8 only */
Py_DEPRECATED(3.8) int (*tp_print)(PyObject *, FILE *, int);

#ifdef COUNT_ALLOCS
/* these must be last and never explicitly initialized */
Expand Down
2 changes: 2 additions & 0 deletions Include/pyport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ extern "C" {
* Py_DEPRECATED(3.4) typedef int T1;
* Py_DEPRECATED(3.8) PyAPI_FUNC(int) Py_OldFunction(void);
*/
#ifndef Py_DEPRECATED
Copy link
Member

@vstinner vstinner Jun 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you modify pyport.h?

Copy link
Member

@vstinner vstinner Jun 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it's fine if compiling _testcapimodule.c emits a deprecation warning. If it becomes an issue, we can just remove the test.

Copy link
Contributor Author

@jdemeyer jdemeyer Jun 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it's fine if compiling _testcapimodule.c emits a deprecation warning.

The more warnings are shown, the less value there is in those warnings. A single warning during compilation stands out, an additional warning in between many not. So we should strive for compiling CPython without any warnings.

If it becomes an issue, we can just remove the test.

I think that's the worst solution. We typically test deprecated functionality in pure Python modules, why shouldn't we test deprecated C API?

Copy link
Contributor

@ncoghlan ncoghlan Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading https://stackoverflow.com/questions/13459602/how-can-i-get-rid-of-deprecated-warnings-in-deprecated-functions-in-gcc/ suggests there are a few ways we can go here:

  1. Mark the _testcapi init function itself as deprecated, so compilers won't care that it accesses deprecated symbols.
  2. Start defining compiler specific _Py_PUSH_PRAGMA_IGNORE_DEPRECATION_WARNINGS and _Py_POP_PRAGMA macros and use those to selectively silence compiler warnings
  3. Use the testcapi warnings to check that deprecated C APIs actually are deprecated (doing that long term might involve pulling out a separately compiled module for those though, so we can more easily distinguish them from actual warnings in the main interpreter and standard library build process)

For right now, I'd suggest seeing if the warning goes away if you mark the testcapi module's init function as deprecated.

#if defined(__GNUC__) \
&& ((__GNUC__ >= 4) || (__GNUC__ == 3) && (__GNUC_MINOR__ >= 1))
#define Py_DEPRECATED(VERSION_UNUSED) __attribute__((__deprecated__))
Expand All @@ -519,6 +520,7 @@ extern "C" {
#else
#define Py_DEPRECATED(VERSION_UNUSED)
#endif
#endif


/* _Py_HOT_FUNCTION
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/C API/2019-06-12-11-45-36.bpo-37221.RhP1E7.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
``tp_print`` is put back at the end of the ``PyTypeObject`` structure
to restore support for old code (in particular generated by Cython)
setting ``tp_print = 0``.
Note that ``tp_print`` will be removed entirely in Python 3.9.
6 changes: 5 additions & 1 deletion Modules/_testcapimodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@

#define PY_SSIZE_T_CLEAN

/* We test deprecated functionality on purpose,
* so we disable the Py_DEPRECATED macro. */
#define Py_DEPRECATED(VERSION_UNUSED)

#include "Python.h"
#include "datetime.h"
#include "marshal.h"
Expand Down Expand Up @@ -6008,7 +6012,7 @@ PyInit__testcapi(void)
Py_INCREF(&MyList_Type);
PyModule_AddObject(m, "MyList", (PyObject *)&MyList_Type);

/* Old Cython code sets tp_print to 0, we check that
/* bpo-37250: old Cython code sets tp_print to 0, we check that
* this doesn't break anything. */
MyList_Type.tp_print = 0;

Expand Down