Repository navigation
Document the preferred style for API functions with three, four or five-way returns #1121
Description
Activity
While it's different, here is an example of API using different negative values for result.
The private _Py_DecodeLocaleEx() function returns 0 on success, or a negative number on error:
- -1 on memory allocation failure
- -2 on decoding error
- -3 if the error handler is not supported
Since
_Py_DecodeLocaleEx()is a private function, it can do whatever it wants 🙂Having said that, it is possible that an API function would want to distinguish between error types.
I don't think that is worth changing the rules for some hypothetical use case, though.Are there any other examples?
AFAICS, the proposed guideline is:
- For functions returning failure or success, the return value is respectively -1 and 0.
- For functions returning failure, lesser result (success), greater result (success), the return values are respectively -1, 0, and 1.
I'm +1 for those two guidelines.
Also, to avoid any potential confusion: when we're talking about returning, I interpret that as the C return statement; I do not interpret that as an "output parameter" (for example a pointer to an
int). Example:// This function _returns_ -1 in case of error, else 0. int func(void) { /* do stuff */ if (error) { raise_exception(); return -1; } return 0; }
For functions that return (the C return statement) two different kinds of values, yes.
But this issue is about functions that (C) return three or more different values, likePyDict_GetItem[Suffix].I think we are all in agreement that in the case of failure, -1 should be returned.
Opinions differ on how to handle success variants.For functions returning failure or success, the return value is respectively -1 and 0.
In PR #106005, adding PyDict_GetItemRef() function, @markshannon asks to return 1 if the key is present and 0 if the key is missing. Is it a bad practice? Is it better to return 0 in both cases (that was my first implementation)?
For functions returning failure or success, the return value is respectively -1 and 0.
In PR #106005, adding PyDict_GetItemRef() function, @markshannon asks to return 1 if the key is present and 0 if the key is missing. Is it a bad practice? Is it better to return 0 in both cases (that was my first implementation)?
Try reading the next line 😉
For functions returning failure, lesser result (success), greater result (success), the return values are respectively -1, 0, and 1.
- linked a pull request that will close this issueAdd guidelines for C API return values #1123
on Jun 23, 2023 I made a PR: #1123
For functions that return (the C return statement) two different kinds of values, yes.
[...]
I think we are all in agreement that in the case of failure, -1 should be returned.Sure; but I prefer to document it so there's even less chance of friction :)
Reacted by Mark ShannonOne other issue is whether the output parameter should be touched if there is no meaningful value.
Take
PyDict_GetItemSuffixToBeDecided(PyDictObject *op, PyObject *key, PyObject **pvalue).
Should we touch*pvalueif returning -1 or 0.
In other words, inPyObject *value; int err = PyDict_GetItemSuffixToBeDecided(op, key, &value);
Should
valuebe undefined iferr <= 0, or should it be set toNULL?It is more efficient to not touch it, since the return value specifies that it has no meaning.
But, do we trust all C extension authors not to read the value, which may point to a real object if they are unlucky? Setting*pvalue = NULLis safer.Reacted by Erlend E. AaslandOne other issue is whether the output parameter should be touched if there is no meaningful value.
Yeah... let's establish guidelines for the return values first; then we can follow up with guidelines for output parameters.
Should value be undefined if err <= 0, or should it be set to NULL?
In my PyDict_GetItemRef() PR, I chose to always initialized
*pvalue, especially in the error case (return -1). IMO there is a minor performance overhead, but it closes a whole class of issue: undefined behavior if the function returns -1. Some C compilers try to evaluate all code paths and say that a&valueargument "can be undefined" when the code is correct. A common workaround makes me sad: initialize the value in the calling site:PyObject *value = NULL;:-(For the "return 0" case (missing key), I also prefer to set
*pvalueto NULL for the same reason, but also because people upgrading their code from PyDict_GetItem() and PyDict_GetItemWithError() may want to check ifvalueis NULL or not, rather than checking the return value:PyObject *value; // OMG! undefined value if (PyDict_GetItemRef(dict, key, &value) < 0) ... error ... else if (value == NULL) ... missing key ... else ... key is present ...Here the function result is not used for the second code path:
... missing key ....I'm not comfortable to document that
*pvalueis intialized for the error case (return -1). But I wrote an unit test for that.I think we are all in agreement that in the case of failure, -1 should be returned.
IMO, a negative value should be returned. Functions should return -1 unless they have a reason to use something else (like distinguish between error types), but callers should check with
< 0.Also, IMO we should say that a negative value should be returned iff an exception is raised. I.e., make it very clear what counts as “failure”.
Reacted by Erlend E. AaslandIMO, a negative value should be returned. Functions should return -1 unless they have a reason to use something else (like distinguish between error types), but callers should check with
< 0.Also, IMO we should say that a negative value should be returned iff an exception is raised. I.e., make it very clear what counts as “failure”.
Adding something like this would indeed make the guidelines clearer. I'll create PR with an amendment shortly.
- linked a pull request that will close this issueImprove C API guidelines for return values #1129
on Jun 26, 2023 @erlend-aasland Should this issue be closed or is there remaining work?
We will leave this to the C API Workgroup. If needed, they can create a new issue or reopen this.
Most C API functions either succeed or fail. Those functions return
NULL/not-NULLor 0/-1. No problem there.However there are functions that can have three or more return kinds.
For example,
found/not-found/error,yield/return/raise, or evenless-than/equal/greater-than/unordered/errorin the case of comparisons.For those functions we want to return the kind as an
int, setting anyPyObject *results through an out pointer.The devguide should document the preferred way to do this, to avoid endless debate for each new C API function.
See python/cpython#105201 (comment) for my preferred approach for three-way return values.
Four and five way return values other than for comparisons are sufficiently uncommon that they can be dealt with on an individual basis.
For comparisons, a slightly strange enum based on powers-of-two is to be used for efficient selection of the desired result by masking.