From 907a57fd66136ac038bc1238d79876e7eea375fd Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 31 Jul 2020 06:01:37 -0700
Subject: [PATCH] Fixing pybind11::bytes() ambiguous overload issue and adding
 missing bytes type to test_constructors(). (#2340)

---
 include/pybind11/pytypes.h |  8 ++++----
 tests/test_pytypes.cpp     |  3 +++
 tests/test_pytypes.py      | 30 ++++++++++++++++++------------
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 9000668b919..3244ed8c525 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -782,7 +782,9 @@ PYBIND11_NAMESPACE_END(detail)
         Name(handle h, stolen_t) : Parent(h, stolen_t{}) { } \
         PYBIND11_DEPRECATED("Use py::isinstance<py::python_type>(obj) instead") \
         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) \
     PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
@@ -792,9 +794,7 @@ PYBIND11_NAMESPACE_END(detail)
     { if (!m_ptr) throw error_already_set(); } \
     Name(object &&o) \
     : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
-    { if (!m_ptr) throw error_already_set(); } \
-    template <typename Policy_> \
-    Name(const ::pybind11::detail::accessor<Policy_> &a) : Name(object(a)) { }
+    { if (!m_ptr) throw error_already_set(); }
 
 #define PYBIND11_OBJECT(Name, Parent, CheckFun) \
     PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
index 9dae6e7d620..0f8d56410f0 100644
--- a/tests/test_pytypes.cpp
+++ b/tests/test_pytypes.cpp
@@ -197,6 +197,7 @@ TEST_SUBMODULE(pytypes, m) {
     // test_constructors
     m.def("default_constructors", []() {
         return py::dict(
+            "bytes"_a=py::bytes(),
             "str"_a=py::str(),
             "bool"_a=py::bool_(),
             "int"_a=py::int_(),
@@ -210,6 +211,7 @@ TEST_SUBMODULE(pytypes, m) {
 
     m.def("converting_constructors", [](py::dict d) {
         return py::dict(
+            "bytes"_a=py::bytes(d["bytes"]),
             "str"_a=py::str(d["str"]),
             "bool"_a=py::bool_(d["bool"]),
             "int"_a=py::int_(d["int"]),
@@ -225,6 +227,7 @@ TEST_SUBMODULE(pytypes, m) {
     m.def("cast_functions", [](py::dict d) {
         // When converting between Python types, obj.cast<T>() should be the same as T(obj)
         return py::dict(
+            "bytes"_a=d["bytes"].cast<py::bytes>(),
             "str"_a=d["str"].cast<py::str>(),
             "bool"_a=d["bool"].cast<py::bool_>(),
             "int"_a=d["int"].cast<py::int_>(),
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
index e5d8355d7d1..5aa07bc136d 100644
--- a/tests/test_pytypes.py
+++ b/tests/test_pytypes.py
@@ -188,23 +188,29 @@ def func(self, x, *args):
 
 def test_constructors():
     """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}
+    if str is bytes:  # Python 2.
+        # Note that bytes.__name__ == 'str' in Python 2.
+        # pybind11::str is unicode even under Python 2.
+        expected["bytes"] = bytes()
+        expected["str"] = u""  # flake8 complains about unicode().
     assert m.default_constructors() == expected
 
     data = {
-        str: 42,
-        bool: "Not empty",
-        int: "42",
-        float: "+1e3",
-        tuple: range(3),
-        list: range(3),
-        dict: [("two", 2), ("one", 1), ("three", 3)],
-        set: [4, 4, 5, 6, 6, 6],
-        memoryview: b'abc'
+        "bytes": b'41',  # Currently no supported or working conversions.
+        "str": 42,
+        "bool": "Not empty",
+        "int": "42",
+        "float": "+1e3",
+        "tuple": range(3),
+        "list": range(3),
+        "dict": [("two", 2), ("one", 1), ("three", 3)],
+        "set": [4, 4, 5, 6, 6, 6],
+        "memoryview": b'abc'
     }
-    inputs = {k.__name__: v for k, v in data.items()}
-    expected = {k.__name__: k(v) for k, v in data.items()}
+    inputs = {k: v for k, v in data.items()}
+    expected = {k: eval(k)(v) for k, v in data.items()}
 
     assert m.converting_constructors(inputs) == expected
     assert m.cast_functions(inputs) == expected