Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[smart_holder] Remove type_caster ODR guard #5255

Merged
merged 1 commit into from
Jul 20, 2024
Merged

[smart_holder] Remove type_caster ODR guard #5255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 3 additions & 16 deletions include/pybind11/cast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)) {
Expand Down
160 changes: 27 additions & 133 deletions include/pybind11/detail/descr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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}};
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -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.
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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>
Expand All @@ -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)
Loading