classification
Title: Remove unused and undocumented PyGen_NeedsFinalizing() function
Type: Stage: resolved
Components: Documentation Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: cheryl.sabella, docs@python, nanjekyejoannah, ncoghlan, pitrou, plasticgap, vstinner
Priority: normal Keywords: patch

Created on 2012-06-17 04:21 by ncoghlan, last changed 2019-09-06 15:43 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
Issue15088.diff plasticgap, 2013-04-30 17:06 review
Pull Requests
URL Status Linked Edit
PR 15702 merged nanjekyejoannah, 2019-09-05 14:15
Messages (13)
msg163012 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-17 04:21
Currently undocumented: http://docs.python.org/py3k/c-api/gen.html
Public API in a released version of Python: http://hg.python.org/cpython/file/3.2/Include/genobject.h
msg188170 - (view) Author: Nathan Housel (plasticgap) Date: 2013-04-30 17:06
Please correct me if I'm wrong, but I think PyGen_NeedsFinalizing should not be an API function because it is only used, nor _PyGen_FetchStopIterationValue. In the attached patch I've removed PyGen_NeedsFinalizing and _PyGen_FetchStopIterationValue fom the public API.
msg217456 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-29 00:04
For the record, PyGen_NeedsFinalizing still exists but it isn't used anymore in the code base (following PEP 442).
msg314420 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2018-03-25 16:36
What should be done for the PyGen_NeedsFinalizing API?  Does it need to be documented or should it be removed since it's no longer used in the code base?

Thanks!
msg314791 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2018-04-01 23:41
Cheryl: it may be useful to do a code search to find out whether any third-party projects are relying on PyGen_NeedsFinalizing.
msg314808 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2018-04-02 14:30
Thanks Antoine.

Do you have a good way for searching third party projects?  I searched on Github and I'm getting a lot of references to the CPython filenames.  I don't know how to check if anyone is making calls to the function.  But, maybe that's the point and it can't be removed if there's a chance that anyone uses it?  Thanks!

This is what I tried on Github:
PyGen_NeedsFinalizing -filename:ceval -filename:genobject -filename:gcmodule
msg314809 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2018-04-02 14:32
I think https://searchcode.com/ may have a larger indexing base than GitHub alone.

And, yes, you'll see many duplicates of the CPython source code... Visual inspection may be needed :-/
msg314902 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2018-04-03 21:49
Thanks again, Antoine.  I'll see what I can come up with.  :-)
msg351204 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python committer) Date: 2019-09-05 15:21
My searches show references to this function in CPython cloned repositories. From @pitrous's views, I think it is better to deprecate it with a removal notice for a later release.

I have deprecated the Function instead and will be removed in the next release in this PR https://github.com/python/cpython/pull/15702 .
msg351207 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-09-05 15:54
PyGen_NeedsFinalizing() was added by:

commit 49fd7fa4431da299196d74087df4a04f99f9c46f
Author: Thomas Wouters <thomas@python.org>
Date:   Fri Apr 21 10:40:58 2006 +0000

    Merge p3yk branch with the trunk up to revision 45595. This breaks a fair
    number of tests, all because of the codecs/_multibytecodecs issue described
    here (it's not a Py3K issue, just something Py3K discovers):
    http://mail.python.org/pipermail/python-dev/2006-April/064051.html
    
    Hye-Shik Chang promised to look for a fix, so no need to fix it here. The
    tests that are expected to break are:
    
    test_codecencodings_cn
    test_codecencodings_hk
    test_codecencodings_jp
    test_codecencodings_kr
    test_codecencodings_tw
    test_codecs
    test_multibytecodec
    
    This merge fixes an actual test failure (test_weakref) in this branch,
    though, so I believe merging is the right thing to do anyway.

It was used in this gcmodule.c function:

/* Return true if object has a finalization method.
 * CAUTION:  An instance of an old-style class has to be checked for a
 *__del__ method, and earlier versions of this used to call PyObject_HasAttr,
 * which in turn could call the class's __getattr__ hook (if any).  That
 * could invoke arbitrary Python code, mutating the object graph in arbitrary
 * ways, and that was the source of some excruciatingly subtle bugs.
 */
static int
has_finalizer(PyObject *op)
{
        if (PyInstance_Check(op)) {
                assert(delstr != NULL);
                return _PyInstance_Lookup(op, delstr) != NULL;
        }
        else if (PyType_HasFeature(op->ob_type, Py_TPFLAGS_HEAPTYPE))
                return op->ob_type->tp_del != NULL;
        else if (PyGen_CheckExact(op))
                return PyGen_NeedsFinalizing((PyGenObject *)op);
        else
                return 0;
}


(2) The PEP 442 implementation made PyGen_NeedsFinalizing() useless:

commit 796564c27b8f2e32b9fbc034bbdda75f9507ca43
Author: Antoine Pitrou <solipsis@pitrou.net>
Date:   Tue Jul 30 19:59:21 2013 +0200

    Issue #18112: PEP 442 implementation (safe object finalization).

Replaced:

/* Return true if object has a finalization method. */
static int
has_finalizer(PyObject *op)
{
    if (PyGen_CheckExact(op))
        return PyGen_NeedsFinalizing((PyGenObject *)op);
    else
        return op->ob_type->tp_del != NULL;
}

with:

/* Return true if object has a pre-PEP 442 finalization method. */
static int
has_legacy_finalizer(PyObject *op)
{
    return op->ob_type->tp_del != NULL;
}

--

The last reference to PyGen_NeedsFinalizing() can be found in ceval.c:

        case TARGET(SETUP_FINALLY): {
            /* NOTE: If you add any new block-setup opcodes that
               are not try/except/finally handlers, you may need
               to update the PyGen_NeedsFinalizing() function.
               */

            PyFrame_BlockSetup(f, SETUP_FINALLY, INSTR_OFFSET() + oparg,
                               STACK_LEVEL());
            DISPATCH();
        }
msg351208 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-09-05 15:58
The function is not documented nor tested.

I searched for usage of PyGen_NeedsFinalizing() in C code in GitHub:
https://github.com/search?l=C&p=1&q=PyGen_NeedsFinalizing&type=Code

I only found copies of the CPython code source: PyGen_NeedsFinalizing() definition in genobject.h.

IHMO we can safely remove the function right now. If someone complains, we can reintroduce it later. We just have to document clearly its removal at:
https://docs.python.org/dev/whatsnew/3.9.html#build-and-c-api-changes
msg351258 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-09-06 15:42
New changeset 74b662cf202753d224d82d5503974cce881f7436 by Victor Stinner (Joannah Nanjekye) in branch 'master':
bpo-15088 : Remove PyGen_NeedsFinalizing() (GH-15702)
https://github.com/python/cpython/commit/74b662cf202753d224d82d5503974cce881f7436
msg351259 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-09-06 15:43
I merged Joannah's change: thanks.

If anyone uses the removed function, please complain before Python 3.9.0 release :-)
History
Date User Action Args
2019-09-06 15:43:02vstinnersetstatus: open -> closed
versions: + Python 3.9, - Python 3.7, Python 3.8
title: PyGen_NeedsFinalizing is public, but undocumented -> Remove unused and undocumented PyGen_NeedsFinalizing() function
messages: + msg351259

resolution: fixed
stage: patch review -> resolved
2019-09-06 15:42:01vstinnersetmessages: + msg351258
2019-09-05 15:58:28vstinnersetmessages: + msg351208
2019-09-05 15:54:22vstinnersetnosy: + vstinner
messages: + msg351207
2019-09-05 15:21:03nanjekyejoannahsetnosy: + nanjekyejoannah
messages: + msg351204
2019-09-05 14:15:51nanjekyejoannahsetstage: patch review
pull_requests: + pull_request15356
2018-04-03 21:49:11cheryl.sabellasetmessages: + msg314902
2018-04-02 14:32:36pitrousetmessages: + msg314809
2018-04-02 14:30:19cheryl.sabellasetmessages: + msg314808
2018-04-01 23:41:10pitrousetmessages: + msg314791
2018-03-25 16:36:59cheryl.sabellasetnosy: + cheryl.sabella

messages: + msg314420
versions: + Python 3.7, Python 3.8, - Python 3.2, Python 3.3
2014-04-29 00:04:07pitrousetnosy: + pitrou
messages: + msg217456
2013-04-30 17:06:31plasticgapsetfiles: + Issue15088.diff

nosy: + plasticgap
messages: + msg188170

keywords: + patch
2012-06-17 04:21:15ncoghlansetassignee: docs@python

nosy: + docs@python
components: + Documentation
versions: + Python 3.2, Python 3.3
2012-06-17 04:21:01ncoghlancreate