classification
Title: Compiler warnings in _PyObject_CallArg1()
Type: Stage: resolved
Components: Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, haypo, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-12-02 00:38 by haypo, last changed 2016-12-02 11:35 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
static_inline.patch haypo, 2016-12-02 00:38 review
Messages (6)
msg282212 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-12-02 00:38
_PyObject_CallArg1() is the following macro:
---
#define _PyObject_CallArg1(func, arg) \
    _PyObject_FastCall((func), &(arg), 1)
---

It works well in most cases, but my change 8f258245c391 or change b9c9691c72c5 added compiler warnings, because an argument which is not directly a PyObject* type is passed as "arg".

I tried to cast in the caller: _PyObject_CallArg1(func, (PyObject*)arg), but sadly it doesn't work :-( I get a compiler error.

Another option is to cast after "&" in the macro:
---
 #define _PyObject_CallArg1(func, arg) \
-    _PyObject_FastCall((func), &(arg), 1)
+    _PyObject_FastCall((func), (PyObject **)&(arg), 1)
---

This option may hide real bugs, so I dislike it.

A better option is to stop using ugly C macros and use a modern static inline function: see attached static_inline.patch. This patch casts to PyObject* before calling _PyObject_CallArg1() to fix warnings.

The question is if compilers are able to emit efficient code for static inline functions using "&" on an argument. I wrote the macro to implement the "&" optimization: convert a PyObject* to a stack of arguments: C array of PyObject* objects.
msg282220 - (view) Author: Roundup Robot (python-dev) Date: 2016-12-02 06:01
New changeset 96245d4af0ca by Benjamin Peterson in branch 'default':
fix _PyObject_CallArg1 compiler warnings (closes #28855)
https://hg.python.org/cpython/rev/96245d4af0ca
msg282221 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2016-12-02 06:08
It doesn't seem like the question is whether to use inline functions but whether to force all callers to cast. Your original code would work if you added all the casts in your static_inline.patch patch.
msg282222 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2016-12-02 06:24
(Sorry, I noticed and landed a fix before completely reading the issue.)
msg282230 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2016-12-02 07:59
I also think forcing callers to cast is fine. Most of our APIs require PyObject *.
msg282237 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-12-02 11:35
No problem, thanks for the fix Benjamin!
History
Date User Action Args
2016-12-02 11:35:37hayposetmessages: + msg282237
2016-12-02 07:59:18benjamin.petersonsetmessages: + msg282230
2016-12-02 06:24:14benjamin.petersonsetmessages: + msg282222
2016-12-02 06:08:34benjamin.petersonsetmessages: + msg282221
2016-12-02 06:01:42python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg282220

resolution: fixed
stage: resolved
2016-12-02 00:38:43haypocreate