Remove type_caster_odr_guard feature. (The feature will continue to live in the pybind11k repo.) (#5255)

This rolls back #4022 (including follow-on tweaks in other PRs).

Author: rwgk

CMakeLists.txt

@@ -157,7 +157,6 @@ set(PYBIND11_HEADERS
     include/pybind11/detail/smart_holder_sfinae_hooks_only.h
     include/pybind11/detail/smart_holder_type_casters.h
     include/pybind11/detail/type_caster_base.h
-    include/pybind11/detail/type_caster_odr_guard.h
     include/pybind11/detail/typeid.h
     include/pybind11/detail/value_and_holder.h
     include/pybind11/attr.h

include/pybind11/cast.h

@@ -14,7 +14,6 @@
 #include "detail/descr.h"
 #include "detail/smart_holder_sfinae_hooks_only.h"
 #include "detail/type_caster_base.h"
-#include "detail/type_caster_odr_guard.h"
 #include "detail/typeid.h"
 #include "pytypes.h"
 
@@ -48,20 +47,8 @@ class type_caster_for_class_ : public type_caster_base<T> {};
 template <typename type, typename SFINAE = void>
 class type_caster : public type_caster_for_class_<type> {};
 
-#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD)
-
-template <typename type>
-using make_caster_for_intrinsic = type_caster_odr_guard<type, type_caster<type>>;
-
-#else
-
 template <typename type>
-using make_caster_for_intrinsic = type_caster<type>;
-
-#endif
-
-template <typename type>
-using make_caster = make_caster_for_intrinsic<intrinsic_t<type>>;
+using make_caster = type_caster<intrinsic_t<type>>;
 
 template <typename T>
 struct type_uses_smart_holder_type_caster {
@@ -1179,8 +1166,8 @@ struct return_value_policy_override<
 };
 
 // Basic python -> C++ casting; throws if casting fails
-template <typename T>
-make_caster_for_intrinsic<T> &load_type(make_caster_for_intrinsic<T> &conv, const handle &handle) {
+template <typename T, typename SFINAE>
+type_caster<T, SFINAE> &load_type(type_caster<T, SFINAE> &conv, const handle &handle) {
     static_assert(!detail::is_pyobject<T>::value,
                   "Internal error: type_caster should only be used for C++ types");
     if (!conv.load(handle, true)) {

include/pybind11/detail/descr.h

@@ -1,6 +1,3 @@
-// Copyright (c) 2022 The Pybind Development Team.
-// All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
 /*
     pybind11/detail/descr.h: Helper type for concatenating type signatures at compile time
 
@@ -23,106 +20,21 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 #    define PYBIND11_DESCR_CONSTEXPR const
 #endif
 
-// struct src_loc below is to support type_caster_odr_guard.h
-// (see https://github.com/pybind/pybind11/pull/4022).
-// The ODR guard creates ODR violations itself (see WARNING below & in type_caster_odr_guard.h),
-// but is currently the only tool available.
-// The ODR is useful to know *for sure* what is safe and what is not, but that is only a
-// subset of what actually works in practice, in a specific environment. The implementation
-// here exploits the gray area (similar to a white hat hacker).
-// The dedicated test_type_caster_odr_guard_1, test_type_caster_odr_guard_2 pair of unit tests
-// passes reliably on almost all platforms that meet the compiler requirements (C++17, C++20),
-// except one (gcc 9.4.0 debug build).
-// In the pybind11 unit tests we want to test the ODR guard in as many environments as possible,
-// but it is NOT recommended to enable the guard in regular builds, production, or
-// debug. The guard is meant to be used similar to a sanitizer, to check for type_caster ODR
-// violations in binaries that are otherwise already fully tested and assumed to be healthy.
-//
-// * MSVC 2017 does not support __builtin_FILE(), __builtin_LINE().
-// * MSVC 193732825 C++17 windows-2020 is failing for unknown reasons.
-// * Intel 2021.6.0.20220226 (g++ 9.4 mode) __builtin_LINE() is unreliable
-//   (line numbers vary between translation units).
-#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE)                                   \
-    && !defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) && defined(PYBIND11_CPP17)                 \
-    && !defined(__INTEL_COMPILER)                                                                 \
-    && (!defined(_MSC_VER)                                                                        \
-        || (_MSC_VER >= 1920 /* MSVC 2019 or newer */                                             \
-            && (_MSC_FULL_VER < 193732825 || _MSC_FULL_VER > 193732826                            \
-                || defined(PYBIND11_CPP20))))
-#    define PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD
-#endif
-
-#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD)
-
-// Not using std::source_location because:
-// 1. "It is unspecified whether the copy/move constructors and the copy/move
-//    assignment operators of source_location are trivial and/or constexpr."
-//    (https://en.cppreference.com/w/cpp/utility/source_location).
-// 2. A matching no-op stub is needed (below) to avoid code duplication.
-struct src_loc {
-    const char *file;
-    unsigned line;
-
-    constexpr src_loc(const char *file, unsigned line) : file(file), line(line) {}
-
-    static constexpr src_loc here(const char *file = __builtin_FILE(),
-                                  unsigned line = __builtin_LINE()) {
-        return src_loc(file, line);
-    }
-
-    constexpr src_loc if_known_or(const src_loc &other) const {
-        if (file != nullptr) {
-            return *this;
-        }
-        return other;
-    }
-};
-
-#else
-
-// No-op stub, to avoid code duplication, expected to be optimized out completely.
-struct src_loc {
-    constexpr src_loc(const char *, unsigned) {}
-
-    static constexpr src_loc here(const char * = nullptr, unsigned = 0) {
-        return src_loc(nullptr, 0);
-    }
-
-    constexpr src_loc if_known_or(const src_loc &) const { return *this; }
-};
-
-#endif
-
-#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD)
-namespace { // WARNING: This creates an ODR violation in the ODR guard itself,
-            //          but we do not have any alternative at the moment.
-// The ODR violation here is a difference in constexpr between multiple TUs.
-// All definitions have the same data layout, the only difference is the
-// text const char* pointee (the pointees are identical in value),
-// src_loc const char* file pointee (the pointees are different in value),
-// src_loc unsigned line value.
-// See also: Comment above; WARNING in type_caster_odr_guard.h
-#endif
-
 /* Concatenate type signatures at compile time */
 template <size_t N, typename... Ts>
 struct descr {
     char text[N + 1]{'\0'};
-    const src_loc sloc;
 
-    explicit constexpr descr(src_loc sloc) : sloc(sloc) {}
+    constexpr descr() = default;
     // NOLINTNEXTLINE(google-explicit-constructor)
-    constexpr descr(char const (&s)[N + 1], src_loc sloc = src_loc::here())
-        : descr(s, make_index_sequence<N>(), sloc) {}
+    constexpr descr(char const (&s)[N + 1]) : descr(s, make_index_sequence<N>()) {}
 
     template <size_t... Is>
-    constexpr descr(char const (&s)[N + 1], index_sequence<Is...>, src_loc sloc = src_loc::here())
-        : text{s[Is]..., '\0'}, sloc(sloc) {}
+    constexpr descr(char const (&s)[N + 1], index_sequence<Is...>) : text{s[Is]..., '\0'} {}
 
     template <typename... Chars>
     // NOLINTNEXTLINE(google-explicit-constructor)
-    constexpr descr(src_loc sloc, char c, Chars... cs)
-        : text{c, static_cast<char>(cs)..., '\0'}, sloc(sloc) {}
+    constexpr descr(char c, Chars... cs) : text{c, static_cast<char>(cs)..., '\0'} {}
 
     static constexpr std::array<const std::type_info *, sizeof...(Ts) + 1> types() {
         return {{&typeid(Ts)..., nullptr}};
@@ -135,8 +47,7 @@ constexpr descr<N1 + N2, Ts1..., Ts2...> plus_impl(const descr<N1, Ts1...> &a,
                                                    index_sequence<Is1...>,
                                                    index_sequence<Is2...>) {
     PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(b);
-    return descr<N1 + N2, Ts1..., Ts2...>{
-        a.sloc.if_known_or(b.sloc), a.text[Is1]..., b.text[Is2]...};
+    return {a.text[Is1]..., b.text[Is2]...};
 }
 
 template <size_t N1, size_t N2, typename... Ts1, typename... Ts2>
@@ -146,33 +57,27 @@ constexpr descr<N1 + N2, Ts1..., Ts2...> operator+(const descr<N1, Ts1...> &a,
 }
 
 template <size_t N>
-constexpr descr<N - 1> const_name(char const (&text)[N], src_loc sloc = src_loc::here()) {
-    return descr<N - 1>(text, sloc);
-}
-constexpr descr<0> const_name(char const (&)[1], src_loc sloc = src_loc::here()) {
-    return descr<0>(sloc);
+constexpr descr<N - 1> const_name(char const (&text)[N]) {
+    return descr<N - 1>(text);
 }
+constexpr descr<0> const_name(char const (&)[1]) { return {}; }
 
 template <size_t Rem, size_t... Digits>
 struct int_to_str : int_to_str<Rem / 10, Rem % 10, Digits...> {};
 template <size_t... Digits>
 struct int_to_str<0, Digits...> {
     // WARNING: This only works with C++17 or higher.
-    // src_loc not tracked (not needed in this situation, at least at the moment).
-    static constexpr auto digits
-        = descr<sizeof...(Digits)>(src_loc{nullptr, 0}, ('0' + Digits)...);
+    static constexpr auto digits = descr<sizeof...(Digits)>(('0' + Digits)...);
 };
 
 // Ternary description (like std::conditional)
 template <bool B, size_t N1, size_t N2>
-constexpr enable_if_t<B, descr<N1 - 1>>
-const_name(char const (&text1)[N1], char const (&)[N2], src_loc sloc = src_loc::here()) {
-    return const_name(text1, sloc);
+constexpr enable_if_t<B, descr<N1 - 1>> const_name(char const (&text1)[N1], char const (&)[N2]) {
+    return const_name(text1);
 }
 template <bool B, size_t N1, size_t N2>
-constexpr enable_if_t<!B, descr<N2 - 1>>
-const_name(char const (&)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) {
-    return const_name(text2, sloc);
+constexpr enable_if_t<!B, descr<N2 - 1>> const_name(char const (&)[N1], char const (&text2)[N2]) {
+    return const_name(text2);
 }
 
 template <bool B, typename T1, typename T2>
@@ -186,13 +91,12 @@ constexpr enable_if_t<!B, T2> const_name(const T1 &, const T2 &d) {
 
 template <size_t Size>
 auto constexpr const_name() -> remove_cv_t<decltype(int_to_str<Size / 10, Size % 10>::digits)> {
-    // src_loc not tracked (not needed in this situation, at least at the moment).
     return int_to_str<Size / 10, Size % 10>::digits;
 }
 
 template <typename Type>
-constexpr descr<1, Type> const_name(src_loc sloc = src_loc::here()) {
-    return {sloc, '%'};
+constexpr descr<1, Type> const_name() {
+    return {'%'};
 }
 
 // If "_" is defined as a macro, py::detail::_ cannot be provided.
@@ -202,18 +106,16 @@ constexpr descr<1, Type> const_name(src_loc sloc = src_loc::here()) {
 #ifndef _
 #    define PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY
 template <size_t N>
-constexpr descr<N - 1> _(char const (&text)[N], src_loc sloc = src_loc::here()) {
-    return const_name<N>(text, sloc);
+constexpr descr<N - 1> _(char const (&text)[N]) {
+    return const_name<N>(text);
 }
 template <bool B, size_t N1, size_t N2>
-constexpr enable_if_t<B, descr<N1 - 1>>
-_(char const (&text1)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) {
-    return const_name<B, N1, N2>(text1, text2, sloc);
+constexpr enable_if_t<B, descr<N1 - 1>> _(char const (&text1)[N1], char const (&text2)[N2]) {
+    return const_name<B, N1, N2>(text1, text2);
 }
 template <bool B, size_t N1, size_t N2>
-constexpr enable_if_t<!B, descr<N2 - 1>>
-_(char const (&text1)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) {
-    return const_name<B, N1, N2>(text1, text2, sloc);
+constexpr enable_if_t<!B, descr<N2 - 1>> _(char const (&text1)[N1], char const (&text2)[N2]) {
+    return const_name<B, N1, N2>(text1, text2);
 }
 template <bool B, typename T1, typename T2>
 constexpr enable_if_t<B, T1> _(const T1 &d1, const T2 &d2) {
@@ -226,16 +128,15 @@ constexpr enable_if_t<!B, T2> _(const T1 &d1, const T2 &d2) {
 
 template <size_t Size>
 auto constexpr _() -> remove_cv_t<decltype(int_to_str<Size / 10, Size % 10>::digits)> {
-    // src_loc not tracked (not needed in this situation, at least at the moment).
     return const_name<Size>();
 }
 template <typename Type>
-constexpr descr<1, Type> _(src_loc sloc = src_loc::here()) {
-    return const_name<Type>(sloc);
+constexpr descr<1, Type> _() {
+    return const_name<Type>();
 }
 #endif // #ifndef _
 
-constexpr descr<0> concat(src_loc sloc = src_loc::here()) { return descr<0>{sloc}; }
+constexpr descr<0> concat() { return {}; }
 
 template <size_t N, typename... Ts>
 constexpr descr<N, Ts...> concat(const descr<N, Ts...> &descr) {
@@ -246,8 +147,7 @@ constexpr descr<N, Ts...> concat(const descr<N, Ts...> &descr) {
 template <size_t N1, size_t N2, typename... Ts1, typename... Ts2>
 constexpr descr<N1 + N2 + 2, Ts1..., Ts2...> operator,(const descr<N1, Ts1...> &a,
                                                        const descr<N2, Ts2...> &b) {
-    // Ensure that src_loc of existing descr is used.
-    return a + const_name(", ", src_loc{nullptr, 0}) + b;
+    return a + const_name(", ") + b;
 }
 
 template <size_t N, typename... Ts, typename... Args>
@@ -259,20 +159,14 @@ template <size_t N, typename... Ts, typename... Args>
 constexpr auto concat(const descr<N, Ts...> &d,
                       const Args &...args) -> decltype(std::declval<descr<N + 2, Ts...>>()
                                                        + concat(args...)) {
-    // Ensure that src_loc of existing descr is used.
-    return d + const_name(", ", src_loc{nullptr, 0}) + concat(args...);
+    return d + const_name(", ") + concat(args...);
 }
 #endif
 
 template <size_t N, typename... Ts>
 constexpr descr<N + 2, Ts...> type_descr(const descr<N, Ts...> &descr) {
-    // Ensure that src_loc of existing descr is used.
-    return const_name("{", src_loc{nullptr, 0}) + descr + const_name("}");
+    return const_name("{") + descr + const_name("}");
 }
 
-#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD)
-} // namespace
-#endif
-
 PYBIND11_NAMESPACE_END(detail)
 PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)