Skip to content

bpo-33012: Fix invalid function cast warnings with gcc 8 for METH_NOARGS #6030

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

Merged
merged 6 commits into from
Apr 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Doc/extending/newtypes_tutorial.rst
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ We define a single method, :meth:`Custom.name()`, that outputs the objects name
concatenation of the first and last names. ::

static PyObject *
Custom_name(CustomObject *self)
Custom_name(CustomObject *self, PyObject *Py_UNUSED(ignored))
{
if (self->first == NULL) {
PyErr_SetString(PyExc_AttributeError, "first");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
gcc 8 has added a new warning heuristic to detect invalid function casts and
a stock python build seems to hit that warning quite often. The most common
is the cast of a METH_NOARGS function (that uses just one argument) to a
PyCFunction. Fix this by adding a dummy argument to all functions that
implement METH_NOARGS.
26 changes: 13 additions & 13 deletions Modules/_collectionsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ deque_inplace_concat(dequeobject *deque, PyObject *other)
}

static PyObject *
deque_copy(PyObject *deque)
deque_copy(PyObject *deque, PyObject *Py_UNUSED(ignored))
Copy link
Member

Choose a reason for hiding this comment

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

Explicit cast to PyCFunction at line 1590 is not needed.

The same with other functions which exactly match the signature of PyCFunction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've kept the casts in place because we will eventually need it. To fully fix the warnings, the ml_meth type should be like so:

PyObject *(*) (PyObject *, PyObject *, ...)

so that it matches with all of the allowed method types provided that there is an explicit cast in place. Right now I am not sure if I should change the PyCFunction type to that or to make a new PyCFunctionAny just for ml_meth. Since we're on this topic, what do you think is the preferred change? I suppose both change API, but not in a completely incompatible way.

Copy link
Member

Choose a reason for hiding this comment

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

The casts are not needed if the signature matches PyCFunction. It is still needed if the function has type PyCFunctionWithKeywords, PyNoArgsFunction, etc. Removing unneeded casts will clean up the code and can prevent introducing new bugs when the signature be changed unintentionally.

{
dequeobject *old_deque = (dequeobject *)deque;
if (Py_TYPE(deque) == &deque_type) {
Expand Down Expand Up @@ -563,7 +563,7 @@ deque_concat(dequeobject *deque, PyObject *other)
return NULL;
}

new_deque = deque_copy((PyObject *)deque);
new_deque = deque_copy((PyObject *)deque, NULL);
if (new_deque == NULL)
return NULL;
result = deque_extend((dequeobject *)new_deque, other);
Expand Down Expand Up @@ -659,7 +659,7 @@ deque_clear(dequeobject *deque)
}

static PyObject *
deque_clearmethod(dequeobject *deque)
deque_clearmethod(dequeobject *deque, PyObject *Py_UNUSED(ignored))
{
deque_clear(deque);
Py_RETURN_NONE;
Expand Down Expand Up @@ -754,7 +754,7 @@ deque_repeat(dequeobject *deque, Py_ssize_t n)
dequeobject *new_deque;
PyObject *rv;

new_deque = (dequeobject *)deque_copy((PyObject *) deque);
new_deque = (dequeobject *)deque_copy((PyObject *) deque, NULL);
if (new_deque == NULL)
return NULL;
rv = deque_inplace_repeat(new_deque, n);
Expand Down Expand Up @@ -1576,7 +1576,7 @@ static PyNumberMethods deque_as_number = {
};

static PyObject *deque_iter(dequeobject *deque);
static PyObject *deque_reviter(dequeobject *deque);
static PyObject *deque_reviter(dequeobject *deque, PyObject *Py_UNUSED(ignored));
PyDoc_STRVAR(reversed_doc,
"D.__reversed__() -- return a reverse iterator over the deque");

Expand All @@ -1587,9 +1587,9 @@ static PyMethodDef deque_methods[] = {
METH_O, appendleft_doc},
{"clear", (PyCFunction)deque_clearmethod,
METH_NOARGS, clear_doc},
{"__copy__", (PyCFunction)deque_copy,
{"__copy__", deque_copy,
METH_NOARGS, copy_doc},
{"copy", (PyCFunction)deque_copy,
{"copy", deque_copy,
METH_NOARGS, copy_doc},
{"count", (PyCFunction)deque_count,
METH_O, count_doc},
Expand Down Expand Up @@ -1774,15 +1774,15 @@ dequeiter_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
}

static PyObject *
dequeiter_len(dequeiterobject *it)
dequeiter_len(dequeiterobject *it, PyObject *Py_UNUSED(ignored))
{
return PyLong_FromSsize_t(it->counter);
}

PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list(it)).");

static PyObject *
dequeiter_reduce(dequeiterobject *it)
dequeiter_reduce(dequeiterobject *it, PyObject *Py_UNUSED(ignored))
{
return Py_BuildValue("O(On)", Py_TYPE(it), it->deque, Py_SIZE(it->deque) - it->counter);
}
Expand Down Expand Up @@ -1841,7 +1841,7 @@ static PyTypeObject dequeiter_type = {
static PyTypeObject dequereviter_type;

static PyObject *
deque_reviter(dequeobject *deque)
deque_reviter(dequeobject *deque, PyObject *Py_UNUSED(ignored))
{
dequeiterobject *it;

Expand Down Expand Up @@ -1896,7 +1896,7 @@ dequereviter_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
return NULL;
assert(type == &dequereviter_type);

it = (dequeiterobject*)deque_reviter((dequeobject *)deque);
it = (dequeiterobject*)deque_reviter((dequeobject *)deque, NULL);
if (!it)
return NULL;
/* consume items from the queue */
Expand Down Expand Up @@ -2001,7 +2001,7 @@ defdict_missing(defdictobject *dd, PyObject *key)
PyDoc_STRVAR(defdict_copy_doc, "D.copy() -> a shallow copy of D.");

static PyObject *
defdict_copy(defdictobject *dd)
defdict_copy(defdictobject *dd, PyObject *Py_UNUSED(ignored))
{
/* This calls the object's class. That only works for subclasses
whose class constructor has the same signature. Subclasses that
Expand All @@ -2015,7 +2015,7 @@ defdict_copy(defdictobject *dd)
}

static PyObject *
defdict_reduce(defdictobject *dd)
defdict_reduce(defdictobject *dd, PyObject *Py_UNUSED(ignored))
{
/* __reduce__ must return a 5-tuple as follows:

Expand Down
42 changes: 21 additions & 21 deletions Modules/_datetimemodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2590,7 +2590,7 @@ delta_getstate(PyDateTime_Delta *self)
}

static PyObject *
delta_total_seconds(PyObject *self)
delta_total_seconds(PyObject *self, PyObject *Py_UNUSED(ignored))
{
PyObject *total_seconds;
PyObject *total_microseconds;
Expand All @@ -2606,7 +2606,7 @@ delta_total_seconds(PyObject *self)
}

static PyObject *
delta_reduce(PyDateTime_Delta* self)
delta_reduce(PyDateTime_Delta* self, PyObject *Py_UNUSED(ignored))
{
return Py_BuildValue("ON", Py_TYPE(self), delta_getstate(self));
}
Expand All @@ -2627,7 +2627,7 @@ static PyMemberDef delta_members[] = {
};

static PyMethodDef delta_methods[] = {
{"total_seconds", (PyCFunction)delta_total_seconds, METH_NOARGS,
{"total_seconds", delta_total_seconds, METH_NOARGS,
PyDoc_STR("Total seconds in the duration.")},

{"__reduce__", (PyCFunction)delta_reduce, METH_NOARGS,
Expand Down Expand Up @@ -2991,7 +2991,7 @@ date_repr(PyDateTime_Date *self)
}

static PyObject *
date_isoformat(PyDateTime_Date *self)
date_isoformat(PyDateTime_Date *self, PyObject *Py_UNUSED(ignored))
{
return PyUnicode_FromFormat("%04d-%02d-%02d",
GET_YEAR(self), GET_MONTH(self), GET_DAY(self));
Expand All @@ -3006,7 +3006,7 @@ date_str(PyDateTime_Date *self)


static PyObject *
date_ctime(PyDateTime_Date *self)
date_ctime(PyDateTime_Date *self, PyObject *Py_UNUSED(ignored))
{
return format_ctime(self, 0, 0, 0);
}
Expand Down Expand Up @@ -3055,15 +3055,15 @@ date_format(PyDateTime_Date *self, PyObject *args)
/* ISO methods. */

static PyObject *
date_isoweekday(PyDateTime_Date *self)
date_isoweekday(PyDateTime_Date *self, PyObject *Py_UNUSED(ignored))
{
int dow = weekday(GET_YEAR(self), GET_MONTH(self), GET_DAY(self));

return PyLong_FromLong(dow + 1);
}

static PyObject *
date_isocalendar(PyDateTime_Date *self)
date_isocalendar(PyDateTime_Date *self, PyObject *Py_UNUSED(ignored))
{
int year = GET_YEAR(self);
int week1_monday = iso_week1_monday(year);
Expand Down Expand Up @@ -3100,7 +3100,7 @@ date_richcompare(PyObject *self, PyObject *other, int op)
}

static PyObject *
date_timetuple(PyDateTime_Date *self)
date_timetuple(PyDateTime_Date *self, PyObject *Py_UNUSED(ignored))
{
return build_struct_time(GET_YEAR(self),
GET_MONTH(self),
Expand Down Expand Up @@ -3149,14 +3149,14 @@ date_hash(PyDateTime_Date *self)
}

static PyObject *
date_toordinal(PyDateTime_Date *self)
date_toordinal(PyDateTime_Date *self, PyObject *Py_UNUSED(ignored))
{
return PyLong_FromLong(ymd_to_ord(GET_YEAR(self), GET_MONTH(self),
GET_DAY(self)));
}

static PyObject *
date_weekday(PyDateTime_Date *self)
date_weekday(PyDateTime_Date *self, PyObject *Py_UNUSED(ignored))
{
int dow = weekday(GET_YEAR(self), GET_MONTH(self), GET_DAY(self));

Expand Down Expand Up @@ -3435,7 +3435,7 @@ tzinfo_fromutc(PyDateTime_TZInfo *self, PyObject *dt)
*/

static PyObject *
tzinfo_reduce(PyObject *self)
tzinfo_reduce(PyObject *self, PyObject *Py_UNUSED(ignored))
{
PyObject *args, *state;
PyObject *getinitargs, *getstate;
Expand Down Expand Up @@ -3502,7 +3502,7 @@ static PyMethodDef tzinfo_methods[] = {
{"fromutc", (PyCFunction)tzinfo_fromutc, METH_O,
PyDoc_STR("datetime in UTC -> datetime in local time.")},

{"__reduce__", (PyCFunction)tzinfo_reduce, METH_NOARGS,
{"__reduce__", tzinfo_reduce, METH_NOARGS,
PyDoc_STR("-> (cls, state)")},

{NULL, NULL}
Expand Down Expand Up @@ -3720,7 +3720,7 @@ timezone_fromutc(PyDateTime_TimeZone *self, PyDateTime_DateTime *dt)
}

static PyObject *
timezone_getinitargs(PyDateTime_TimeZone *self)
timezone_getinitargs(PyDateTime_TimeZone *self, PyObject *Py_UNUSED(ignored))
{
if (self->name == NULL)
return Py_BuildValue("(O)", self->offset);
Expand Down Expand Up @@ -5173,7 +5173,7 @@ datetime_isoformat(PyDateTime_DateTime *self, PyObject *args, PyObject *kw)
}

static PyObject *
datetime_ctime(PyDateTime_DateTime *self)
datetime_ctime(PyDateTime_DateTime *self, PyObject *Py_UNUSED(ignored))
{
return format_ctime((PyDateTime_Date *)self,
DATE_GET_HOUR(self),
Expand Down Expand Up @@ -5652,7 +5652,7 @@ datetime_astimezone(PyDateTime_DateTime *self, PyObject *args, PyObject *kw)
}

static PyObject *
datetime_timetuple(PyDateTime_DateTime *self)
datetime_timetuple(PyDateTime_DateTime *self, PyObject *Py_UNUSED(ignored))
{
int dstflag = -1;

Expand Down Expand Up @@ -5727,7 +5727,7 @@ local_to_seconds(int year, int month, int day,
#define EPOCH_SECONDS (719163LL * 24 * 60 * 60)

static PyObject *
datetime_timestamp(PyDateTime_DateTime *self)
datetime_timestamp(PyDateTime_DateTime *self, PyObject *Py_UNUSED(ignored))
{
PyObject *result;

Expand All @@ -5736,7 +5736,7 @@ datetime_timestamp(PyDateTime_DateTime *self)
delta = datetime_subtract((PyObject *)self, PyDateTime_Epoch);
if (delta == NULL)
return NULL;
result = delta_total_seconds(delta);
result = delta_total_seconds(delta, NULL);
Py_DECREF(delta);
}
else {
Expand All @@ -5757,15 +5757,15 @@ datetime_timestamp(PyDateTime_DateTime *self)
}

static PyObject *
datetime_getdate(PyDateTime_DateTime *self)
datetime_getdate(PyDateTime_DateTime *self, PyObject *Py_UNUSED(ignored))
{
return new_date(GET_YEAR(self),
GET_MONTH(self),
GET_DAY(self));
}

static PyObject *
datetime_gettime(PyDateTime_DateTime *self)
datetime_gettime(PyDateTime_DateTime *self, PyObject *Py_UNUSED(ignored))
{
return new_time(DATE_GET_HOUR(self),
DATE_GET_MINUTE(self),
Expand All @@ -5776,7 +5776,7 @@ datetime_gettime(PyDateTime_DateTime *self)
}

static PyObject *
datetime_gettimetz(PyDateTime_DateTime *self)
datetime_gettimetz(PyDateTime_DateTime *self, PyObject *Py_UNUSED(ignored))
{
return new_time(DATE_GET_HOUR(self),
DATE_GET_MINUTE(self),
Expand All @@ -5787,7 +5787,7 @@ datetime_gettimetz(PyDateTime_DateTime *self)
}

static PyObject *
datetime_utctimetuple(PyDateTime_DateTime *self)
datetime_utctimetuple(PyDateTime_DateTime *self, PyObject *Py_UNUSED(ignored))
{
int y, m, d, hh, mm, ss;
PyObject *tzinfo;
Expand Down
2 changes: 1 addition & 1 deletion Modules/_io/bytesio.c
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ _io_BytesIO_close_impl(bytesio *self)
*/

static PyObject *
bytesio_getstate(bytesio *self)
bytesio_getstate(bytesio *self, PyObject *Py_UNUSED(ignored))
{
PyObject *initvalue = _io_BytesIO_getvalue_impl(self);
PyObject *dict;
Expand Down
2 changes: 1 addition & 1 deletion Modules/_io/fileio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1123,7 +1123,7 @@ _io_FileIO_isatty_impl(fileio *self)
}

static PyObject *
fileio_getstate(fileio *self)
fileio_getstate(fileio *self, PyObject *Py_UNUSED(ignored))
{
PyErr_Format(PyExc_TypeError,
"cannot serialize '%s' object", Py_TYPE(self)->tp_name);
Expand Down
2 changes: 1 addition & 1 deletion Modules/_io/stringio.c
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ _io_StringIO_seekable_impl(stringio *self)
*/

static PyObject *
stringio_getstate(stringio *self)
stringio_getstate(stringio *self, PyObject *Py_UNUSED(ignored))
{
PyObject *initvalue = _io_StringIO_getvalue_impl(self);
PyObject *dict;
Expand Down
4 changes: 2 additions & 2 deletions Modules/_io/textio.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ PyDoc_STRVAR(textiobase_detach_doc,
);

static PyObject *
textiobase_detach(PyObject *self)
textiobase_detach(PyObject *self, PyObject *Py_UNUSED(ignored))
{
return _unsupported("detach");
}
Expand Down Expand Up @@ -148,7 +148,7 @@ textiobase_errors_get(PyObject *self, void *context)


static PyMethodDef textiobase_methods[] = {
{"detach", (PyCFunction)textiobase_detach, METH_NOARGS, textiobase_detach_doc},
{"detach", textiobase_detach, METH_NOARGS, textiobase_detach_doc},
{"read", textiobase_read, METH_VARARGS, textiobase_read_doc},
{"readline", textiobase_readline, METH_VARARGS, textiobase_readline_doc},
{"write", textiobase_write, METH_VARARGS, textiobase_write_doc},
Expand Down
2 changes: 1 addition & 1 deletion Modules/_io/winconsoleio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ _io__WindowsConsoleIO_isatty_impl(winconsoleio *self)
}

static PyObject *
winconsoleio_getstate(winconsoleio *self)
winconsoleio_getstate(winconsoleio *self, PyObject *Py_UNUSED(ignored))
{
PyErr_Format(PyExc_TypeError,
"cannot serialize '%s' object", Py_TYPE(self)->tp_name);
Expand Down
Loading