[issue43181] Python macros don’t shield arguments

[STINNER Victor]

STINNER Victor  added the comment:

Me:
> I don't think that PR 24533 should be backported to Python 3.8 and Python 
> 3.9. I prefer to avoid any risk of regression, and so only change Python 3.10.

Sometimes. I'm wise :-D This change broken 2 extension modules which rely on 
the fact that the macro magically add parenthesis:

python-cvxopt: "if Matrix_Check((PyObject *)val)" with: "#define 
Matrix_Check(self) PyObject_TypeCheck(self, _tp)"
https://bugzilla.redhat.com/show_bug.cgi?id=1941557
PR proposed: https://github.com/cvxopt/cvxopt/pull/190

python-Bottleneck: "if PyArray_Check(a_obj) {" with: "#define PyArray_Check(op) 
PyObject_TypeCheck(op, _Type)"
https://bugzilla.redhat.com/show_bug.cgi?id=1941559

"if func(...) {" is not valid C code, but the old macro added magically 
parenthesis!

It's hard to say if the Python change is a backward incompatible or not... I 
prefer to keep it and fix the few broken C extensions.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[STINNER Victor]

STINNER Victor  added the comment:

You can create a single issue, and later we will see if it's needed to split it 
into sub-issues.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

> If possible, I would prefer to only replace macros with static inline 
> functions if the changes avoids clear macro pitfalls.

Yes, of course. Should I create a meta issue for this, or do you prefer raising 
one issue pr. fix?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[STINNER Victor]

STINNER Victor  added the comment:

If possible, I would prefer to only replace macros with static inline functions 
if the changes avoids clear macro pitfalls. It's not because macros have 
pitfalls that we must replace them all blindly.

Also, this issue is closed. At this point, I'm not convinced that it's worth it 
to fix PyObject_TypeCheck() in Python 3.8 and 3.9, so I leave the issue closed.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

> As I wrote previously, I dislike macros. If someone is changed, I would 
> prefer to convert the function into a static inline function which doesn't 
> have macros pitfalls.

Should we create a meta issue for this? Most macros are trivial, and can safely 
be converted, given that they're not used as l-values. Converting one header 
file at the time would make it easy to review, and it would be possible to make 
good progress in a few months time.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[STINNER Victor]

STINNER Victor  added the comment:

> Yet... the first argument is still unshielded, passed to a macro that expects 
> one single macro argument.

Are you talking about the current code in the master branch or the 3.9 branch? 
If you are talking about the _PyObject_CAST(ob) call, would you mind to 
elaborate how it is an issue? The code in master LGTM.

> That’s not a regression, it wasn’t shielded in 3.8 either, but why not just 
> parenthesise each macro argument that denotes an expression (as opposed to 
> e.g. name)?

In the master branch, Include/ and its subdirectories contains 158 .h files for 
a total of 20K lines of C code. It's not easy to check all files. If you 
spotted bugs, would you mind to give examples, or a way to automatically detect 
bugs?

As I wrote previously, I dislike macros. If someone is changed, I would prefer 
to convert the function into a static inline function which doesn't have macros 
pitfalls.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Vitaliy]

Vitaliy  added the comment:

Thanks for the fix, it works for my use case. (btw that was
#define U(...) __VA_ARGS__
and not what I wrote).

> I don't think that PR 24533 should be backported to Python 3.8 and Python 
> 3.9. I prefer to avoid any risk of regression, and so only change Python 3.10.

> For Python 3.9 and older, a workaround is to wrap the call to 
> PyObject_TypeCheck() with your own static inline function.

For Python 3.8 that fix wouldn’t be needed as the `tp` argument was 
parenthesised in the macro.

Yet... the first argument is still unshielded, passed to a macro that expects 
one single macro argument. That’s not a regression, it wasn’t shielded in 3.8 
either, but why not just parenthesise each macro argument that denotes an 
expression (as opposed to e.g. name)?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[STINNER Victor]

STINNER Victor  added the comment:

Thanks Vitaliy for the bug report and Erlend for the fix ;-)

For Python 3.9 and older, a workaround is to wrap the call to 
PyObject_TypeCheck() with your own static inline function.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 4bb2a1ebc569eee6f1b46ecef1965a26ae8cb76d by Erlend Egeberg 
Aasland in branch 'master':
bpo-43181: Convert PyObject_TypeCheck to static inline function (GH-24533)
https://github.com/python/cpython/commit/4bb2a1ebc569eee6f1b46ecef1965a26ae8cb76d


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

> I don't think that PR 24533 should be backported to Python 3.8 and Python 
> 3.9. I prefer to avoid any risk of regression, and so only change Python 3.10.

That sounds reasonable.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[STINNER Victor]

STINNER Victor  added the comment:

I don't think that PR 24533 should be backported to Python 3.8 and Python 3.9. 
I prefer to avoid any risk of regression, and so only change Python 3.10.

--
versions:  -Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Erlend Egeberg Aasland]

Change by Erlend Egeberg Aasland :


--
keywords: +patch
pull_requests: +23320
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/24533

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

Silence implies consent? I'll throw up a PR, then :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

> Are you interested to convert the PyObject_TypeCheck() macro to a static 
> inline function?

I'd like to give it a shot if it's ok for you all.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Erlend Egeberg Aasland]

Change by Erlend Egeberg Aasland :


--
nosy: +erlendaasland

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[STINNER Victor]

STINNER Victor  added the comment:

I'm replacing macros with static inline functions to avoid issues like this one 
(ex: bpo-35059). Are you interested to convert the PyObject_TypeCheck() macro 
to a static inline function?

"""
There is a lot of macros like:
#define PyObject_TypeCheck(ob, tp) \
(Py_IS_TYPE(ob, tp) || PyType_IsSubtype(Py_TYPE(ob), (tp)))
These work fine until an argument happen to contain a comma.
"""

This macro also has a common bug of macros: if one argument has a side effect, 
it is executed twice! For example, if an argument is a function call, the 
function is called twice.

See:

* https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html
* https://vstinner.github.io/split-include-directory-python38.html "Convert 
macros to static inline functions"

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Gregory P. Smith]

Change by Gregory P. Smith :


--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Gregory P. Smith]

Gregory P. Smith  added the comment:

This bug is valid, that macro and likely others aren't up to best practices.

C macros are a PITA to get right.

--
keywords: +3.9regression
nosy: +gregory.p.smith
stage:  -> needs patch
type:  -> compile error
versions: +Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[William Pickard]

William Pickard  added the comment:

This feels like it's more of an issue with the C++ compiler you're using. (I 
can tell it's C++ because of template syntax)

--
nosy: +WildCard65

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43181] Python macros don’t shield arguments

[Vitaliy]

New submission from Vitaliy :

There is a lot of macros like:
#define PyObject_TypeCheck(ob, tp) \
(Py_IS_TYPE(ob, tp) || PyType_IsSubtype(Py_TYPE(ob), (tp)))
These work fine until an argument happen to contain a comma. That’s possible as 
a result of other macro’s expansion. E.g. if U(x) is defined as x,
PyObject_TypeCheck(ob, U(f(c)))
expands to
(Py_IS_TYPE(ob, f(c)) || ...)
but < and > aren’t treated as brackets by the preprocessor so Py_IS_TYPE is now 
invoked with 3 arguments instead of just 2, breaking module compilation.

As arguments are expected to be values, surrounding each with parentheses 
solves the problem. But there are many such macros so that’s not an one-line 
fix.

Note: the example is from PyGLM (simplified), it doesn’t compile on 3.9 due to 
this issue.

--
components: C API
messages: 386746
nosy: numberZero
priority: normal
severity: normal
status: open
title: Python macros don’t shield arguments
versions: Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com