Skip to content
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.

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

bpo-37879: Fix warnings in _testcapimodule #16004

Merged
merged 2 commits into from Sep 12, 2019

Conversation

@encukou
Copy link
Member

commented Sep 11, 2019

Commit ff023ed introduced some warnings that got through my review. Here is a fix.

https://bugs.python.org/issue37879

@@ -6303,7 +6303,8 @@ heapctypesubclasswithfinalizer_init(PyObject *self, PyObject *args, PyObject *kw
static void
heapctypesubclasswithfinalizer_finalize(PyObject *self)
{
PyObject *error_type, *error_value, *error_traceback, *m, *oldtype, *newtype;
PyObject *error_type, *error_value, *error_traceback, *m;
PyObject *oldtype=NULL, *newtype=NULL;

This comment has been minimized.

Copy link
@corona10

corona10 Sep 12, 2019

Member

nit

PyObject *oldtype = NULL, *newtype = NULL;

Always put spaces around assignment, Boolean and comparison operators.
from PEP7 :)

@@ -6233,7 +6233,7 @@ static PyType_Slot HeapCType_slots[] = {
{Py_tp_init, heapctype_init},
{Py_tp_members, heapctype_members},
{Py_tp_dealloc, heapctype_dealloc},
{Py_tp_doc, heapctype__doc__},
{Py_tp_doc, (char*)heapctype__doc__},

This comment has been minimized.

Copy link
@corona10

corona10 Sep 12, 2019

Member

{Py_tp_doc, (void*)s__doc__},

There are some cases that handling type casting with Py_tp_doc
They are using void*. How about casting to void*?

This comment has been minimized.

Copy link
@encukou

encukou Sep 12, 2019

Author Member

Either will work.
Ideally we'd want all the slots to be const, but some of them are PyObjects with mutable refcounts :(

@encukou encukou merged commit 5e9caee into python:master Sep 12, 2019

4 checks passed

Azure Pipelines PR #20190912.17 succeeded
Details
bedevere/issue-number Issue number 37879 found
Details
bedevere/news "skip news" label found
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@encukou encukou deleted the encukou:fix-warnings branch Sep 12, 2019

encukou added a commit to encukou/cpython that referenced this pull request Sep 12, 2019
@encukou

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

Thanks for the review!

encukou added a commit that referenced this pull request Sep 12, 2019
[3.8] bpo-37879: Suppress subtype_dealloc decref when base type is a …
…C heap type (GH-15323, GH-16004) (GH-15966)

The instance destructor for a type is responsible for preparing
an instance for deallocation by decrementing the reference counts
of its referents.

If an instance belongs to a heap type, the type object of an instance
has its reference count decremented while for static types, which
are permanently allocated, the type object is unaffected by the
instance destructor.

Previously, the default instance destructor searched the class
hierarchy for an inherited instance destructor and, if present,
would invoke it.

Then, if the instance type is a heap type, it would decrement the
reference count of that heap type.  However, this could result in the
premature destruction of a type because the inherited instance
destructor should have already decremented the reference count
of the type object.

This change avoids the premature destruction of the type object
by suppressing the decrement of its reference count when an
inherited, non-default instance destructor has been invoked.

Finally, an assertion on the Py_SIZE of a type was deleted.  Heap
types have a non zero size, making this into an incorrect assertion.

#15323.
(cherry picked from commit ff023ed)
Fixup: #16004.
(cherry picked from commit 5e9caee)

Co-authored-by: Eddie Elizondo <eduardo.elizondorueda@gmail.com>
vrpolakatcisco added a commit to vrpolakatcisco/cpython that referenced this pull request Sep 12, 2019
DinoV added a commit to DinoV/cpython that referenced this pull request Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.