Document the preferred style for API functions with three, four or five-way returns

[markshannon]

Most C API functions either succeed or fail. Those functions return NULL/not-NULL or 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 even less-than/equal/greater-than/unordered/error in the case of comparisons.

For those functions we want to return the kind as an int, setting any PyObject * 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.

[vstinner]

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

[markshannon]

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?

[erlend-aasland]

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;
}

[markshannon]

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, like PyDict_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.

[vstinner]

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)?

[markshannon]

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.

[erlend-aasland]

I made a PR: #1123

[erlend-aasland]

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 :)

[markshannon]

One 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 *pvalue if returning -1 or 0.
In other words, in

    PyObject *value;
    int err = PyDict_GetItemSuffixToBeDecided(op, key, &value);

Should value be undefined if err <= 0, or should it be set to NULL?

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 = NULL is safer.

[erlend-aasland]

One 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.

[vstinner]

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 &value argument "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 *pvalue to NULL for the same reason, but also because people upgrading their code from PyDict_GetItem() and PyDict_GetItemWithError() may want to check if value is 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 *pvalue is intialized for the error case (return -1). But I wrote an unit test for that.

[encukou]

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”.

[erlend-aasland]

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”.

Adding something like this would indeed make the guidelines clearer. I'll create PR with an amendment shortly.

[willingc]

@erlend-aasland Should this issue be closed or is there remaining work?

[erlend-aasland]

We will leave this to the C API Workgroup. If needed, they can create a new issue or reopen this.