Fixing pybind11::bytes() ambiguous conversion issue.

Adding missing `bytes` type to `test_constructors()`, to exercise the code change.

The changes in the PR were cherry-picked from PR #2409 (with a very minor
modification in test_pytypes.py related to flake8). Via PR #2409, these
changes were extensively tested in the Google environment, as summarized here:
https://docs.google.com/document/d/1TPL-J__mph_yHa1quDvsO12E_F5OZnvBaZlW9IIrz8M/
The changes in this PR did not cause an issues at all.

Note that `test_constructors()` before this PR passes for Python 2 only
because `pybind11::str` can hold `PyUnicodeObject` or `PyBytesObject`. As a
side-effect of this PR, `test_constructors()` no longer relies on this
permissive `pybind11::str` behavior. However, the permissive behavior is still
exercised/exposed via the existing `test_pybind11_str_raw_str()`.

The test code change is designed to enable easy removal later, when Python 2
support is dropped.

For completeness: confusingly, the non-test code changes travelled through PR

Example `ambiguous conversion` error fixed by this PR:
```
pybind11/tests/test_pytypes.cpp:214:23: error: ambiguous conversion for functional-style cast from 'pybind11::detail::item_accessor' (aka 'accessor<accessor_policies::generic_item>') to 'py::bytes'
            "bytes"_a=py::bytes(d["bytes"]),
                      ^~~~~~~~~~~~~~~~~~~~
pybind11/include/pybind11/detail/../pytypes.h:957:21: note: candidate constructor
    PYBIND11_OBJECT(bytes, object, PYBIND11_BYTES_CHECK)
                    ^
pybind11/include/pybind11/detail/../pytypes.h:957:21: note: candidate constructor
pybind11/include/pybind11/detail/../pytypes.h:987:15: note: candidate constructor
inline bytes::bytes(const pybind11::str &s) {
              ^
1 error generated.
```
This commit is contained in:
Ralf W. Grosse-Kunstleve 2020-08-27 17:49:52 -07:00 committed by Ralf W. Grosse-Kunstleve
parent 6a192781fc
commit 3c061f2168
3 changed files with 19 additions and 5 deletions

View File

@ -796,7 +796,9 @@ PYBIND11_NAMESPACE_END(detail)
Name(handle h, stolen_t) : Parent(h, stolen_t{}) { } \ Name(handle h, stolen_t) : Parent(h, stolen_t{}) { } \
PYBIND11_DEPRECATED("Use py::isinstance<py::python_type>(obj) instead") \ PYBIND11_DEPRECATED("Use py::isinstance<py::python_type>(obj) instead") \
bool check() const { return m_ptr != nullptr && (bool) CheckFun(m_ptr); } \ bool check() const { return m_ptr != nullptr && (bool) CheckFun(m_ptr); } \
static bool check_(handle h) { return h.ptr() != nullptr && CheckFun(h.ptr()); } static bool check_(handle h) { return h.ptr() != nullptr && CheckFun(h.ptr()); } \
template <typename Policy_> \
Name(const ::pybind11::detail::accessor<Policy_> &a) : Name(object(a)) { }
#define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ConvertFun) \ #define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ConvertFun) \
PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
@ -806,9 +808,7 @@ PYBIND11_NAMESPACE_END(detail)
{ if (!m_ptr) throw error_already_set(); } \ { if (!m_ptr) throw error_already_set(); } \
Name(object &&o) \ Name(object &&o) \
: Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \ : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
{ if (!m_ptr) throw error_already_set(); } \ { if (!m_ptr) throw error_already_set(); }
template <typename Policy_> \
Name(const ::pybind11::detail::accessor<Policy_> &a) : Name(object(a)) { }
#define PYBIND11_OBJECT(Name, Parent, CheckFun) \ #define PYBIND11_OBJECT(Name, Parent, CheckFun) \
PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \

View File

@ -197,6 +197,7 @@ TEST_SUBMODULE(pytypes, m) {
// test_constructors // test_constructors
m.def("default_constructors", []() { m.def("default_constructors", []() {
return py::dict( return py::dict(
"bytes"_a=py::bytes(),
"str"_a=py::str(), "str"_a=py::str(),
"bool"_a=py::bool_(), "bool"_a=py::bool_(),
"int"_a=py::int_(), "int"_a=py::int_(),
@ -210,6 +211,7 @@ TEST_SUBMODULE(pytypes, m) {
m.def("converting_constructors", [](py::dict d) { m.def("converting_constructors", [](py::dict d) {
return py::dict( return py::dict(
"bytes"_a=py::bytes(d["bytes"]),
"str"_a=py::str(d["str"]), "str"_a=py::str(d["str"]),
"bool"_a=py::bool_(d["bool"]), "bool"_a=py::bool_(d["bool"]),
"int"_a=py::int_(d["int"]), "int"_a=py::int_(d["int"]),
@ -225,6 +227,7 @@ TEST_SUBMODULE(pytypes, m) {
m.def("cast_functions", [](py::dict d) { m.def("cast_functions", [](py::dict d) {
// When converting between Python types, obj.cast<T>() should be the same as T(obj) // When converting between Python types, obj.cast<T>() should be the same as T(obj)
return py::dict( return py::dict(
"bytes"_a=d["bytes"].cast<py::bytes>(),
"str"_a=d["str"].cast<py::str>(), "str"_a=d["str"].cast<py::str>(),
"bool"_a=d["bool"].cast<py::bool_>(), "bool"_a=d["bool"].cast<py::bool_>(),
"int"_a=d["int"].cast<py::int_>(), "int"_a=d["int"].cast<py::int_>(),

View File

@ -190,11 +190,17 @@ def test_accessors():
def test_constructors(): def test_constructors():
"""C++ default and converting constructors are equivalent to type calls in Python""" """C++ default and converting constructors are equivalent to type calls in Python"""
types = [str, bool, int, float, tuple, list, dict, set] types = [bytes, str, bool, int, float, tuple, list, dict, set]
expected = {t.__name__: t() for t in types} expected = {t.__name__: t() for t in types}
if env.PY2:
# Note that bytes.__name__ == 'str' in Python 2.
# pybind11::str is unicode even under Python 2.
expected["bytes"] = bytes()
expected["str"] = unicode() # noqa: F821
assert m.default_constructors() == expected assert m.default_constructors() == expected
data = { data = {
bytes: b'41', # Currently no supported or working conversions.
str: 42, str: 42,
bool: "Not empty", bool: "Not empty",
int: "42", int: "42",
@ -207,6 +213,11 @@ def test_constructors():
} }
inputs = {k.__name__: v for k, v in data.items()} inputs = {k.__name__: v for k, v in data.items()}
expected = {k.__name__: k(v) for k, v in data.items()} expected = {k.__name__: k(v) for k, v in data.items()}
if env.PY2: # Similar to the above. See comments above.
inputs["bytes"] = b'41'
inputs["str"] = 42
expected["bytes"] = b'41'
expected["str"] = u"42"
assert m.converting_constructors(inputs) == expected assert m.converting_constructors(inputs) == expected
assert m.cast_functions(inputs) == expected assert m.cast_functions(inputs) == expected