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.

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

Elide to-python conversion of setter return values #4621

Merged
merged 14 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 14 additions & 2 deletions include/pybind11/attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ struct is_method {
explicit is_method(const handle &c) : class_(c) {}
};

/// Annotation for setters
struct is_setter {};

/// Annotation for operators
struct is_operator {};

Expand Down Expand Up @@ -188,8 +191,8 @@ struct argument_record {
struct function_record {
function_record()
: is_constructor(false), is_new_style_constructor(false), is_stateless(false),
is_operator(false), is_method(false), has_args(false), has_kwargs(false),
prepend(false) {}
is_operator(false), is_method(false), is_setter(false), has_args(false),
has_kwargs(false), prepend(false) {}

/// Function name
char *name = nullptr; /* why no C++ strings? They generate heavier code.. */
Expand Down Expand Up @@ -230,6 +233,9 @@ struct function_record {
/// True if this is a method
bool is_method : 1;

/// True if this is a setter
bool is_setter : 1;

/// True if the function has a '*args' argument
bool has_args : 1;

Expand Down Expand Up @@ -426,6 +432,12 @@ struct process_attribute<is_method> : process_attribute_default<is_method> {
}
};

/// Process an attribute which indicates that this function is a setter
template <>
struct process_attribute<is_setter> : process_attribute_default<is_setter> {
static void init(const is_setter &, function_record *r) { r->is_setter = true; }
};

/// Process an attribute which indicates the parent scope of a method
template <>
struct process_attribute<scope> : process_attribute_default<scope> {
Expand Down
18 changes: 13 additions & 5 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class cpp_function : public function {
cpp_function() = default;
// NOLINTNEXTLINE(google-explicit-constructor)
cpp_function(std::nullptr_t) {}
cpp_function(std::nullptr_t, const is_setter &) {}

/// Construct a cpp_function from a vanilla function pointer
template <typename Return, typename... Args, typename... Extra>
Expand Down Expand Up @@ -244,10 +245,16 @@ class cpp_function : public function {
using Guard = extract_guard_t<Extra...>;

/* Perform the function call */
handle result
= cast_out::cast(std::move(args_converter).template call<Return, Guard>(cap->f),
policy,
call.parent);
handle result;
if (call.func.is_setter) {
(void) std::move(args_converter).template call<Return, Guard>(cap->f);
result = none().release();
} else {
result = cast_out::cast(
std::move(args_converter).template call<Return, Guard>(cap->f),
policy,
call.parent);
}

/* Invoke call policy post-call hook */
process_attributes<Extra...>::postcall(call, result);
Expand Down Expand Up @@ -1729,7 +1736,8 @@ class class_ : public detail::generic_type {
template <typename Getter, typename Setter, typename... Extra>
class_ &
def_property(const char *name, const Getter &fget, const Setter &fset, const Extra &...extra) {
return def_property(name, fget, cpp_function(method_adaptor<type>(fset)), extra...);
return def_property(
name, fget, cpp_function(method_adaptor<type>(fset), is_setter()), extra...);
}
template <typename Getter, typename... Extra>
class_ &def_property(const char *name,
Expand Down
34 changes: 34 additions & 0 deletions tests/test_methods_and_attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,38 @@ struct RValueRefParam {
std::size_t func4(std::string &&s) const & { return s.size(); }
};

namespace pybind11_tests {
namespace exercise_is_setter {

struct FieldBase {
int int_value() const { return int_value_; }

FieldBase &SetIntValue(int int_value) {
int_value_ = int_value;
return *this;
}

private:
int int_value_ = -99;
};

struct Field : FieldBase {};

void add_bindings(py::module &m) {
py::module sm = m.def_submodule("exercise_is_setter");
// NOTE: FieldBase is not wrapped, therefore ...
py::class_<Field>(sm, "Field")
.def(py::init<>())
.def_property(
"int_value",
&Field::int_value,
&Field::SetIntValue // ... the `FieldBase &` return value here cannot be converted.
);
}

} // namespace exercise_is_setter
} // namespace pybind11_tests

TEST_SUBMODULE(methods_and_attributes, m) {
// test_methods_and_attributes
py::class_<ExampleMandA> emna(m, "ExampleMandA");
Expand Down Expand Up @@ -456,4 +488,6 @@ TEST_SUBMODULE(methods_and_attributes, m) {
.def("func2", &RValueRefParam::func2)
.def("func3", &RValueRefParam::func3)
.def("func4", &RValueRefParam::func4);

pybind11_tests::exercise_is_setter::add_bindings(m);
}
9 changes: 9 additions & 0 deletions tests/test_methods_and_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,3 +522,12 @@ def test_rvalue_ref_param():
assert r.func2("1234") == 4
assert r.func3("12345") == 5
assert r.func4("123456") == 6


def test_is_setter():
fld = m.exercise_is_setter.Field()
assert fld.int_value == -99
setter_return = fld.int_value = 100
assert isinstance(setter_return, int)
assert setter_return == 100
assert fld.int_value == 100