Merge branch 'master' into sh_merge_master

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

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 @@ public:
     }
 
     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();
     }
 

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

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

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;

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;
+        loader_life_support** stack = get_stack_pp();
-        if (stack.empty())
+        if (*stack != this)
             pybind11_fail("loader_life_support: internal error");
-
+        *stack = parent;
-        auto ptr = stack.back();
+        for (auto* item : keep_alive)
-        stack.pop_back();
+            Py_DECREF(item);
-        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();
     }
 
     /// 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;
+        loader_life_support* frame =  *get_stack_pp();
-        if (stack.empty())
+        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());
     }
 };
 

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());

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

include/pybind11/pytypes.h

@@ -733,7 +733,9 @@ public:
     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 @@ public:
     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 @@ public:
         return rv;
     }
 
+    // NOLINTNEXTLINE(readability-const-return-type) // PR #3263
     reference operator*() const {
         if (m_ptr && !value.ptr()) {
             auto& self = const_cast<iterator &>(*this);
@@ -1414,15 +1419,20 @@ public:
     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); }
 };

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("."))

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)
 

tests/test_cmake_build/test.py

@@ -3,5 +3,8 @@ import sys
 
 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]))

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

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

tests/test_pytypes.py

@@ -623,6 +623,12 @@ def test_weakref(create_weakref, create_weakref_with_callback):
     assert callback.called
 
 
+def test_cpp_iterators():
+    assert m.tuple_iterator() == 12
+    assert m.dict_iterator() == 305 + 711
+    assert m.passed_iterator(iter((-7, 3))) == -4
+
+
 def test_implementation_details():
     lst = [39, 43, 92, 49, 22, 29, 93, 98, 26, 57, 8]
     tup = tuple(lst)

tests/test_thread.cpp

@@ -0,0 +1,66 @@
+/*
+    tests/test_thread.cpp -- call pybind11 bound methods in threads
+
+    Copyright (c) 2021 Laramie Leavitt (Google LLC) <lar@google.com>
+
+    All rights reserved. Use of this source code is governed by a
+    BSD-style license that can be found in the LICENSE file.
+*/
+
+#include <pybind11/cast.h>
+#include <pybind11/pybind11.h>
+
+#include <chrono>
+#include <thread>
+
+#include "pybind11_tests.h"
+
+namespace py = pybind11;
+
+namespace {
+
+struct IntStruct {
+    explicit IntStruct(int v) : value(v) {};
+    ~IntStruct() { value = -value; }
+    IntStruct(const IntStruct&) = default;
+    IntStruct& operator=(const IntStruct&) = default;
+
+    int value;
+};
+
+} // namespace
+
+TEST_SUBMODULE(thread, m) {
+
+    py::class_<IntStruct>(m, "IntStruct").def(py::init([](const int i) { return IntStruct(i); }));
+
+    // implicitly_convertible uses loader_life_support when an implicit
+    // conversion is required in order to lifetime extend the reference.
+    //
+    // This test should be run with ASAN for better effectiveness.
+    py::implicitly_convertible<int, IntStruct>();
+
+    m.def("test", [](int expected, const IntStruct &in) {
+        {
+            py::gil_scoped_release release;
+            std::this_thread::sleep_for(std::chrono::milliseconds(5));
+        }
+
+        if (in.value != expected) {
+            throw std::runtime_error("Value changed!!");
+        }
+    });
+
+    m.def(
+        "test_no_gil",
+        [](int expected, const IntStruct &in) {
+            std::this_thread::sleep_for(std::chrono::milliseconds(5));
+            if (in.value != expected) {
+                throw std::runtime_error("Value changed!!");
+            }
+        },
+        py::call_guard<py::gil_scoped_release>());
+
+    // NOTE: std::string_view also uses loader_life_support to ensure that
+    // the string contents remain alive, but that's a C++ 17 feature.
+}

tests/test_thread.py

@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+
+import threading
+
+from pybind11_tests import thread as m
+
+
+class Thread(threading.Thread):
+    def __init__(self, fn):
+        super(Thread, self).__init__()
+        self.fn = fn
+        self.e = None
+
+    def run(self):
+        try:
+            for i in range(10):
+                self.fn(i, i)
+        except Exception as e:
+            self.e = e
+
+    def join(self):
+        super(Thread, self).join()
+        if self.e:
+            raise self.e
+
+
+def test_implicit_conversion():
+    a = Thread(m.test)
+    b = Thread(m.test)
+    c = Thread(m.test)
+    for x in [a, b, c]:
+        x.start()
+    for x in [c, b, a]:
+        x.join()
+
+
+def test_implicit_conversion_no_gil():
+    a = Thread(m.test_no_gil)
+    b = Thread(m.test_no_gil)
+    c = Thread(m.test_no_gil)
+    for x in [a, b, c]:
+        x.start()
+    for x in [c, b, a]:
+        x.join()