[issue45325] Allow "p" in Py_BuildValue

Jelle Zijlstra added the comment: I stumbled upon the PR and tend to agree with Serhiy that this could be a source of nasty bugs. Passing the wrong type to a va_arg() argument is undefined behavior (e.g.

[issue45325] Allow "p" in Py_BuildValue

Matt Wozniski added the comment: > "!value" or "!!value" also has the issue if I understood correctly. No, just as "value != 0" is an int, so is "!value". -- ___ Python tracker

[issue45325] Allow "p" in Py_BuildValue

Pablo Galindo Salgado added the comment: > But I am not in strict opposition against this feature. It is just that I > have fixed so many bugs, that I try to search potential flaws and drawbacks > in any new feature. Now you know about this, and you can decide whether the > benefit is

[issue45325] Allow "p" in Py_BuildValue

STINNER Victor added the comment: > I think that the risk for other formatting parameters is smaller, because you > know, that there are different formatting parameters for different integer > and floating point types, and for pointers, and you know that you should care > about truncation

[issue45325] Allow "p" in Py_BuildValue

Serhiy Storchaka added the comment: > But isn't that risk the same for other formatting parameters? I think that the risk for other formatting parameters is smaller, because you know, that there are different formatting parameters for different integer and floating point types, and for

[issue45325] Allow "p" in Py_BuildValue

Matt Wozniski added the comment: The leftmost argument of the ternary is an int for every example that Victor and I found in the stdlib, so no casting would be required in any of these cases. -- ___ Python tracker

[issue45325] Allow "p" in Py_BuildValue

Matt Wozniski added the comment: I spotted three other uses in the stdlib: Modules/_io/_iomodule.c raw = PyObject_CallFunction(RawIO_class, "OsOO", path_or_fd, rawmode, closefd ? Py_True : Py_False,

[issue45325] Allow "p" in Py_BuildValue

Matt Wozniski added the comment: > but there is a catch -- the arguments should be a C int Or a type that promotes to int. If you pass a C short or char, or a C++ bool, it is implicitly promoted to int. > so you will need to write "expr ? 1 : 0" Or alternatively "!!expr" > which is not

[issue45325] Allow "p" in Py_BuildValue

Pablo Galindo Salgado added the comment: > My concern is that such feature can provoke bugs. The risk can exceed the > minor benefit. But isn't that risk the same for other formatting parameters? -- ___ Python tracker

[issue45325] Allow "p" in Py_BuildValue

Pablo Galindo Salgado added the comment: You can see how is implemented on am64 for example here: https://blog.nelhage.com/2010/10/amd64-and-va_arg/ -- ___ Python tracker

[issue45325] Allow "p" in Py_BuildValue

Serhiy Storchaka added the comment: > What happens if you pass a double, it is stored as a double on the C stack, > and then Py_BuildValue() will read junk data? AFAIK, it is complicated. On old computer primitive compilers just pushed arguments one by one on the stack (in

[issue45325] Allow "p" in Py_BuildValue

Pablo Galindo Salgado added the comment: The va_start macro just sets up a pointer to the first function parameter, e.g.:- void func (int a, ...) { // va_start char *p = (char *) + sizeof a; } which makes p point to the second parameter. The va_arg macro does this:- void func

[issue45325] Allow "p" in Py_BuildValue

STINNER Victor added the comment: > but there is a catch -- the arguments should be a C int. If it is not, you > can break a stack. I never understood how "..." arguments work under the hood. What happens if you pass a double, it is stored as a double on the C stack, and then

[issue45325] Allow "p" in Py_BuildValue

Pablo Galindo Salgado added the comment: > There was no much need of this feature. In rare cases when we needed to build > a bool in Py_BuildValue (I have found only 2 cases in the stdlib, and one of > them is added by me) we use the following idiom: Is true that this is not very common,

[issue45325] Allow "p" in Py_BuildValue

Pablo Galindo Salgado added the comment: > Either create a new draft PR, or put it in the same PR 28634. I prefer to reach an agreement first here before I add more things to the PR -- ___ Python tracker

[issue45325] Allow "p" in Py_BuildValue

STINNER Victor added the comment: I would be interested to see how the "p" format could be used in existing code. I found a few places would benefit of it. Either create a new draft PR, or put it in the same PR 28634. Modules/_itertoolsmodule.c: return Py_BuildValue("O(N)(OO)",

[issue45325] Allow "p" in Py_BuildValue

Serhiy Storchaka added the comment: There was no much need of this feature. In rare cases when we needed to build a bool in Py_BuildValue (I have found only 2 cases in the stdlib, and one of them is added by me) we use the following idiom: Py_BuildValue("...O...", ..., expr ? Py_True :

[issue45325] Allow "p" in Py_BuildValue

Change by Pablo Galindo Salgado : -- keywords: +patch pull_requests: +27003 stage: -> patch review pull_request: https://github.com/python/cpython/pull/28634 ___ Python tracker

[issue45325] Allow "p" in Py_BuildValue

New submission from Pablo Galindo Salgado : We should allow Py_BuildValue to take a "p" argument that takes a C int and produces a Python Bool so it would be symmetric with the p format code for PyArg_Parse that takes a Python object and returns a C int containing PyObject_IsTrue(obj).