gh-112068: C API: Add support of nullable arguments in PyArg_Parse (suffix) - #121303
Conversation
Sorry, something went wrong.
picnixz
left a comment
There was a problem hiding this comment.
I'm really sorry that you needed to implement the suffix just for me to actually understand that it's probably harder to maintain and to extend compared to the prefix case.
The only reason why I would advocate for the suffix is more because of its use in other languages but maybe I shouldn't have kept that argument at the top of my list and should have rather go for a more simple implementation (for instance in TypeScript, you write function sumTwoMaybeThree(x: number, y: number, z?: number): number and in other languages as I said you have the ?. operator.
In addition, since (...)? is not yet implemented, I assume that it would be even more complex since you need to open / close contexts and after all that, you still need to parse that ?. However ?(...) would be easier to implement I think since you could add the flag beforehand and process (...) normally.
By the way, is there a reason why we have a huge switch instead of some separate functions like in _testcapi/getargs.c? is it only for efficiency purposes and to reduce function calls (though those could be probably inlined)?
Sorry, something went wrong.
In case of multicharacter format unit you need to reorganize the code and add new code at all right places. This is actually simpler than I thought, because the same macros can be used in all cases (except (...)?), but this is still more complex than the prefix implementation, and more error-prone. For (...)?, when it encounters an opening (, it needs to search the closing ) and look if there is a following ?. Note that parentheses can be nested, so this is not so trivial. If the closing )? is found and the argument is None, it should scan the format string again and skip PyArg_Parse() arguments corresponded to nested format units. The cost of additional scanning is played every time, even if the new feature is not used. Well, this can be optimized for common cases, but at the cost of more complicated code. |
Sorry, something went wrong.
|
I managed to implement (...)? without affecting performance. But the code is still more complex. I need to check it many more times. |
Sorry, something went wrong.
|
Added the documentation. It is now ready for review. |
Sorry, something went wrong.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
…PyArg_Parse (pythonGH-121303)" This reverts commit f5f1ac8.
…PyArg_Parse (pythonGH-121303)" This reverts commit f5f1ac8.
…PyArg_Parse (pythonGH-121303)" This reverts commit f5f1ac8.
…nts in PyArg_Parse (pythonGH-121303)" (pythonGH-136991) (cherry picked from commit 3a89dfe) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…nts in PyArg_Parse (pythonGH-121303)" (pythonGH-136991) (python#137006)
This is a variant of #121187, but with suffix instead of prefix. i? instead of ?i.