From 75811061b187ce49ea67e35b5e66dc5870503f0a Mon Sep 17 00:00:00 2001 From: ykiko Date: Fri, 23 Aug 2024 00:37:09 +0800 Subject: [PATCH] some fix. --- .github/workflows/pybind11.yml | 57 ++++------- include/pybind11/internal/cast.h | 2 +- include/pybind11/internal/class.h | 21 ++-- include/pybind11/internal/function.h | 142 +++++++++++++-------------- include/pybind11/internal/object.h | 22 ++--- include/pybind11/internal/types.h | 40 ++++---- include/pybind11/stl.h | 6 +- include/pybind11/tests/function.cpp | 2 +- 8 files changed, 138 insertions(+), 154 deletions(-) diff --git a/.github/workflows/pybind11.yml b/.github/workflows/pybind11.yml index eb413216..0e40dae0 100644 --- a/.github/workflows/pybind11.yml +++ b/.github/workflows/pybind11.yml @@ -2,11 +2,15 @@ name: CMake Build and Test on: push: - branches: - - pkpy-v2 + paths-ignore: + - "docs/**" + - "web/**" + - "**.md" pull_request: - branches: - - pkpy-v2 + paths-ignore: + - "docs/**" + - "web/**" + - "**.md" jobs: build_linux: @@ -14,7 +18,7 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v4 - + - name: Set up GCC run: | sudo apt-get update @@ -23,18 +27,11 @@ jobs: - name: Set up CMake uses: jwlawson/actions-setup-cmake@v1.10 - - name: Install dependencies - run: | - git submodule update --init --recursive - git clone https://github.com/google/googletest.git - - - name: Build - run: | - cmake -B build -DENABLE_TEST=ON - cmake --build build --config Release --parallel - - name: Test run: | + cd include/pybind11/tests + cmake -B build + cmake --build build --config Release --parallel build/pybind11_test build_win: @@ -42,25 +39,18 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v4 - + - name: Set up MSVC uses: ilammy/msvc-dev-cmd@v1 - name: Set up CMake uses: jwlawson/actions-setup-cmake@v1.10 - - name: Install dependencies - run: | - git submodule update --init --recursive - git clone https://github.com/google/googletest.git - - - name: Build - run: | - cmake -B build -DENABLE_TEST=ON - cmake --build build --config Release --parallel - - name: Test run: | + cd include\pybind11\tests + cmake -B build + cmake --build build --config Release --parallel build\Release\pybind11_test.exe build_mac: @@ -68,7 +58,7 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v4 - + - name: Set up Clang run: | brew install llvm @@ -78,16 +68,9 @@ jobs: - name: Set up CMake uses: jwlawson/actions-setup-cmake@v1.10 - - name: Install dependencies - run: | - git submodule update --init --recursive - git clone https://github.com/google/googletest.git - - - name: Build - run: | - cmake -B build -DENABLE_TEST=ON - cmake --build build --config Release --parallel - - name: Test run: | + cd include/pybind11/tests + cmake -B build -DENABLE_TEST=ON + cmake --build build --config Release --parallel build/pybind11_test diff --git a/include/pybind11/internal/cast.h b/include/pybind11/internal/cast.h index 2e847d95..b092050c 100644 --- a/include/pybind11/internal/cast.h +++ b/include/pybind11/internal/cast.h @@ -79,7 +79,7 @@ struct type_caster>> { bool load(handle src, bool) { if(isinstance(src)) { - data = py_toint(src.ptr()); + data = static_cast(py_toint(src.ptr())); return true; } diff --git a/include/pybind11/internal/class.h b/include/pybind11/internal/class.h index 50f30b6b..472a6996 100644 --- a/include/pybind11/internal/class.h +++ b/include/pybind11/internal/class.h @@ -1,6 +1,7 @@ #pragma once #include "module.h" +#include "type_traits.h" namespace pkbind { @@ -38,7 +39,8 @@ public: [[maybe_unused]] auto kwargs = py_offset(stack, 2); auto info = &type_info::of(); - int slot = (type_list::template count ? -1 : 0); + int slot = ((std::is_same_v || ...) ? -1 : 0); + std::cout << "<< " << slot << "\n"; void* data = py_newobject(retv, steal(cls).index(), slot, sizeof(instance)); new (data) instance{instance::Flag::Own, operator new (info->size), info}; return true; @@ -82,8 +84,9 @@ public: constexpr bool is_first_base_of_v = std::is_base_of_v || std::is_same_v; if constexpr(!is_first_base_of_v) { - static_assert(is_first_base_of_v, - "If you want to bind member function, the first argument must be the base class"); + static_assert( + is_first_base_of_v, + "If you want to bind member function, the first argument must be the base class"); } else { impl::bind_function(*this, name, std::forward(f), extra...); } @@ -108,7 +111,8 @@ public: template class_& def_readwrite(const char* name, MP mp, const Extras&... extras) { if constexpr(!std::is_member_object_pointer_v) { - static_assert(std::is_member_object_pointer_v, "def_readwrite only supports pointer to data member"); + static_assert(std::is_member_object_pointer_v, + "def_readwrite only supports pointer to data member"); } else { impl::bind_property( *this, @@ -127,7 +131,8 @@ public: template class_& def_readonly(const char* name, MP mp, const Extras&... extras) { if constexpr(!std::is_member_object_pointer_v) { - static_assert(std::is_member_object_pointer_v, "def_readonly only supports pointer to data member"); + static_assert(std::is_member_object_pointer_v, + "def_readonly only supports pointer to data member"); } else { impl::bind_property( *this, @@ -143,7 +148,11 @@ public: template class_& def_property(const char* name, Getter&& g, Setter&& s, const Extras&... extras) { - impl::bind_property(*this, name, std::forward(g), std::forward(s), extras...); + impl::bind_property(*this, + name, + std::forward(g), + std::forward(s), + extras...); return *this; } diff --git a/include/pybind11/internal/function.h b/include/pybind11/internal/function.h index 8be22c8c..934ea7a1 100644 --- a/include/pybind11/internal/function.h +++ b/include/pybind11/internal/function.h @@ -147,8 +147,8 @@ public: using Callable = std::decay_t; if constexpr(std::is_trivially_copyable_v && sizeof(Callable) <= sizeof(buffer)) { - // if the callable object is trivially copyable and the size is less than 16 bytes, store it in the - // buffer + // if the callable object is trivially copyable and the size is less than 16 bytes, + // store it in the buffer new (buffer) auto(std::forward(f)); destructor = [](function_record* self) { reinterpret_cast(self->buffer)->~Callable(); @@ -178,18 +178,10 @@ public: function_record& operator= (function_record&&) = delete; ~function_record() { - if(destructor) { - destructor(this); - } - if(arguments) { - delete arguments; - } - if(next) { - delete next; - } - if(signature) { - delete[] signature; - } + if(destructor) { destructor(this); } + if(arguments) { delete arguments; } + if(next) { delete next; } + if(signature) { delete[] signature; } } void append(function_record* record) { @@ -220,9 +212,7 @@ public: bool has_self = argc == 3; std::vector args; handle self = py_offset(stack.ptr(), 0); - if(has_self) { - args.push_back(self); - } + if(has_self) { args.push_back(self); } auto tuple = py_offset(stack.ptr(), 0 + has_self); for(int i = 0; i < py_tuple_len(tuple); ++i) { @@ -239,9 +229,7 @@ public: // foreach function record and call the function with not convert while(p != nullptr) { auto result = p->wrapper(*p, args, kwargs, false, self); - if(result) { - return; - } + if(result) { return; } p = p->next; } @@ -249,9 +237,7 @@ public: // foreach function record and call the function with convert while(p != nullptr) { auto result = p->wrapper(*p, args, kwargs, true, self); - if(result) { - return; - } + if(result) { return; } p = p->next; } @@ -299,14 +285,16 @@ void invoke(Fn&& fn, }; if constexpr(!is_void) { - py_assign(py_retval(), pkbind::cast(unpack(std::get(casters).value()...), policy, parent).ptr()); + py_assign(py_retval(), + pkbind::cast(unpack(std::get(casters).value()...), policy, parent).ptr()); } else { unpack(std::get(casters).value()...); py_newnone(py_retval()); } } else { if constexpr(!is_void) { - py_assign(py_retval(), pkbind::cast(fn(std::get(casters).value()...), policy, parent).ptr()); + py_assign(py_retval(), + pkbind::cast(fn(std::get(casters).value()...), policy, parent).ptr()); } else { fn(std::get(casters).value()...); py_newnone(py_retval()); @@ -315,7 +303,10 @@ void invoke(Fn&& fn, } template -struct template_parser, std::tuple, std::index_sequence> { +struct template_parser, + std::tuple, + std::index_sequence> { using types = type_list; /// count of the Callable parameters. @@ -333,7 +324,8 @@ struct template_parser, std::tuple, std // FIXME: temporarily, args and kwargs must be at the end of the arguments list /// if have py::kwargs, it must be at the end of the arguments list. - static_assert(kwargs_count == 0 || kwargs_pos == argc - 1, "py::kwargs must be the last parameter"); + static_assert(kwargs_count == 0 || kwargs_pos == argc - 1, + "py::kwargs must be the last parameter"); /// if have py::args, it must be before py::kwargs or at the end of the arguments list. static_assert(args_count == 0 || args_pos == kwargs_pos - 1 || args_pos == argc - 1, "py::args must be before py::kwargs or at the end of the parameter list"); @@ -348,14 +340,18 @@ struct template_parser, std::tuple, std constexpr inline static auto policy_pos = extras::template find; - constexpr inline static auto last_arg_without_default_pos = types::template find_last; - constexpr inline static auto first_arg_with_default_pos = types::template find; - static_assert(last_arg_without_default_pos < first_arg_with_default_pos || first_arg_with_default_pos == -1, + constexpr inline static auto last_arg_without_default_pos = + types::template find_last; + constexpr inline static auto first_arg_with_default_pos = + types::template find; + static_assert(last_arg_without_default_pos < first_arg_with_default_pos || + first_arg_with_default_pos == -1, "parameter with default value must be after parameter without default value"); /// count of named parameters(explicit with name). constexpr inline static auto named_only_argc = extras::template count; - constexpr inline static auto named_default_argc = extras::template count; + constexpr inline static auto named_default_argc = + extras::template count; constexpr inline static auto named_argc = named_only_argc + named_default_argc; /// count of normal parameters(which are not py::args or py::kwargs). @@ -368,9 +364,7 @@ struct template_parser, std::tuple, std static void initialize(function_record& record, const Extras&... extras) { auto extras_tuple = std::make_tuple(extras...); constexpr static bool has_named_args = (named_argc > 0); - if constexpr(policy_pos != -1) { - record.policy = std::get(extras_tuple); - } + if constexpr(policy_pos != -1) { record.policy = std::get(extras_tuple); } // TODO: set others @@ -410,15 +404,15 @@ struct template_parser, std::tuple, std sig += type_info::of().name; if(!record.arguments->defaults[index].empty()) { sig += " = "; - sig += record.arguments->defaults[index].attr("__repr__")().cast(); + sig += record.arguments->defaults[index] + .attr("__repr__")() + .cast(); } } else { sig += "_: "; sig += type_info::of().name; } - if(index + 1 < argc) { - sig += ", "; - } + if(index + 1 < argc) { sig += ", "; } index++; }; (append(type_identity{}), ...); @@ -430,8 +424,8 @@ struct template_parser, std::tuple, std } } - /// try to call a C++ function(store in function_record) with the arguments which are from Python. - /// if success, return true, otherwise return false. + /// try to call a C++ function(store in function_record) with the arguments which are from + /// Python. if success, return true, otherwise return false. static bool call(function_record& record, std::vector& args, std::vector>& kwargs, @@ -451,9 +445,7 @@ struct template_parser, std::tuple, std // load arguments from call arguments if(args.size() > normal_argc) { - if constexpr(args_pos == -1) { - return false; - } + if constexpr(args_pos == -1) { return false; } } for(std::size_t i = 0; i < std::min(normal_argc, (int)args.size()); ++i) { @@ -463,9 +455,10 @@ struct template_parser, std::tuple, std object repack_args; // pack the args if constexpr(args_pos != -1) { - const auto n = args.size() > normal_argc ? args.size() - normal_argc : 0; + const auto n = + static_cast(args.size() > normal_argc ? args.size() - normal_argc : 0); auto pack = tuple(n); - for(std::size_t i = 0; i < n; ++i) { + for(int i = 0; i < n; ++i) { pack[i] = args[normal_argc + i]; } repack_args = std::move(pack); @@ -500,16 +493,18 @@ struct template_parser, std::tuple, std // check if all the arguments are valid for(std::size_t i = 0; i < argc; ++i) { - if(!stack[i]) { - return false; - } + if(!stack[i]) { return false; } } // ok, all the arguments are valid, call the function std::tuple...> casters; if(((std::get(casters).load(stack[Is], convert)) && ...)) { - invoke(record.as(), std::index_sequence{}, casters, record.policy, parent); + invoke(record.as(), + std::index_sequence{}, + casters, + record.policy, + parent); return true; } @@ -533,15 +528,14 @@ class cpp_function : public function { static bool is_function_record(handle h) { if(isinstance(h)) { auto slot = py_getslot(h.ptr(), 0); - if(slot) { - return py_typeof(slot) == m_type; - } + if(slot) { return py_typeof(slot) == m_type; } } return false; } template - cpp_function(bool is_method, const char* name, Fn&& fn, const Extras&... extras) : function(alloc_t{}) { + cpp_function(bool is_method, const char* name, Fn&& fn, const Extras&... extras) : + function(alloc_t{}) { // bind the function std::string sig = name; sig += is_method ? "(self, *args, **kwargs)" : "(*args, **kwargs)"; @@ -562,13 +556,9 @@ class cpp_function : public function { py_exception(tp_IndexError, e.what()); } catch(std::range_error& e) { py_exception(tp_ValueError, e.what()); - } catch(stop_iteration& e) { - StopIteration(); - } catch(index_error& e) { + } catch(stop_iteration&) { StopIteration(); } catch(index_error& e) { py_exception(tp_IndexError, e.what()); - } catch(key_error& e) { - py_exception(tp_KeyError, e.what()); - } catch(value_error& e) { + } catch(key_error& e) { py_exception(tp_KeyError, e.what()); } catch(value_error& e) { py_exception(tp_ValueError, e.what()); } catch(type_error& e) { py_exception(tp_TypeError, e.what()); @@ -576,9 +566,7 @@ class cpp_function : public function { py_exception(tp_ImportError, e.what()); } catch(attribute_error& e) { py_exception(tp_AttributeError, e.what()); - } catch(std::exception& e) { - py_exception(tp_RuntimeError, e.what()); - } + } catch(std::exception& e) { py_exception(tp_RuntimeError, e.what()); } return false; }; py_newfunction(m_ptr, sig.c_str(), call, nullptr, 1); @@ -590,7 +578,8 @@ class cpp_function : public function { } template - cpp_function(Fn&& fn, const Extras&... extras) : cpp_function("lambda", std::forward(fn), extras...) {} + cpp_function(Fn&& fn, const Extras&... extras) : + cpp_function("lambda", std::forward(fn), extras...) {} }; class property : public object { @@ -620,7 +609,8 @@ namespace impl { template void bind_function(handle obj, const char* name_, Fn&& fn, const Extras&... extras) { - constexpr bool has_named_args = ((std::is_same_v || std::is_same_v) || ...); + constexpr bool has_named_args = + ((std::is_same_v || std::is_same_v) || ...); auto name = py_name(name_); auto func = py_getdict(obj.ptr(), name); @@ -634,23 +624,33 @@ void bind_function(handle obj, const char* name_, Fn&& fn, const Extras&... extr } } else { if constexpr(is_static) { - py_setdict(obj.ptr(), - name, - staticmethod(cpp_function(is_method, name_, std::forward(fn), extras...).ptr()).ptr()); + py_setdict( + obj.ptr(), + name, + staticmethod(cpp_function(is_method, name_, std::forward(fn), extras...).ptr()) + .ptr()); } else { if constexpr(has_named_args && is_method) { + py_setdict( + obj.ptr(), + name, + cpp_function(is_method, name_, std::forward(fn), arg("self"), extras...) + .ptr()); + } else { py_setdict(obj.ptr(), name, - cpp_function(is_method, name_, std::forward(fn), arg("self"), extras...).ptr()); - } else { - py_setdict(obj.ptr(), name, cpp_function(is_method, name_, std::forward(fn), extras...).ptr()); + cpp_function(is_method, name_, std::forward(fn), extras...).ptr()); } } } } template -void bind_property(handle obj, const char* name, Getter&& getter_, Setter&& setter_, const Extras&... extras) { +void bind_property(handle obj, + const char* name, + Getter&& getter_, + Setter&& setter_, + const Extras&... extras) { if constexpr(std::is_same_v, std::nullptr_t>) { cpp_function getter(true, name, diff --git a/include/pybind11/internal/object.h b/include/pybind11/internal/object.h index 86da0103..db90fcd9 100644 --- a/include/pybind11/internal/object.h +++ b/include/pybind11/internal/object.h @@ -120,7 +120,7 @@ public: name(const char* data, int size) : data(py_namev({data, size})) {} - name(std::string_view str) : name(str.data(), str.size()) {} + name(std::string_view str) : name(str.data(), static_cast(str.size())) {} name(handle h); @@ -198,9 +198,7 @@ public: object() = default; object(const object& other) : handle(other), m_index(other.m_index) { - if(other.in_pool()) { - object_pool::inc_ref(other); - } + if(other.in_pool()) { object_pool::inc_ref(other); } } object(object&& other) : handle(other), m_index(other.m_index) { @@ -210,12 +208,8 @@ public: object& operator= (const object& other) { if(this != &other) { - if(in_pool()) { - object_pool::dec_ref(*this); - } - if(other.in_pool()) { - object_pool::inc_ref(other); - } + if(in_pool()) { object_pool::dec_ref(*this); } + if(other.in_pool()) { object_pool::inc_ref(other); } m_ptr = other.m_ptr; m_index = other.m_index; } @@ -224,9 +218,7 @@ public: object& operator= (object&& other) { if(this != &other) { - if(in_pool()) { - object_pool::dec_ref(*this); - } + if(in_pool()) { object_pool::dec_ref(*this); } m_ptr = other.m_ptr; m_index = other.m_index; other.m_ptr = nullptr; @@ -236,9 +228,7 @@ public: } ~object() { - if(in_pool()) { - object_pool::dec_ref(*this); - } + if(in_pool()) { object_pool::dec_ref(*this); } } bool is_singleton() const { return m_ptr && m_index == -1; } diff --git a/include/pybind11/internal/types.h b/include/pybind11/internal/types.h index ad6cbec2..70896877 100644 --- a/include/pybind11/internal/types.h +++ b/include/pybind11/internal/types.h @@ -4,22 +4,24 @@ namespace pkbind { -#define PKBIND_TYPE_IMPL(parent, child, expr) \ - \ -private: \ - friend class type; \ - static auto type_or_check() { return expr; } \ - \ -public: \ - using parent::parent; \ - using parent::operator=; \ - child(const object& o) : parent(type::isinstance(o) ? o : type::of()(o)) {} \ - child(object&& o) : parent(type::isinstance(o) ? std::move(o) : type::of()(std::move(o))) {} +#define PKBIND_TYPE_IMPL(parent, child, expr) \ + \ +private: \ + friend class type; \ + static auto type_or_check() { return expr; } \ + \ +public: \ + using parent::parent; \ + using parent::operator=; \ + child(const object& o) : parent(type::isinstance(o) ? o : type::of()(o)) {} \ + child(object&& o) : \ + parent(type::isinstance(o) ? std::move(o) : type::of()(std::move(o))) {} class type : public object { protected: template - constexpr inline static bool is_check_v = std::is_invocable_r_v; + constexpr inline static bool is_check_v = + std::is_invocable_r_v; static auto type_or_check() { return tp_type; } @@ -108,7 +110,9 @@ public: object operator* () const { return m_value; } - friend bool operator== (const iterator& lhs, const iterator& rhs) { return lhs.m_value.ptr() == rhs.m_value.ptr(); } + friend bool operator== (const iterator& lhs, const iterator& rhs) { + return lhs.m_value.ptr() == rhs.m_value.ptr(); + } friend bool operator!= (const iterator& lhs, const iterator& rhs) { return !(lhs == rhs); } @@ -134,7 +138,7 @@ class str : public object { str(const char* data, int size) : object(alloc_t{}) { py_newstrn(m_ptr, data, size); } - str(const char* data) : str(data, strlen(data)) {} + str(const char* data) : str(data, static_cast(strlen(data))) {} str(std::string_view s) : str(s.data(), static_cast(s.size())) {} @@ -147,7 +151,7 @@ class tuple : public object { tuple(int size) : object(alloc_t{}) { py_newtuple(m_ptr, size); } - tuple(std::initializer_list args) : tuple(args.size()) { + tuple(std::initializer_list args) : tuple(static_cast(args.size())) { int index = 0; for(auto& arg: args) { py_tuple_setitem(m_ptr, index++, arg.ptr()); @@ -194,7 +198,7 @@ class list : public object { list(int size) : object(alloc_t{}) { py_newlistn(m_ptr, size); } - list(std::initializer_list args) : list(args.size()) { + list(std::initializer_list args) : list(static_cast(args.size())) { int index = 0; for(auto& arg: args) { py_list_setitem(m_ptr, index++, arg.ptr()); @@ -338,9 +342,7 @@ class capsule : public object { static void register_() { m_type = py_newtype("capsule", tp_object, nullptr, [](void* data) { auto impl = static_cast(data); - if(impl->data && impl->destructor) { - impl->destructor(impl->data); - } + if(impl->data && impl->destructor) { impl->destructor(impl->data); } }); } diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index f629e2ca..d8bb7481 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -30,7 +30,7 @@ struct type_caster> { if(list.size() != N) { return false; } - for(std::size_t i = 0; i < N; ++i) { + for(int i = 0; i < N; ++i) { type_caster caster; if(!caster.load(list[i], convert)) { return false; } data[i] = std::move(caster.value()); @@ -139,7 +139,8 @@ struct type_caster>> { static object cast(U&& src, return_value_policy policy, handle parent) { auto dict = pkbind::dict(); for(auto&& [key, value]: src) { - dict[pkbind::cast(std::move(key), policy, parent)] = pkbind::cast(std::move(value), policy, parent); + dict[pkbind::cast(std::move(key), policy, parent)] = + pkbind::cast(std::move(value), policy, parent); } return dict; } @@ -167,4 +168,3 @@ struct type_caster>> { }; } // namespace pkbind - diff --git a/include/pybind11/tests/function.cpp b/include/pybind11/tests/function.cpp index 7e2e0146..59f1c023 100644 --- a/include/pybind11/tests/function.cpp +++ b/include/pybind11/tests/function.cpp @@ -351,7 +351,7 @@ TEST_F(PYBIND11_TEST, lambda) { size_t c; size_t d; - int operator() (int x, int y) { return x + y + a + b + c + d; } + size_t operator() (size_t x, size_t y) { return x + y + a + b + c + d; } ~NotSmall() { destructor_calls++; } };