Skip to content

Commit

Permalink
Merge pull request #30078 from rwgk/pywrapcc_merge_sh
Browse files Browse the repository at this point in the history
git merge smart_holder
  • Loading branch information
Ralf W. Grosse-Kunstleve authored Nov 8, 2023
2 parents bf3c48a + a3a9087 commit a66e50e
Show file tree
Hide file tree
Showing 11 changed files with 167 additions and 37 deletions.
15 changes: 5 additions & 10 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,18 @@ repos:

# Clang format the codebase automatically
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: "v17.0.3"
rev: "v17.0.4"
hooks:
- id: clang-format
types_or: [c++, c, cuda]

# Black, the code formatter, natively supports pre-commit
- repo: https://github.com/psf/black-pre-commit-mirror
rev: "23.10.1" # Keep in sync with blacken-docs
hooks:
- id: black

# Ruff, the Python auto-correcting linter written in Rust
# Ruff, the Python auto-correcting linter/formatter written in Rust
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.2
rev: v0.1.4
hooks:
- id: ruff
args: ["--fix", "--show-fixes"]
- id: ruff-format

# Check static types with mypy
- repo: https://github.com/pre-commit/mirrors-mypy
Expand Down Expand Up @@ -89,7 +84,7 @@ repos:
hooks:
- id: blacken-docs
additional_dependencies:
- black==23.3.0 # keep in sync with black hook
- black==23.*

# Changes tabs to spaces
- repo: https://github.com/Lucas-C/pre-commit-hooks
Expand Down
2 changes: 1 addition & 1 deletion include/pybind11/detail/smart_holder_poc.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ struct guarded_delete {

template <typename T, typename std::enable_if<std::is_destructible<T>::value, int>::type = 0>
inline void builtin_delete_if_destructible(void *raw_ptr) {
delete static_cast<T *>(raw_ptr);
std::default_delete<T>{}(static_cast<T *>(raw_ptr));
}

template <typename T, typename std::enable_if<!std::is_destructible<T>::value, int>::type = 0>
Expand Down
45 changes: 42 additions & 3 deletions include/pybind11/detail/smart_holder_type_casters.h
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,27 @@ struct shared_ptr_trampoline_self_life_support {
}
};

template <typename T,
typename D,
typename std::enable_if<std::is_default_constructible<D>::value, int>::type = 0>
inline std::unique_ptr<T, D> unique_with_deleter(T *raw_ptr, std::unique_ptr<D> &&deleter) {
if (deleter == nullptr) {
return std::unique_ptr<T, D>(raw_ptr);
}
return std::unique_ptr<T, D>(raw_ptr, std::move(*deleter));
}

template <typename T,
typename D,
typename std::enable_if<!std::is_default_constructible<D>::value, int>::type = 0>
inline std::unique_ptr<T, D> unique_with_deleter(T *raw_ptr, std::unique_ptr<D> &&deleter) {
if (deleter == nullptr) {
pybind11_fail("smart_holder_type_casters: deleter is not default constructible and no"
" instance available to return.");
}
return std::unique_ptr<T, D>(raw_ptr, std::move(*deleter));
}

template <typename T>
struct smart_holder_type_caster_load {
using holder_type = pybindit::memory::smart_holder;
Expand Down Expand Up @@ -589,7 +610,7 @@ struct smart_holder_type_caster_load {
" std::unique_ptr.");
}
if (!have_holder()) {
return nullptr;
return unique_with_deleter<T, D>(nullptr, std::unique_ptr<D>());
}
throw_if_uninitialized_or_disowned_holder();
throw_if_instance_is_currently_owned_by_shared_ptr();
Expand All @@ -616,13 +637,30 @@ struct smart_holder_type_caster_load {
"instance cannot safely be transferred to C++.");
}

// Temporary variable to store the extracted deleter in.
std::unique_ptr<D> extracted_deleter;

auto *gd = std::get_deleter<pybindit::memory::guarded_delete>(holder().vptr);
if (gd && gd->use_del_fun) { // Note the ensure_compatible_rtti_uqp_del<T, D>() call above.
// In smart_holder_poc, a custom deleter is always stored in a guarded delete.
// The guarded delete's std::function<void(void*)> actually points at the
// custom_deleter type, so we can verify it is of the custom deleter type and
// finally extract its deleter.
using custom_deleter_D = pybindit::memory::custom_deleter<T, D>;
const auto &custom_deleter_ptr = gd->del_fun.template target<custom_deleter_D>();
assert(custom_deleter_ptr != nullptr);
// Now that we have confirmed the type of the deleter matches the desired return
// value we can extract the function.
extracted_deleter = std::unique_ptr<D>(new D(std::move(custom_deleter_ptr->deleter)));
}

// Critical transfer-of-ownership section. This must stay together.
if (self_life_support != nullptr) {
holder().disown();
} else {
holder().release_ownership();
}
auto result = std::unique_ptr<T, D>(raw_type_ptr);
auto result = unique_with_deleter<T, D>(raw_type_ptr, std::move(extracted_deleter));
if (self_life_support != nullptr) {
self_life_support->activate_life_support(load_impl.loaded_v_h);
} else {
Expand Down Expand Up @@ -1056,7 +1094,8 @@ struct smart_holder_type_caster<std::unique_ptr<T const, D>>
static handle
cast(std::unique_ptr<T const, D> &&src, return_value_policy policy, handle parent) {
return smart_holder_type_caster<std::unique_ptr<T, D>>::cast(
std::unique_ptr<T, D>(const_cast<T *>(src.release())), // Const2Mutbl
std::unique_ptr<T, D>(const_cast<T *>(src.release()),
std::move(src.get_deleter())), // Const2Mutbl
policy,
parent);
}
Expand Down
5 changes: 5 additions & 0 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -3043,10 +3043,15 @@ get_type_override(const void *this_ptr, const type_info *this_type, const char *
if ((std::string) str(f_code->co_name) == name && f_code->co_argcount > 0) {
PyObject *locals = PyEval_GetLocals();
if (locals != nullptr) {
# if PY_VERSION_HEX >= 0x030b0000
PyObject *co_varnames = PyCode_GetVarnames(f_code);
# else
PyObject *co_varnames = PyObject_GetAttrString((PyObject *) f_code, "co_varnames");
# endif
PyObject *self_arg = PyTuple_GET_ITEM(co_varnames, 0);
Py_DECREF(co_varnames);
PyObject *self_caller = dict_getitem(locals, self_arg);
Py_DECREF(locals);
if (self_caller == self.ptr()) {
Py_DECREF(f_code);
Py_DECREF(frame);
Expand Down
4 changes: 3 additions & 1 deletion pybind11/setup_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@
from setuptools import Extension as _Extension
from setuptools.command.build_ext import build_ext as _build_ext
except ImportError:
from distutils.command.build_ext import build_ext as _build_ext # type: ignore[assignment]
from distutils.command.build_ext import ( # type: ignore[assignment]
build_ext as _build_ext,
)
from distutils.extension import Extension as _Extension # type: ignore[assignment]

import distutils.ccompiler
Expand Down
3 changes: 0 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ messages_control.disable = [
[tool.ruff]
target-version = "py37"
src = ["src"]
line-length = 120

[tool.ruff.lint]
extend-select = [
Expand All @@ -71,7 +70,6 @@ extend-select = [
"C4", # flake8-comprehensions
"EM", # flake8-errmsg
"ICN", # flake8-import-conventions
"ISC", # flake8-implicit-str-concat
"PGH", # pygrep-hooks
"PIE", # flake8-pie
"PL", # pylint
Expand All @@ -90,7 +88,6 @@ ignore = [
"SIM118", # iter(x) is not always the same as iter(x.keys())
]
unfixable = ["T20"]
exclude = []
isort.known-first-party = ["env", "pybind11_cross_module_tests", "pybind11_tests"]

[tool.ruff.lint.per-file-ignores]
Expand Down
61 changes: 61 additions & 0 deletions tests/test_class_sh_basic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,43 @@ struct uconsumer { // unique_ptr consumer
const std::unique_ptr<atyp> &rtrn_cref() const { return held; }
};

/// Custom deleter that is default constructible.
struct custom_deleter {
std::string trace_txt;

custom_deleter() = default;
explicit custom_deleter(const std::string &trace_txt_) : trace_txt(trace_txt_) {}

custom_deleter(const custom_deleter &other) { trace_txt = other.trace_txt + "_CpCtor"; }

custom_deleter &operator=(const custom_deleter &rhs) {
trace_txt = rhs.trace_txt + "_CpLhs";
return *this;
}

custom_deleter(custom_deleter &&other) noexcept {
trace_txt = other.trace_txt + "_MvCtorTo";
other.trace_txt += "_MvCtorFrom";
}

custom_deleter &operator=(custom_deleter &&rhs) noexcept {
trace_txt = rhs.trace_txt + "_MvLhs";
rhs.trace_txt += "_MvRhs";
return *this;
}

void operator()(atyp *p) const { std::default_delete<atyp>()(p); }
void operator()(const atyp *p) const { std::default_delete<const atyp>()(p); }
};
static_assert(std::is_default_constructible<custom_deleter>::value, "");

/// Custom deleter that is not default constructible.
struct custom_deleter_nd : custom_deleter {
custom_deleter_nd() = delete;
explicit custom_deleter_nd(const std::string &trace_txt_) : custom_deleter(trace_txt_) {}
};
static_assert(!std::is_default_constructible<custom_deleter_nd>::value, "");

// clang-format off

atyp rtrn_valu() { atyp obj{"rtrn_valu"}; return obj; }
Expand Down Expand Up @@ -64,6 +101,18 @@ std::unique_ptr<atyp const, sddc> rtrn_udcp() { return std::unique_ptr<atyp cons
std::string pass_udmp(std::unique_ptr<atyp, sddm> obj) { return "pass_udmp:" + obj->mtxt; }
std::string pass_udcp(std::unique_ptr<atyp const, sddc> obj) { return "pass_udcp:" + obj->mtxt; }

std::unique_ptr<atyp, custom_deleter> rtrn_udmp_del() { return std::unique_ptr<atyp, custom_deleter>(new atyp{"rtrn_udmp_del"}, custom_deleter{"udmp_deleter"}); }
std::unique_ptr<atyp const, custom_deleter> rtrn_udcp_del() { return std::unique_ptr<atyp const, custom_deleter>(new atyp{"rtrn_udcp_del"}, custom_deleter{"udcp_deleter"}); }

std::string pass_udmp_del(std::unique_ptr<atyp, custom_deleter> obj) { return "pass_udmp_del:" + obj->mtxt + "," + obj.get_deleter().trace_txt; }
std::string pass_udcp_del(std::unique_ptr<atyp const, custom_deleter> obj) { return "pass_udcp_del:" + obj->mtxt + "," + obj.get_deleter().trace_txt; }

std::unique_ptr<atyp, custom_deleter_nd> rtrn_udmp_del_nd() { return std::unique_ptr<atyp, custom_deleter_nd>(new atyp{"rtrn_udmp_del_nd"}, custom_deleter_nd{"udmp_deleter_nd"}); }
std::unique_ptr<atyp const, custom_deleter_nd> rtrn_udcp_del_nd() { return std::unique_ptr<atyp const, custom_deleter_nd>(new atyp{"rtrn_udcp_del_nd"}, custom_deleter_nd{"udcp_deleter_nd"}); }

std::string pass_udmp_del_nd(std::unique_ptr<atyp, custom_deleter_nd> obj) { return "pass_udmp_del_nd:" + obj->mtxt + "," + obj.get_deleter().trace_txt; }
std::string pass_udcp_del_nd(std::unique_ptr<atyp const, custom_deleter_nd> obj) { return "pass_udcp_del_nd:" + obj->mtxt + "," + obj.get_deleter().trace_txt; }

// clang-format on

// Helpers for testing.
Expand Down Expand Up @@ -130,6 +179,18 @@ TEST_SUBMODULE(class_sh_basic, m) {
m.def("pass_udmp", pass_udmp);
m.def("pass_udcp", pass_udcp);

m.def("rtrn_udmp_del", rtrn_udmp_del);
m.def("rtrn_udcp_del", rtrn_udcp_del);

m.def("pass_udmp_del", pass_udmp_del);
m.def("pass_udcp_del", pass_udcp_del);

m.def("rtrn_udmp_del_nd", rtrn_udmp_del_nd);
m.def("rtrn_udcp_del_nd", rtrn_udcp_del_nd);

m.def("pass_udmp_del_nd", pass_udmp_del_nd);
m.def("pass_udcp_del_nd", pass_udcp_del_nd);

py::classh<uconsumer>(m, "uconsumer")
.def(py::init<>())
.def("valid", &uconsumer::valid)
Expand Down
29 changes: 29 additions & 0 deletions tests/test_class_sh_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,35 @@ def test_load_with_rtrn_f(pass_f, rtrn_f, expected):
assert pass_f(rtrn_f()) == expected


@pytest.mark.parametrize(
("pass_f", "rtrn_f", "regex_expected"),
[
(
m.pass_udmp_del,
m.rtrn_udmp_del,
"pass_udmp_del:rtrn_udmp_del,udmp_deleter(_MvCtorTo)*_MvCtorTo",
),
(
m.pass_udcp_del,
m.rtrn_udcp_del,
"pass_udcp_del:rtrn_udcp_del,udcp_deleter(_MvCtorTo)*_MvCtorTo",
),
(
m.pass_udmp_del_nd,
m.rtrn_udmp_del_nd,
"pass_udmp_del_nd:rtrn_udmp_del_nd,udmp_deleter_nd(_MvCtorTo)*_MvCtorTo",
),
(
m.pass_udcp_del_nd,
m.rtrn_udcp_del_nd,
"pass_udcp_del_nd:rtrn_udcp_del_nd,udcp_deleter_nd(_MvCtorTo)*_MvCtorTo",
),
],
)
def test_deleter_roundtrip(pass_f, rtrn_f, regex_expected):
assert re.match(regex_expected, pass_f(rtrn_f()))


@pytest.mark.parametrize(
("pass_f", "rtrn_f", "expected"),
[
Expand Down
4 changes: 1 addition & 3 deletions tests/test_enum.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ def test_unscoped_enum():
ETwo : Docstring for ETwo
EThree : Docstring for EThree""".split(
"\n"
):
EThree : Docstring for EThree""".split("\n"):
assert docstring_line in m.UnscopedEnum.__doc__

# Unscoped enums will accept ==/!= int comparisons
Expand Down
34 changes: 19 additions & 15 deletions tests/test_methods_and_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,25 +232,29 @@ def test_no_mixed_overloads():

with pytest.raises(RuntimeError) as excinfo:
m.ExampleMandA.add_mixed_overloads1()
assert str(
excinfo.value
) == "overloading a method with both static and instance methods is not supported; " + (
"#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for more details"
if not detailed_error_messages_enabled
else "error while attempting to bind static method ExampleMandA.overload_mixed1"
"(arg0: float) -> str"
assert (
str(excinfo.value)
== "overloading a method with both static and instance methods is not supported; "
+ (
"#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for more details"
if not detailed_error_messages_enabled
else "error while attempting to bind static method ExampleMandA.overload_mixed1"
"(arg0: float) -> str"
)
)

with pytest.raises(RuntimeError) as excinfo:
m.ExampleMandA.add_mixed_overloads2()
assert str(
excinfo.value
) == "overloading a method with both static and instance methods is not supported; " + (
"#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for more details"
if not detailed_error_messages_enabled
else "error while attempting to bind instance method ExampleMandA.overload_mixed2"
"(self: pybind11_tests.methods_and_attributes.ExampleMandA, arg0: int, arg1: int)"
" -> str"
assert (
str(excinfo.value)
== "overloading a method with both static and instance methods is not supported; "
+ (
"#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for more details"
if not detailed_error_messages_enabled
else "error while attempting to bind instance method ExampleMandA.overload_mixed2"
"(self: pybind11_tests.methods_and_attributes.ExampleMandA, arg0: int, arg1: int)"
" -> str"
)
)


Expand Down
2 changes: 1 addition & 1 deletion tools/pybind11Config.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ default is ``MODULE``. There are several options:
``OPT_SIZE``
Optimize for size, even if the ``CMAKE_BUILD_TYPE`` is not ``MinSizeRel``.
``THIN_LTO``
Use thin TLO instead of regular if there's a choice (pybind11's selection
Use thin LTO instead of regular if there's a choice (pybind11's selection
is disabled if ``CMAKE_INTERPROCEDURAL_OPTIMIZATIONS`` is set).
``WITHOUT_SOABI``
Disable the SOABI component (``PYBIND11_NEWPYTHON`` mode only).
Expand Down

0 comments on commit a66e50e

Please sign in to comment.