From 362f69e3e821aa4eebbd678fa9bc6911fe425e1b Mon Sep 17 00:00:00 2001
From: Ryan Cahoon <ryan.cahoon@zoox.com>
Date: Thu, 7 Oct 2021 23:13:20 -0700
Subject: [PATCH 1/8] fix: the types for return_value_policy_override in
 optional_caster

`return_value_policy_override` was not being applied correctly in
`optional_caster` in two ways:
- The `is_lvalue_reference` condition referenced `T`, which was the
`optional<T>` type parameter from the class, when it should have used `T_`,
which was the parameter to the `cast` function. `T_` can potentially be a
reference type, but `T` will never be.
- The type parameter passed to `return_value_policy_override` should be
`T::value_type`, not `T`. This matches the way that the other STL container
type casters work.

The result of these issues was that a method/property definition which used a
`reference` or `reference_internal` return value policy would create a Python
value that's bound by reference to a temporary C++ object, resulting in
undefined behavior. For reasons that I was not able to figure out fully, it
seems like this causes problems when using old versions of `boost::optional`,
but not with recent versions of `boost::optional` or the `libstdc++`
implementation of `std::optional`. The issue (that the override to
`return_value_policy::move` is never being applied) is present for all
implementations, it just seems like that somehow doesn't result in problems for
the some implementation of `optional`. This change includes a regression type
with a custom optional-like type which was able to reproduce the issue.

Part of the issue with using the wrong types may have stemmed from the type
variables `T` and `T_` having very similar names. This also changes the type
variables in `optional_caster` to use slightly more descriptive names, which
also more closely follow the naming convention used by the other STL casters.

Fixes #3330
---
 include/pybind11/stl.h |  16 ++--
 tests/test_stl.cpp     | 184 ++++++++++++++++++++++++++++++++++++++++-
 tests/test_stl.py      |  67 +++++++++++++++
 3 files changed, 257 insertions(+), 10 deletions(-)

diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h
index 2c017b4fef..3608d2989c 100644
--- a/include/pybind11/stl.h
+++ b/include/pybind11/stl.h
@@ -245,17 +245,17 @@ template <typename Key, typename Value, typename Hash, typename Equal, typename
   : map_caster<std::unordered_map<Key, Value, Hash, Equal, Alloc>, Key, Value> { };
 
 // This type caster is intended to be used for std::optional and std::experimental::optional
-template<typename T> struct optional_caster {
-    using value_conv = make_caster<typename T::value_type>;
+template<typename Type, typename Value = typename Type::value_type> struct optional_caster {
+    using value_conv = make_caster<Value>;
 
-    template <typename T_>
-    static handle cast(T_ &&src, return_value_policy policy, handle parent) {
+    template <typename T>
+    static handle cast(T &&src, return_value_policy policy, handle parent) {
         if (!src)
             return none().inc_ref();
         if (!std::is_lvalue_reference<T>::value) {
-            policy = return_value_policy_override<T>::policy(policy);
+            policy = return_value_policy_override<Value>::policy(policy);
         }
-        return value_conv::cast(*std::forward<T_>(src), policy, parent);
+        return value_conv::cast(*std::forward<T>(src), policy, parent);
     }
 
     bool load(handle src, bool convert) {
@@ -269,11 +269,11 @@ template<typename T> struct optional_caster {
         if (!inner_caster.load(src, convert))
             return false;
 
-        value.emplace(cast_op<typename T::value_type &&>(std::move(inner_caster)));
+        value.emplace(cast_op<Value &&>(std::move(inner_caster)));
         return true;
     }
 
-    PYBIND11_TYPE_CASTER(T, _("Optional[") + value_conv::name + _("]"));
+    PYBIND11_TYPE_CASTER(Type, _("Optional[") + value_conv::name + _("]"));
 };
 
 #if defined(PYBIND11_HAS_OPTIONAL)
diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index 7e3363c5ea..a2d6960476 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -19,6 +19,18 @@
 #include <vector>
 #include <string>
 
+#if defined(PYBIND11_TEST_BOOST)
+#include <boost/optional.hpp>
+
+namespace pybind11 { namespace detail {
+template <typename T>
+struct type_caster<boost::optional<T>> : optional_caster<boost::optional<T>> {};
+
+template <>
+struct type_caster<boost::none_t> : void_caster<boost::none_t> {};
+}} // namespace pybind11::detail
+#endif
+
 // Test with `std::variant` in C++17 mode, or with `boost::variant` in C++11/14
 #if defined(PYBIND11_HAS_VARIANT)
 using std::variant;
@@ -67,6 +79,92 @@ struct OptionalHolder
 };
 
 
+enum class EnumType {
+  k0 = 0,
+  k1 = 1,
+};
+
+// This is used to test that return-by-ref and return-by-copy policies are
+// handled properly for optional types. This is a regression test for a dangling
+// reference issue. The issue seemed to require the enum value type to
+// reproduce - it didn't seem to happen if the value type is just an integer.
+template <template <typename> class OptionalImpl>
+class OptionalProperties {
+public:
+    using OptionalEnumValue = OptionalImpl<EnumType>;
+
+    OptionalProperties() : value(EnumType::k1) {}
+    ~OptionalProperties() {
+        // Reset value to detect use-after-destruction.
+        // This is set to a specific value rather than NULL to ensure that
+        // the memory that contains the value gets re-written.
+        value = EnumType::k0;
+    }
+
+    OptionalEnumValue& access_by_ref() { return value; }
+    OptionalEnumValue access_by_copy() { return value; }
+
+private:
+    OptionalEnumValue value;
+};
+
+// This type mimics aspects of boost::optional from old versions of Boost,
+// which exposed a dangling reference bug in Pybind11. Recent versions of
+// boost::optional, as well as libstdc++'s std::optional, don't seem to be
+// affected by the same issue. This is meant to be a minimal implementation
+// required to reproduce the issue, not fully standard-compliant.
+// See issue #3330 for more details.
+template <typename T>
+class ReferenceSensitiveOptional {
+public:
+    using value_type = T;
+
+    ReferenceSensitiveOptional() = default;
+    ReferenceSensitiveOptional(const T& value) : storage{value} {}
+    ReferenceSensitiveOptional(T&& value) : storage{std::move(value)} {}
+    ReferenceSensitiveOptional& operator=(const T& value) {
+        storage = {value};
+        return *this;
+    }
+    ReferenceSensitiveOptional& operator=(T&& value) {
+        storage = {std::move(value)};
+        return *this;
+    }
+
+    template <typename... Args>
+    T& emplace(Args&&... args) {
+        storage.clear();
+        storage.emplace_back(std::forward<Args>(args)...);
+        return storage.back();
+    }
+
+    const T& value() const noexcept {
+        assert(!storage.empty());
+        return storage[0];
+    }
+
+    const T& operator*() const noexcept {
+        return value();
+    }
+
+    const T* operator->() const noexcept {
+        return &value();
+    }
+
+    explicit operator bool() const noexcept {
+        return !storage.empty();
+    }
+
+private:
+    std::vector<T> storage;
+};
+
+namespace pybind11 { namespace detail {
+template <typename T>
+struct type_caster<ReferenceSensitiveOptional<T>> : optional_caster<ReferenceSensitiveOptional<T>> {};
+}} // namespace pybind11::detail
+
+
 TEST_SUBMODULE(stl, m) {
     // test_vector
     m.def("cast_vector", []() { return std::vector<int>{1}; });
@@ -145,6 +243,10 @@ TEST_SUBMODULE(stl, m) {
         return v;
     });
 
+    pybind11::enum_<EnumType>(m, "EnumType")
+        .value("k0", EnumType::k0)
+        .value("k1", EnumType::k1);
+
     // test_move_out_container
     struct MoveOutContainer {
         struct Value { int value; };
@@ -213,6 +315,12 @@ TEST_SUBMODULE(stl, m) {
         .def(py::init<>())
         .def_readonly("member", &opt_holder::member)
         .def("member_initialized", &opt_holder::member_initialized);
+
+    using opt_props = OptionalProperties<std::optional>;
+    pybind11::class_<opt_props>(m, "OptionalProperties")
+        .def(pybind11::init<>())
+        .def_property_readonly("access_by_ref", &opt_props::access_by_ref)
+        .def_property_readonly("access_by_copy", &opt_props::access_by_copy);
 #endif
 
 #ifdef PYBIND11_HAS_EXP_OPTIONAL
@@ -239,8 +347,76 @@ TEST_SUBMODULE(stl, m) {
         .def(py::init<>())
         .def_readonly("member", &opt_exp_holder::member)
         .def("member_initialized", &opt_exp_holder::member_initialized);
+
+    using opt_exp_props = OptionalProperties<std::experimental::optional>;
+    pybind11::class_<opt_exp_props>(m, "OptionalExpProperties")
+        .def(pybind11::init<>())
+        .def_property_readonly("access_by_ref", &opt_exp_props::access_by_ref)
+        .def_property_readonly("access_by_copy", &opt_exp_props::access_by_copy);
+#endif
+
+#if defined(PYBIND11_TEST_BOOST)
+    // test_boost_optional
+    m.attr("has_boost_optional") = true;
+
+    using boost_opt_int = boost::optional<int>;
+    using boost_opt_no_assign = boost::optional<NoAssign>;
+    m.def("double_or_zero_boost", [](const boost_opt_int& x) -> int {
+        return x.value_or(0) * 2;
+    });
+    m.def("half_or_none_boost", [](int x) -> boost_opt_int {
+        return x ? boost_opt_int(x / 2) : boost_opt_int();
+    });
+    m.def("test_nullopt_boost", [](boost_opt_int x) {
+        return x.value_or(42);
+    }, py::arg_v("x", boost::none, "None"));
+    m.def("test_no_assign_boost", [](const boost_opt_no_assign &x) {
+        return x ? x->value : 42;
+    }, py::arg_v("x", boost::none, "None"));
+
+    using opt_boost_holder = OptionalHolder<boost::optional, MoveOutDetector>;
+    py::class_<opt_boost_holder>(m, "OptionalBoostHolder", "Class with optional member")
+        .def(py::init<>())
+        .def_readonly("member", &opt_boost_holder::member)
+        .def("member_initialized", &opt_boost_holder::member_initialized);
+
+    using opt_boost_props = OptionalProperties<boost::optional>;
+    pybind11::class_<opt_boost_props>(m, "OptionalBoostProperties")
+        .def(pybind11::init<>())
+        .def_property_readonly("access_by_ref", &opt_boost_props::access_by_ref)
+        .def_property_readonly("access_by_copy", &opt_boost_props::access_by_copy);
 #endif
 
+    // test_refsensitive_optional
+    m.attr("has_refsensitive_optional") = true;
+
+    using refsensitive_opt_int = ReferenceSensitiveOptional<int>;
+    using refsensitive_opt_no_assign = ReferenceSensitiveOptional<NoAssign>;
+    m.def("double_or_zero_refsensitive", [](const refsensitive_opt_int& x) -> int {
+        return (x ? x.value() : 0) * 2;
+    });
+    m.def("half_or_none_refsensitive", [](int x) -> refsensitive_opt_int {
+        return x ? refsensitive_opt_int(x / 2) : refsensitive_opt_int();
+    });
+    m.def("test_nullopt_refsensitive", [](refsensitive_opt_int x) {
+        return x ? x.value() : 42;
+    }, py::arg_v("x", refsensitive_opt_int(), "None"));
+    m.def("test_no_assign_refsensitive", [](const refsensitive_opt_no_assign &x) {
+        return x ? x->value : 42;
+    }, py::arg_v("x", refsensitive_opt_no_assign(), "None"));
+
+    using opt_refsensitive_holder = OptionalHolder<ReferenceSensitiveOptional, MoveOutDetector>;
+    py::class_<opt_refsensitive_holder>(m, "OptionalRefSensitiveHolder", "Class with optional member")
+        .def(py::init<>())
+        .def_readonly("member", &opt_refsensitive_holder::member)
+        .def("member_initialized", &opt_refsensitive_holder::member_initialized);
+
+    using opt_refsensitive_props = OptionalProperties<ReferenceSensitiveOptional>;
+    pybind11::class_<opt_refsensitive_props>(m, "OptionalRefSensitiveProperties")
+        .def(pybind11::init<>())
+        .def_property_readonly("access_by_ref", &opt_refsensitive_props::access_by_ref)
+        .def_property_readonly("access_by_copy", &opt_refsensitive_props::access_by_copy);
+
 #ifdef PYBIND11_HAS_FILESYSTEM
     // test_fs_path
     m.attr("has_filesystem") = true;
@@ -280,8 +456,12 @@ TEST_SUBMODULE(stl, m) {
     m.def("tpl_ctor_set", [](std::unordered_set<TplCtorClass> &) {});
 #if defined(PYBIND11_HAS_OPTIONAL)
     m.def("tpl_constr_optional", [](std::optional<TplCtorClass> &) {});
-#elif defined(PYBIND11_HAS_EXP_OPTIONAL)
-    m.def("tpl_constr_optional", [](std::experimental::optional<TplCtorClass> &) {});
+#endif
+#if defined(PYBIND11_HAS_EXP_OPTIONAL)
+    m.def("tpl_constr_optional_exp", [](std::experimental::optional<TplCtorClass> &) {});
+#endif
+#if defined(PYBIND11_TEST_BOOST)
+    m.def("tpl_constr_optional_boost", [](boost::optional<TplCtorClass> &) {});
 #endif
 
     // test_vec_of_reference_wrapper
diff --git a/tests/test_stl.py b/tests/test_stl.py
index 3f63d6c3a4..804d561152 100644
--- a/tests/test_stl.py
+++ b/tests/test_stl.py
@@ -132,6 +132,10 @@ def test_optional():
     assert mvalue.initialized
     assert holder.member_initialized()
 
+    props = m.OptionalProperties()
+    assert int(props.access_by_ref) == 1
+    assert int(props.access_by_copy) == 1
+
 
 @pytest.mark.skipif(
     not hasattr(m, "has_exp_optional"), reason="no <experimental/optional>"
@@ -160,6 +164,69 @@ def test_exp_optional():
     assert mvalue.initialized
     assert holder.member_initialized()
 
+    props = m.OptionalExpProperties()
+    assert int(props.access_by_ref) == 1
+    assert int(props.access_by_copy) == 1
+
+
+@pytest.mark.skipif(not hasattr(m, "has_boost_optional"), reason="no <boost/optional>")
+def test_boost_optional():
+    assert m.double_or_zero_boost(None) == 0
+    assert m.double_or_zero_boost(42) == 84
+    pytest.raises(TypeError, m.double_or_zero_boost, "foo")
+
+    assert m.half_or_none_boost(0) is None
+    assert m.half_or_none_boost(42) == 21
+    pytest.raises(TypeError, m.half_or_none_boost, "foo")
+
+    assert m.test_nullopt_boost() == 42
+    assert m.test_nullopt_boost(None) == 42
+    assert m.test_nullopt_boost(42) == 42
+    assert m.test_nullopt_boost(43) == 43
+
+    assert m.test_no_assign_boost() == 42
+    assert m.test_no_assign_boost(None) == 42
+    assert m.test_no_assign_boost(m.NoAssign(43)) == 43
+    pytest.raises(TypeError, m.test_no_assign_boost, 43)
+
+    holder = m.OptionalBoostHolder()
+    mvalue = holder.member
+    assert mvalue.initialized
+    assert holder.member_initialized()
+
+    props = m.OptionalBoostProperties()
+    assert int(props.access_by_ref) == 1
+    assert int(props.access_by_copy) == 1
+
+
+def test_reference_sensitive_optional():
+    assert m.double_or_zero_refsensitive(None) == 0
+    assert m.double_or_zero_refsensitive(42) == 84
+    pytest.raises(TypeError, m.double_or_zero_refsensitive, "foo")
+
+    assert m.half_or_none_refsensitive(0) is None
+    assert m.half_or_none_refsensitive(42) == 21
+    pytest.raises(TypeError, m.half_or_none_refsensitive, "foo")
+
+    assert m.test_nullopt_refsensitive() == 42
+    assert m.test_nullopt_refsensitive(None) == 42
+    assert m.test_nullopt_refsensitive(42) == 42
+    assert m.test_nullopt_refsensitive(43) == 43
+
+    assert m.test_no_assign_refsensitive() == 42
+    assert m.test_no_assign_refsensitive(None) == 42
+    assert m.test_no_assign_refsensitive(m.NoAssign(43)) == 43
+    pytest.raises(TypeError, m.test_no_assign_refsensitive, 43)
+
+    holder = m.OptionalRefSensitiveHolder()
+    mvalue = holder.member
+    assert mvalue.initialized
+    assert holder.member_initialized()
+
+    props = m.OptionalRefSensitiveProperties()
+    assert int(props.access_by_ref) == 1
+    assert int(props.access_by_copy) == 1
+
 
 @pytest.mark.skipif(not hasattr(m, "has_filesystem"), reason="no <filesystem>")
 def test_fs_path():

From b486544b4781014979dfda420fca9519b815a8b3 Mon Sep 17 00:00:00 2001
From: Aaron Gokaslan <skylion.aaron@gmail.com>
Date: Mon, 18 Oct 2021 12:18:01 -0400
Subject: [PATCH 2/8] Fix clang-tidy complaints

---
 tests/test_stl.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index a2d6960476..280d0b426d 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -120,7 +120,9 @@ class ReferenceSensitiveOptional {
     using value_type = T;
 
     ReferenceSensitiveOptional() = default;
+    // NOLINTNEXTLINE(google-explicit-constructor)
     ReferenceSensitiveOptional(const T& value) : storage{value} {}
+    // NOLINTNEXTLINE(google-explicit-constructor)
     ReferenceSensitiveOptional(T&& value) : storage{std::move(value)} {}
     ReferenceSensitiveOptional& operator=(const T& value) {
         storage = {value};
@@ -162,7 +164,8 @@ class ReferenceSensitiveOptional {
 namespace pybind11 { namespace detail {
 template <typename T>
 struct type_caster<ReferenceSensitiveOptional<T>> : optional_caster<ReferenceSensitiveOptional<T>> {};
-}} // namespace pybind11::detail
+} // namespace pybind11::detail
+} // namespace pybind11
 
 
 TEST_SUBMODULE(stl, m) {
@@ -365,7 +368,7 @@ TEST_SUBMODULE(stl, m) {
         return x.value_or(0) * 2;
     });
     m.def("half_or_none_boost", [](int x) -> boost_opt_int {
-        return x ? boost_opt_int(x / 2) : boost_opt_int();
+        return x != 0 ? boost_opt_int(x / 2) : boost_opt_int();
     });
     m.def("test_nullopt_boost", [](boost_opt_int x) {
         return x.value_or(42);

From 4bdd6fc161aa259efe51fca6b9a4aecdfbacd3be Mon Sep 17 00:00:00 2001
From: Aaron Gokaslan <skylion.aaron@gmail.com>
Date: Mon, 18 Oct 2021 12:20:45 -0400
Subject: [PATCH 3/8] Add missing NOLINT

---
 tests/test_stl.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index 280d0b426d..ff2b7a7179 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -401,6 +401,7 @@ TEST_SUBMODULE(stl, m) {
     m.def("half_or_none_refsensitive", [](int x) -> refsensitive_opt_int {
         return x ? refsensitive_opt_int(x / 2) : refsensitive_opt_int();
     });
+    // NOLINTNEXTLINE(performance-unnecessary-value-param)
     m.def("test_nullopt_refsensitive", [](refsensitive_opt_int x) {
         return x ? x.value() : 42;
     }, py::arg_v("x", refsensitive_opt_int(), "None"));

From cab49554b9c2332a4244cf0abff87f78ae11cb61 Mon Sep 17 00:00:00 2001
From: Aaron Gokaslan <skylion.aaron@gmail.com>
Date: Mon, 18 Oct 2021 14:27:35 -0400
Subject: [PATCH 4/8] Apply a couple more fixes

---
 tests/test_stl.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index ff2b7a7179..0f0419b50c 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -164,7 +164,7 @@ class ReferenceSensitiveOptional {
 namespace pybind11 { namespace detail {
 template <typename T>
 struct type_caster<ReferenceSensitiveOptional<T>> : optional_caster<ReferenceSensitiveOptional<T>> {};
-} // namespace pybind11::detail
+} // namespace detail
 } // namespace pybind11
 
 
@@ -399,7 +399,7 @@ TEST_SUBMODULE(stl, m) {
         return (x ? x.value() : 0) * 2;
     });
     m.def("half_or_none_refsensitive", [](int x) -> refsensitive_opt_int {
-        return x ? refsensitive_opt_int(x / 2) : refsensitive_opt_int();
+        return x != 0 ? refsensitive_opt_int(x / 2) : refsensitive_opt_int();
     });
     // NOLINTNEXTLINE(performance-unnecessary-value-param)
     m.def("test_nullopt_refsensitive", [](refsensitive_opt_int x) {

From 37970974da27b44fbdf3aef1adc335ce63592cba Mon Sep 17 00:00:00 2001
From: Henry Schreiner <HenrySchreinerIII@gmail.com>
Date: Mon, 25 Oct 2021 17:10:14 -0400
Subject: [PATCH 5/8] fix: support GCC 4.8

---
 tests/test_stl.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index 0f0419b50c..18b82f26bd 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -71,7 +71,8 @@ namespace std {
 template <template <typename> class OptionalImpl, typename T>
 struct OptionalHolder
 {
-    OptionalHolder() = default;
+    // NOLINTNEXTLINE(modernize-use-equals-default): breaks GCC 4.8
+    OptionalHolder() {};
     bool member_initialized() const {
         return member && member->initialized;
     }

From 2412ad0d95d407fe9bf208f7a924438378aa3f81 Mon Sep 17 00:00:00 2001
From: Henry Schreiner <henryschreineriii@gmail.com>
Date: Mon, 25 Oct 2021 17:44:32 -0400
Subject: [PATCH 6/8] tests: avoid warning about unknown compiler for compilers
 missing C++17

---
 tests/CMakeLists.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index d4e7b71e1d..e1b18dd74b 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -256,7 +256,9 @@ if(Boost_FOUND)
 endif()
 
 # Check if we need to add -lstdc++fs or -lc++fs or nothing
-if(MSVC)
+if(DEFINED CMAKE_CXX_STANDARD AND CMAKE_CXX_STANDARD LESS 17)
+  set(STD_FS_NO_LIB_NEEDED TRUE)
+elseif(MSVC)
   set(STD_FS_NO_LIB_NEEDED TRUE)
 else()
   file(
@@ -286,7 +288,7 @@ elseif(${STD_FS_NEEDS_CXXFS})
 elseif(${STD_FS_NO_LIB_NEEDED})
   set(STD_FS_LIB "")
 else()
-  message(WARNING "Unknown compiler - not passing -lstdc++fs")
+  message(WARNING "Unknown C++17 compiler - not passing -lstdc++fs")
   set(STD_FS_LIB "")
 endif()
 

From 1aed3becc685729e160f2f49bebf03292dcda765 Mon Sep 17 00:00:00 2001
From: Ryan Cahoon <ryan.cahoon@zoox.com>
Date: Mon, 25 Oct 2021 18:11:48 -0700
Subject: [PATCH 7/8] Remove unneeded test module attribute

---
 tests/test_stl.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index 18b82f26bd..84f91b2e61 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -392,8 +392,6 @@ TEST_SUBMODULE(stl, m) {
 #endif
 
     // test_refsensitive_optional
-    m.attr("has_refsensitive_optional") = true;
-
     using refsensitive_opt_int = ReferenceSensitiveOptional<int>;
     using refsensitive_opt_no_assign = ReferenceSensitiveOptional<NoAssign>;
     m.def("double_or_zero_refsensitive", [](const refsensitive_opt_int& x) -> int {

From 30a0b290ca1767308009721c69b0c8dcefbf217d Mon Sep 17 00:00:00 2001
From: Ryan Cahoon <ryan.cahoon@zoox.com>
Date: Mon, 25 Oct 2021 18:12:12 -0700
Subject: [PATCH 8/8] Change test enum to have more unique int values

---
 tests/test_stl.cpp | 14 +++++++-------
 tests/test_stl.py  | 16 ++++++++--------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index 84f91b2e61..bc5c6553a2 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -81,8 +81,8 @@ struct OptionalHolder
 
 
 enum class EnumType {
-  k0 = 0,
-  k1 = 1,
+  kSet = 42,
+  kUnset = 85,
 };
 
 // This is used to test that return-by-ref and return-by-copy policies are
@@ -94,12 +94,12 @@ class OptionalProperties {
 public:
     using OptionalEnumValue = OptionalImpl<EnumType>;
 
-    OptionalProperties() : value(EnumType::k1) {}
+    OptionalProperties() : value(EnumType::kSet) {}
     ~OptionalProperties() {
         // Reset value to detect use-after-destruction.
-        // This is set to a specific value rather than NULL to ensure that
+        // This is set to a specific value rather than nullopt to ensure that
         // the memory that contains the value gets re-written.
-        value = EnumType::k0;
+        value = EnumType::kUnset;
     }
 
     OptionalEnumValue& access_by_ref() { return value; }
@@ -248,8 +248,8 @@ TEST_SUBMODULE(stl, m) {
     });
 
     pybind11::enum_<EnumType>(m, "EnumType")
-        .value("k0", EnumType::k0)
-        .value("k1", EnumType::k1);
+        .value("kSet", EnumType::kSet)
+        .value("kUnset", EnumType::kUnset);
 
     // test_move_out_container
     struct MoveOutContainer {
diff --git a/tests/test_stl.py b/tests/test_stl.py
index 804d561152..e217975944 100644
--- a/tests/test_stl.py
+++ b/tests/test_stl.py
@@ -133,8 +133,8 @@ def test_optional():
     assert holder.member_initialized()
 
     props = m.OptionalProperties()
-    assert int(props.access_by_ref) == 1
-    assert int(props.access_by_copy) == 1
+    assert int(props.access_by_ref) == 42
+    assert int(props.access_by_copy) == 42
 
 
 @pytest.mark.skipif(
@@ -165,8 +165,8 @@ def test_exp_optional():
     assert holder.member_initialized()
 
     props = m.OptionalExpProperties()
-    assert int(props.access_by_ref) == 1
-    assert int(props.access_by_copy) == 1
+    assert int(props.access_by_ref) == 42
+    assert int(props.access_by_copy) == 42
 
 
 @pytest.mark.skipif(not hasattr(m, "has_boost_optional"), reason="no <boost/optional>")
@@ -195,8 +195,8 @@ def test_boost_optional():
     assert holder.member_initialized()
 
     props = m.OptionalBoostProperties()
-    assert int(props.access_by_ref) == 1
-    assert int(props.access_by_copy) == 1
+    assert int(props.access_by_ref) == 42
+    assert int(props.access_by_copy) == 42
 
 
 def test_reference_sensitive_optional():
@@ -224,8 +224,8 @@ def test_reference_sensitive_optional():
     assert holder.member_initialized()
 
     props = m.OptionalRefSensitiveProperties()
-    assert int(props.access_by_ref) == 1
-    assert int(props.access_by_copy) == 1
+    assert int(props.access_by_ref) == 42
+    assert int(props.access_by_copy) == 42
 
 
 @pytest.mark.skipif(not hasattr(m, "has_filesystem"), reason="no <filesystem>")