Merge branch 'master' into sh_merge_master

rwgk · rwgk · commit ce62ec56d7d4 · 2021-09-12T19:55:42.000-07:00
diff --git a/.clang-tidy b/.clang-tidy
@@ -31,9 +31,10 @@ modernize-use-override,
 modernize-use-using,
 *performance*,
 readability-avoid-const-params-in-decls,
+readability-const-return-type,
 readability-container-size-empty,
-readability-else-after-return,
 readability-delete-null-pointer,
+readability-else-after-return,
 readability-implicit-bool-conversion,
 readability-make-member-function-const,
 readability-misplaced-array-index,
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
@@ -399,6 +399,22 @@ template <typename StringType, bool IsView = false> struct string_caster {
 #endif
         }
 
+#if PY_VERSION_HEX >= 0x03030000
+        // On Python >= 3.3, for UTF-8 we avoid the need for a temporary `bytes`
+        // object by using `PyUnicode_AsUTF8AndSize`.
+        if (PYBIND11_SILENCE_MSVC_C4127(UTF_N == 8)) {
+            Py_ssize_t size = -1;
+            const auto *buffer
+                = reinterpret_cast<const CharT *>(PyUnicode_AsUTF8AndSize(load_src.ptr(), &size));
+            if (!buffer) {
+                PyErr_Clear();
+                return false;
+            }
+            value = StringType(buffer, static_cast<size_t>(size));
+            return true;
+        }
+#endif
+
         auto utfNbytes = reinterpret_steal<object>(PyUnicode_AsEncodedString(
             load_src.ptr(), UTF_N == 8 ? "utf-8" : UTF_N == 16 ? "utf-16" : "utf-32", nullptr));
         if (!utfNbytes) { PyErr_Clear(); return false; }
@@ -1205,13 +1221,14 @@ class argument_loader {
     }
 
     template <typename Return, typename Guard, typename Func>
+    // NOLINTNEXTLINE(readability-const-return-type)
     enable_if_t<!std::is_void<Return>::value, Return> call(Func &&f) && {
-        return std::move(*this).template call_impl<Return>(std::forward<Func>(f), indices{}, Guard{});
+        return std::move(*this).template call_impl<remove_cv_t<Return>>(std::forward<Func>(f), indices{}, Guard{});
     }
 
     template <typename Return, typename Guard, typename Func>
     enable_if_t<std::is_void<Return>::value, void_type> call(Func &&f) && {
-        std::move(*this).template call_impl<Return>(std::forward<Func>(f), indices{}, Guard{});
+        std::move(*this).template call_impl<remove_cv_t<Return>>(std::forward<Func>(f), indices{}, Guard{});
         return void_type();
     }
 
diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h
@@ -11,11 +11,11 @@
 
 #define PYBIND11_VERSION_MAJOR 2
 #define PYBIND11_VERSION_MINOR 8
-#define PYBIND11_VERSION_PATCH 0.dev1
+#define PYBIND11_VERSION_PATCH 0.dev2
 
 // Similar to Python's convention: https://docs.python.org/3/c-api/apiabiversion.html
 // Additional convention: 0xD = dev
-#define PYBIND11_VERSION_HEX 0x020800D1
+#define PYBIND11_VERSION_HEX 0x020800D2
 
 #define PYBIND11_NAMESPACE_BEGIN(name) namespace name {
 #define PYBIND11_NAMESPACE_END(name) }
diff --git a/include/pybind11/detail/descr.h b/include/pybind11/detail/descr.h
@@ -77,7 +77,8 @@ constexpr enable_if_t<B, T1> _(const T1 &d, const T2 &) { return d; }
 template <bool B, typename T1, typename T2>
 constexpr enable_if_t<!B, T2> _(const T1 &, const T2 &d) { return d; }
 
-template <size_t Size> auto constexpr _() -> decltype(int_to_str<Size / 10, Size % 10>::digits) {
+template <size_t Size>
+auto constexpr _() -> remove_cv_t<decltype(int_to_str<Size / 10, Size % 10>::digits)> {
     return int_to_str<Size / 10, Size % 10>::digits;
 }
 
diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h
@@ -107,7 +107,7 @@ struct internals {
     std::unordered_map<const PyObject *, std::vector<PyObject *>> patients;
     std::forward_list<ExceptionTranslator> registered_exception_translators;
     std::unordered_map<std::string, void *> shared_data; // Custom data to be shared across extensions
-    std::vector<PyObject *> loader_patient_stack; // Used by `loader_life_support`
+    std::vector<PyObject *> unused_loader_patient_stack_remove_at_v5;
     std::forward_list<std::string> static_strings; // Stores the std::strings backing detail::c_str()
     PyTypeObject *static_property_type;
     PyTypeObject *default_metaclass;
@@ -305,12 +305,12 @@ PYBIND11_NOINLINE internals &get_internals() {
         #if PY_VERSION_HEX >= 0x03070000
             internals_ptr->tstate = PyThread_tss_alloc();
             if (!internals_ptr->tstate || (PyThread_tss_create(internals_ptr->tstate) != 0))
-                pybind11_fail("get_internals: could not successfully initialize the TSS key!");
+                pybind11_fail("get_internals: could not successfully initialize the tstate TSS key!");
             PyThread_tss_set(internals_ptr->tstate, tstate);
         #else
             internals_ptr->tstate = PyThread_create_key();
             if (internals_ptr->tstate == -1)
-                pybind11_fail("get_internals: could not successfully initialize the TLS key!");
+                pybind11_fail("get_internals: could not successfully initialize the tstate TLS key!");
             PyThread_set_key_value(internals_ptr->tstate, tstate);
         #endif
         internals_ptr->istate = tstate->interp;
diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h
@@ -31,47 +31,54 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 /// A life support system for temporary objects created by `type_caster::load()`.
 /// Adding a patient will keep it alive up until the enclosing function returns.
 class loader_life_support {
+private:
+    loader_life_support* parent = nullptr;
+    std::unordered_set<PyObject *> keep_alive;
+
+    static loader_life_support** get_stack_pp() {
+#if defined(WITH_THREAD)
+        thread_local static loader_life_support* per_thread_stack = nullptr;
+        return &per_thread_stack;
+#else
+        static loader_life_support* global_stack = nullptr;
+        return &global_stack;
+#endif
+    }
+
 public:
     /// A new patient frame is created when a function is entered
     loader_life_support() {
-        get_internals().loader_patient_stack.push_back(nullptr);
+        loader_life_support** stack = get_stack_pp();
+        parent = *stack;
+        *stack = this;
     }
 
     /// ... and destroyed after it returns
     ~loader_life_support() {
-        auto &stack = get_internals().loader_patient_stack;
-        if (stack.empty())
+        loader_life_support** stack = get_stack_pp();
+        if (*stack != this)
             pybind11_fail("loader_life_support: internal error");
-
-        auto ptr = stack.back();
-        stack.pop_back();
-        Py_CLEAR(ptr);
-
-        // A heuristic to reduce the stack's capacity (e.g. after long recursive calls)
-        if (stack.capacity() > 16 && !stack.empty() && stack.capacity() / stack.size() > 2)
-            stack.shrink_to_fit();
+        *stack = parent;
+        for (auto* item : keep_alive)
+            Py_DECREF(item);
     }
 
     /// This can only be used inside a pybind11-bound function, either by `argument_loader`
     /// at argument preparation time or by `py::cast()` at execution time.
     PYBIND11_NOINLINE static void add_patient(handle h) {
-        auto &stack = get_internals().loader_patient_stack;
-        if (stack.empty())
+        loader_life_support* frame =  *get_stack_pp();
+        if (!frame) {
+            // NOTE: It would be nice to include the stack frames here, as this indicates
+            // use of pybind11::cast<> outside the normal call framework, finding such
+            // a location is challenging. Developers could consider printing out
+            // stack frame addresses here using something like __builtin_frame_address(0)
             throw cast_error("When called outside a bound function, py::cast() cannot "
                              "do Python -> C++ conversions which require the creation "
                              "of temporary values");
-
-        auto &list_ptr = stack.back();
-        if (list_ptr == nullptr) {
-            list_ptr = PyList_New(1);
-            if (!list_ptr)
-                pybind11_fail("loader_life_support: error allocating list");
-            PyList_SET_ITEM(list_ptr, 0, h.inc_ref().ptr());
-        } else {
-            auto result = PyList_Append(list_ptr, h.ptr());
-            if (result == -1)
-                pybind11_fail("loader_life_support: error adding patient");
         }
+
+        if (frame->keep_alive.insert(h.ptr()).second)
+            Py_INCREF(h.ptr());
     }
 };
 
diff --git a/include/pybind11/eval.h b/include/pybind11/eval.h
@@ -136,6 +136,15 @@ object eval_file(str fname, object global = globals(), object local = object())
         pybind11_fail("File \"" + fname_str + "\" could not be opened!");
     }
 
+    // In Python2, this should be encoded by getfilesystemencoding.
+    // We don't boher setting it since Python2 is past EOL anyway.
+    // See PR#3233
+#if PY_VERSION_HEX >= 0x03000000
+    if (!global.contains("__file__")) {
+        global["__file__"] = std::move(fname);
+    }
+#endif
+
 #if PY_VERSION_HEX < 0x03000000 && defined(PYPY_VERSION)
     PyObject *result = PyRun_File(f, fname_str.c_str(), start, global.ptr(),
                                   local.ptr());
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
@@ -2101,6 +2101,7 @@ iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
                     throw stop_iteration();
                 }
                 return *s.it;
+            // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
             }, std::forward<Extra>(extra)..., Policy);
     }
 
@@ -2116,13 +2117,13 @@ template <return_value_policy Policy = return_value_policy::reference_internal,
           typename KeyType = decltype((*std::declval<Iterator>()).first),
 #endif
           typename... Extra>
-iterator make_key_iterator(Iterator first, Sentinel last, Extra &&... extra) {
+iterator make_key_iterator(Iterator first, Sentinel last, Extra &&...extra) {
     using state = detail::iterator_state<Iterator, Sentinel, true, Policy>;
 
     if (!detail::get_type_info(typeid(state), false)) {
         class_<state>(handle(), "iterator", pybind11::module_local())
             .def("__iter__", [](state &s) -> state& { return s; })
-            .def("__next__", [](state &s) -> KeyType {
+            .def("__next__", [](state &s) -> detail::remove_cv_t<KeyType> {
                 if (!s.first_or_done)
                     ++s.it;
                 else
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
@@ -733,7 +733,9 @@ class generic_iterator : public Policy {
     generic_iterator() = default;
     generic_iterator(handle seq, ssize_t index) : Policy(seq, index) { }
 
+    // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
     reference operator*() const { return Policy::dereference(); }
+    // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
     reference operator[](difference_type n) const { return *(*this + n); }
     pointer operator->() const { return **this; }
 
@@ -773,11 +775,12 @@ class sequence_fast_readonly {
 protected:
     using iterator_category = std::random_access_iterator_tag;
     using value_type = handle;
-    using reference = const handle;
+    using reference = const handle; // PR #3263
     using pointer = arrow_proxy<const handle>;
 
     sequence_fast_readonly(handle obj, ssize_t n) : ptr(PySequence_Fast_ITEMS(obj.ptr()) + n) { }
 
+    // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
     reference dereference() const { return *ptr; }
     void increment() { ++ptr; }
     void decrement() { --ptr; }
@@ -816,12 +819,13 @@ class dict_readonly {
 protected:
     using iterator_category = std::forward_iterator_tag;
     using value_type = std::pair<handle, handle>;
-    using reference = const value_type;
+    using reference = const value_type; // PR #3263
     using pointer = arrow_proxy<const value_type>;
 
     dict_readonly() = default;
     dict_readonly(handle obj, ssize_t pos) : obj(obj), pos(pos) { increment(); }
 
+    // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
     reference dereference() const { return {key, value}; }
     void increment() {
         if (PyDict_Next(obj.ptr(), &pos, &key, &value) == 0) {
@@ -966,7 +970,7 @@ class iterator : public object {
     using iterator_category = std::input_iterator_tag;
     using difference_type = ssize_t;
     using value_type = handle;
-    using reference = const handle;
+    using reference = const handle; // PR #3263
     using pointer = const handle *;
 
     PYBIND11_OBJECT_DEFAULT(iterator, object, PyIter_Check)
@@ -982,6 +986,7 @@ class iterator : public object {
         return rv;
     }
 
+    // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
     reference operator*() const {
         if (m_ptr && !value.ptr()) {
             auto& self = const_cast<iterator &>(*this);
@@ -1414,14 +1419,19 @@ class capsule : public object {
     T* get_pointer() const {
         auto name = this->name();
         T *result = static_cast<T *>(PyCapsule_GetPointer(m_ptr, name));
-        if (!result) pybind11_fail("Unable to extract capsule contents!");
+        if (!result) {
+            PyErr_Clear();
+            pybind11_fail("Unable to extract capsule contents!");
+        }
         return result;
     }
 
     /// Replaces a capsule's pointer *without* calling the destructor on the existing one.
     void set_pointer(const void *value) {
-        if (PyCapsule_SetPointer(m_ptr, const_cast<void *>(value)) != 0)
+        if (PyCapsule_SetPointer(m_ptr, const_cast<void *>(value)) != 0) {
+            PyErr_Clear();
             pybind11_fail("Could not set capsule pointer");
+        }
     }
 
     const char *name() const { return PyCapsule_GetName(m_ptr); }
diff --git a/pybind11/_version.py b/pybind11/_version.py
@@ -8,5 +8,5 @@ def _to_int(s):
         return s
 
 
-__version__ = "2.8.0.dev1"
+__version__ = "2.8.0.dev2"
 version_info = tuple(_to_int(s) for s in __version__.split("."))
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -143,6 +143,7 @@ set(PYBIND11_TEST_FILES
     test_stl.cpp
     test_stl_binders.cpp
     test_tagbased_polymorphic.cpp
+    test_thread.cpp
     test_union.cpp
     test_virtual_functions.cpp)
 
diff --git a/tests/test_cmake_build/test.py b/tests/test_cmake_build/test.py
@@ -3,5 +3,8 @@
 
 import test_cmake_build
 
+if str is not bytes:  # If not Python2
+    assert isinstance(__file__, str)  # Test this is properly set
+
 assert test_cmake_build.add(1, 2) == 3
 print("{} imports, runs, and adds: 1 + 2 = 3".format(sys.argv[1]))
diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp
@@ -178,6 +178,7 @@ TEST_SUBMODULE(eigen, m) {
         ReturnTester() { print_created(this); }
         ~ReturnTester() { print_destroyed(this); }
         static Eigen::MatrixXd create() { return Eigen::MatrixXd::Ones(10, 10); }
+        // NOLINTNEXTLINE(readability-const-return-type)
         static const Eigen::MatrixXd createConst() { return Eigen::MatrixXd::Ones(10, 10); }
         Eigen::MatrixXd &get() { return mat; }
         Eigen::MatrixXd *getPtr() { return &mat; }
@@ -244,6 +245,9 @@ TEST_SUBMODULE(eigen, m) {
 
     // test_fixed, and various other tests
     m.def("fixed_r", [mat]() -> FixedMatrixR { return FixedMatrixR(mat); });
+    // Our Eigen does a hack which respects constness through the numpy writeable flag.
+    // Therefore, the const return actually affects this type despite being an rvalue.
+    // NOLINTNEXTLINE(readability-const-return-type)
     m.def("fixed_r_const", [mat]() -> const FixedMatrixR { return FixedMatrixR(mat); });
     m.def("fixed_c", [mat]() -> FixedMatrixC { return FixedMatrixC(mat); });
     m.def("fixed_copy_r", [](const FixedMatrixR &m) -> FixedMatrixR { return m; });
diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
@@ -462,6 +462,49 @@ TEST_SUBMODULE(pytypes, m) {
     m.def("weakref_from_object_and_function",
           [](py::object o, py::function f) { return py::weakref(std::move(o), std::move(f)); });
 
+// See PR #3263 for background (https://github.com/pybind/pybind11/pull/3263):
+// pytypes.h could be changed to enforce the "most correct" user code below, by removing
+// `const` from iterator `reference` using type aliases, but that will break existing
+// user code.
+#if (defined(__APPLE__) && defined(__clang__)) || defined(PYPY_VERSION)
+// This is "most correct" and enforced on these platforms.
+#    define PYBIND11_AUTO_IT auto it
+#else
+// This works on many platforms and is (unfortunately) reflective of existing user code.
+// NOLINTNEXTLINE(bugprone-macro-parentheses)
+#    define PYBIND11_AUTO_IT auto &it
+#endif
+
+    m.def("tuple_iterator", []() {
+        auto tup = py::make_tuple(5, 7);
+        int tup_sum = 0;
+        for (PYBIND11_AUTO_IT : tup) {
+            tup_sum += it.cast<int>();
+        }
+        return tup_sum;
+    });
+
+    m.def("dict_iterator", []() {
+        py::dict dct;
+        dct[py::int_(3)] = 5;
+        dct[py::int_(7)] = 11;
+        int kv_sum = 0;
+        for (PYBIND11_AUTO_IT : dct) {
+            kv_sum += it.first.cast<int>() * 100 + it.second.cast<int>();
+        }
+        return kv_sum;
+    });
+
+    m.def("passed_iterator", [](const py::iterator &py_it) {
+        int elem_sum = 0;
+        for (PYBIND11_AUTO_IT : py_it) {
+            elem_sum += it.cast<int>();
+        }
+        return elem_sum;
+    });
+
+#undef PYBIND11_AUTO_IT
+
     // Tests below this line are for pybind11 IMPLEMENTATION DETAILS:
 
     m.def("sequence_item_get_ssize_t", [](const py::object &o) {
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
diff --git a/tests/test_thread.cpp b/tests/test_thread.cpp
diff --git a/tests/test_thread.py b/tests/test_thread.py

Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,8 @@ constexpr enable_if_t<B, T1> _(const T1 &d, const T2 &) { return d; }`
`77`	`77`	`template <bool B, typename T1, typename T2>`
`78`	`78`	`constexpr enable_if_t<!B, T2> _(const T1 &, const T2 &d) { return d; }`
`79`	`79`
`80`		`-template <size_t Size> auto constexpr _() -> decltype(int_to_str<Size / 10, Size % 10>::digits) {`
	`80`	`+template <size_t Size>`
	`81`	`+auto constexpr _() -> remove_cv_t<decltype(int_to_str<Size / 10, Size % 10>::digits)> {`
`81`	`82`	`return int_to_str<Size / 10, Size % 10>::digits;`
`82`	`83`	`}`
`83`	`84`