classification
Title: Argument Clinic: generate second arg for METH_NOARGS
Type: behavior Stage: resolved
Components: Demos and Tools Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: larry, python-dev, skrah, vstinner
Priority: normal Keywords:

Created on 2013-12-13 14:43 by skrah, last changed 2017-11-08 15:59 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
larry.clinic.fix.meth_noargs.1.diff.txt larry, 2013-12-15 17:18 review
Pull Requests
URL Status Linked Edit
PR 4341 merged encukou, 2017-11-08 15:51
Messages (14)
msg206093 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-12-13 14:43
I was just reading the _pickle sources and it appears that AC does not
generate a second arg for METH_NOARGS functions:

#define _PICKLE_PICKLERMEMOPROXY_CLEAR_METHODDEF    \
    {"clear", (PyCFunction)_pickle_PicklerMemoProxy_clear, METH_NOARGS, _pickle_PicklerMemoProxy_clear__doc__},

static PyObject *
_pickle_PicklerMemoProxy_clear(PicklerMemoProxyObject *self)


While this is a common occurrence in the source tree, the consensus
in #15402 was that the unused second arg should be present, e.g.:

msg166250
msg166405
msg206095 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-12-13 15:11
I prefer to omit the unused parameter, even if NULL *is* passed to the function.

If you prefer to add the unused parameter, what do you propose to avoid compiler warnings if unused parameters are checked?
msg206096 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2013-12-13 15:28
Stefan is right.  I'll fix Clinic.
msg206102 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-12-13 16:00
STINNER Victor <report@bugs.python.org> wrote:
> If you prefer to add the unused parameter, what do you propose to avoid compiler warnings if unused parameters are checked?

This works quite portably in _decimal (I don't get warnings from gcc, icc,
suncc, Visual Studio, aCC, clang):

#if defined(__GNUC__) && !defined(__INTEL_COMPILER)
  #define UNUSED __attribute__((unused))
#else
  #define UNUSED
#endif

static PyObject *
signaldict_copy(PyObject *self, PyObject *args UNUSED)
{
    return flags_as_dict(SdFlags(self));
}

We could call the macro PY_UNUSED or something.
msg206103 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2013-12-13 16:03
A quick google suggests:

http://sourcefrog.net/weblog/software/languages/C/unused.html
msg206104 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2013-12-13 16:06
To do it properly with Clang requires a pragma:

http://stackoverflow.com/questions/3417837/what-is-the-best-way-to-supress-unused-variable-x-warning/18724213#18724213

What a mess.
msg206105 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-12-13 16:11
> We could call the macro PY_UNUSED or something.

I would prefer Py_UNUSED name. This sounds like a nice addition to Include/pymacros.h.

In C++, you can omit the parameter name, so the macro should take the parameter name: Py_UNUSED(name). Example:

   int foo(int Py_UNUSED(bar)) { return 1 }

In Visual Studio, you can use:

#define Py_UNUSED(NAME) __pragma(warning(suppress:4100)) NAME

For Clang, you can try:

#define Py_UNUSED(NAME) _Pragma(diagnostic ignored "-Wunused") NAME
msg206106 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-12-13 16:15
Larry Hastings <report@bugs.python.org> wrote:
> To do it properly with Clang requires a pragma:

Hmm. I just tested and clang warns with -Wall -W, but does not warn if
__attribute__((unused)) is present.

The macro I posted really works on all obscure buildbot platforms.
msg206108 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-12-13 16:17
Stefan Krah <report@bugs.python.org> wrote:
> The macro I posted really works on all obscure buildbot platforms.

N.B. clang also defines __GNUC__, as does the intel compiler.
msg206110 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-12-13 16:33
STINNER Victor <report@bugs.python.org> wrote:
> I would prefer Py_UNUSED name. This sounds like a nice addition to Include/pymacros.h.

Yes, Py_UNUSED is nicer.
msg206244 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2013-12-15 17:18
Here's a first attempt at a patch.

The Visual Studio pragma disables for the rest of the file, which is undesirable.  Maybe we could turn it on and off inline, but it's not clear to me that that would have the desired effect of turning off the warning for explicitly that parameter declaration.

Also, a little googling confirms that clang supports the GCC __attribute((unused)) extension, so I just went with that.
msg206281 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-12-16 08:50
> The Visual Studio pragma disables for the rest of the file, which is undesirable.  Maybe we could turn it on and off inline, but it's not clear to me that that would have the desired effect of turning off the warning for explicitly that parameter declaration.

Oh, I didn't know that it is file-wide. There are
__pragma(warning(push)) and __pragma(warning(pop)) commands to disable
a pragma. I don't know it is can be used using Py_UNUSED(name) macro
(is it possible to "pop" the pragma before the function body?).

If a compiler does not provide a syntax to disable the warning just in
one function, the warning should be disabled for the compilation of
the whole project.
msg207304 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-04 19:09
New changeset c4ababa110a2 by Larry Hastings in branch 'default':
Issue #19976: Argument Clinic METH_NOARGS functions now always
http://hg.python.org/cpython/rev/c4ababa110a2
msg305867 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-11-08 15:59
New changeset 2138163621196a186975796afb2b0a6aa335231d by Victor Stinner (Petr Viktorin) in branch 'master':
bpo-29179: Document the Py_UNUSED macro (#4341)
https://github.com/python/cpython/commit/2138163621196a186975796afb2b0a6aa335231d
History
Date User Action Args
2017-11-08 15:59:23vstinnersetmessages: + msg305867
2017-11-08 15:51:37encukousetpull_requests: + pull_request4296
2014-01-10 02:00:58larrysetstatus: open -> closed
assignee: larry
resolution: fixed
stage: resolved
2014-01-04 19:09:29python-devsetnosy: + python-dev
messages: + msg207304
2013-12-16 08:50:34vstinnersetmessages: + msg206281
2013-12-15 17:18:06larrysetfiles: + larry.clinic.fix.meth_noargs.1.diff.txt

messages: + msg206244
2013-12-13 16:33:34skrahsetmessages: + msg206110
2013-12-13 16:17:20skrahsetmessages: + msg206108
2013-12-13 16:15:36skrahsetmessages: + msg206106
2013-12-13 16:11:42vstinnersetmessages: + msg206105
2013-12-13 16:06:34larrysetmessages: + msg206104
2013-12-13 16:03:58larrysetmessages: + msg206103
2013-12-13 16:00:24skrahsetmessages: + msg206102
2013-12-13 15:28:56larrysetmessages: + msg206096
2013-12-13 15:11:08vstinnersetnosy: + vstinner
messages: + msg206095
2013-12-13 14:43:06skrahcreate