pybind / pybind/pybind11

[BUG] Documentation suggests that custom type casters do not need to clear exceptions

Open
#2,732 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
18k
Forks
2.3k
Avg merge
5d 17h
Merged PRs (30d)
10

Description

Issue description

The documentation at https://pybind11.readthedocs.io/en/stable/advanced/cast/custom.html suggests that custom type casters do not need to clear any Python exception that may have been set (the implementation of load basically returning !PyErr_Occurred()). Upon first reading, this seems reasonable as pybind11 may perhaps use that info for better error reporting. However, type casters actually need to clear the exception -- at least to make variant loading work.

I don't know if this should be fixed on the docs side, or on the implementation side.

Reproducible example code

The entire example, except the PYBIND11_MODULE block, is copied verbatim from https://pybind11.readthedocs.io/en/stable/advanced/cast/custom.html.

#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

struct inty { long long_value; };

namespace pybind11 { namespace detail {
    template <> struct type_caster<inty> {
    public:
        /**
         * This macro establishes the name 'inty' in
         * function signatures and declares a local variable
         * 'value' of type inty
         */
        PYBIND11_TYPE_CASTER(inty, _("inty"));

        /**
         * Conversion part 1 (Python->C++): convert a PyObject into a inty
         * instance or return false upon failure. The second argument
         * indicates whether implicit conversions should be applied.
         */
        bool load(handle src, bool) {
            /* Extract PyObject from handle */
            PyObject *source = src.ptr();
            /* Try converting into a Python integer value */
            PyObject *tmp = PyNumber_Long(source);
            if (!tmp) {
                return false;
            }
            /* Now try to convert into a C++ int */
            value.long_value = PyLong_AsLong(tmp);
            Py_DECREF(tmp);
            /* Ensure return code was OK (to avoid out-of-range errors etc) */
            return !(value.long_value == -1 && !PyErr_Occurred());
        }

        /**
         * Conversion part 2 (C++ -> Python): convert an inty instance into
         * a Python object. The second and third arguments are used to
         * indicate the return value policy and parent object (for
         * ``return_value_policy::reference_internal``) and are generally
         * ignored by implicit casters.
         */
        static handle cast(inty src, return_value_policy /* policy */, handle /* parent */) {
            return PyLong_FromLong(src.long_value);
        }
    };
}}

PYBIND11_MODULE(python_example, m) {
    m.def("get_index", [](std::variant<inty, std::string> v) { return v.index(); });
}

Calling get_index("a") throws

ValueError: invalid literal for int() with base 10: 'a'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
SystemError: <built-in method get_index of PyCapsule object at 0x7fda858a08d0> returned a result with an error set

Fixing the implementation of load to always clear set exceptions instead makes get_index("a") correctly return 1.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with advanced/cast/custom.html and the provided PYBIND11_MODULE/get_index reproducer; inspect variant loading and custom type-caster exception handling. Confirm the failure and determine whether the documentation or implementation needs correction; done when the example no longer leaves an error set and the documented guidance matches behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, python
Domain
api, documentation
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.