From 6abf2baa62063a39ccc4a9535351de571004ddd4 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 8 Sep 2021 18:53:38 -0700 Subject: [PATCH] CodeHealth: Enabling clang-tidy google-explicit-constructor (#3250) * Adding google-explicit-constructor to .clang-tidy * clang-tidy explicit attr.h (all automatic) * clang-tidy explicit cast.h (all automatic) * clang-tidy detail/init.h (1 NOLINT) * clang-tidy detail/type_caster_base.h (2 NOLINT) * clang-tidy pybind11.h (7 NOLINT) * clang-tidy detail/common.h (3 NOLINT) * clang-tidy detail/descr.h (2 NOLINT) * clang-tidy pytypes.h (23 NOLINT, only 1 explicit) * clang-tidy eigen.h (7 NOLINT, 0 explicit) * Adding 2 explicit in functional.h * Adding 4 explicit in iostream.h * clang-tidy numpy.h (1 NOLINT, 1 explicit) * clang-tidy embed.h (0 NOLINT, 1 explicit) * clang-tidy tests/local_bindings.h (0 NOLINT, 4 explicit) * clang-tidy tests/pybind11_cross_module_tests.cpp (0 NOLINT, 1 explicit) * clang-tidy tests/pybind11_tests.h (0 NOLINT, 2 explicit) * clang-tidy tests/test_buffers.cpp (0 NOLINT, 2 explicit) * clang-tidy tests/test_builtin_casters.cpp (0 NOLINT, 4 explicit) * clang-tidy tests/test_class.cpp (0 NOLINT, 6 explicit) * clang-tidy tests/test_copy_move.cpp (0 NOLINT, 7 explicit) * clang-tidy tests/test_embed/external_module.cpp (0 NOLINT, 1 explicit) * clang-tidy tests/test_embed/test_interpreter.cpp (0 NOLINT, 1 explicit) * clang-tidy tests/object.h (0 NOLINT, 2 explicit) * clang-tidy batch of fully automatic fixes. * Workaround for MSVC 19.16.27045.0 C++17 Python 2 C++ syntax error. --- .clang-tidy | 1 + include/pybind11/attr.h | 24 ++++++++--- include/pybind11/cast.h | 14 ++++--- include/pybind11/detail/common.h | 3 ++ include/pybind11/detail/descr.h | 2 + include/pybind11/detail/init.h | 5 ++- include/pybind11/detail/type_caster_base.h | 18 ++++---- include/pybind11/eigen.h | 7 ++++ include/pybind11/embed.h | 8 ++-- include/pybind11/functional.h | 6 ++- include/pybind11/iostream.h | 14 ++++--- include/pybind11/numpy.h | 3 +- include/pybind11/pybind11.h | 9 +++- include/pybind11/pytypes.h | 25 ++++++++++- tests/local_bindings.h | 8 ++-- tests/object.h | 4 +- tests/pybind11_cross_module_tests.cpp | 2 +- tests/pybind11_tests.h | 4 +- tests/test_buffers.cpp | 4 +- tests/test_builtin_casters.cpp | 22 +++++++--- tests/test_class.cpp | 12 +++--- tests/test_copy_move.cpp | 14 +++---- tests/test_embed/external_module.cpp | 2 +- tests/test_embed/test_interpreter.cpp | 2 +- tests/test_exceptions.cpp | 4 +- tests/test_factory_constructors.cpp | 49 ++++++++++++++-------- tests/test_local_bindings.cpp | 2 +- tests/test_methods_and_attributes.cpp | 8 ++-- tests/test_modules.cpp | 2 +- tests/test_multiple_inheritance.cpp | 10 ++--- tests/test_numpy_vectorize.cpp | 4 +- tests/test_pickling.cpp | 4 +- tests/test_sequences_and_iterators.cpp | 16 +++---- tests/test_smart_ptr.cpp | 32 +++++++------- tests/test_stl.cpp | 3 +- tests/test_stl_binders.cpp | 2 +- tests/test_tagbased_polymorphic.cpp | 12 +++--- tests/test_virtual_functions.cpp | 2 +- 38 files changed, 231 insertions(+), 132 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index cefffba1ea..a6437dd494 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -5,6 +5,7 @@ Checks: ' cppcoreguidelines-init-variables, cppcoreguidelines-slicing, clang-analyzer-optin.cplusplus.VirtualCall, +google-explicit-constructor, llvm-namespace-comment, misc-misplaced-const, misc-non-copyable-objects, diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index ab1fe80446..13f68bbe8f 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -18,7 +18,9 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) /// @{ /// Annotation for methods -struct is_method { handle class_; is_method(const handle &c) : class_(c) { } }; +struct is_method { handle class_; + explicit is_method(const handle &c) : class_(c) {} +}; /// Annotation for operators struct is_operator { }; @@ -27,16 +29,24 @@ struct is_operator { }; struct is_final { }; /// Annotation for parent scope -struct scope { handle value; scope(const handle &s) : value(s) { } }; +struct scope { handle value; + explicit scope(const handle &s) : value(s) {} +}; /// Annotation for documentation -struct doc { const char *value; doc(const char *value) : value(value) { } }; +struct doc { const char *value; + explicit doc(const char *value) : value(value) {} +}; /// Annotation for function names -struct name { const char *value; name(const char *value) : value(value) { } }; +struct name { const char *value; + explicit name(const char *value) : value(value) {} +}; /// Annotation indicating that a function is an overload associated with a given "sibling" -struct sibling { handle value; sibling(const handle &value) : value(value.ptr()) { } }; +struct sibling { handle value; + explicit sibling(const handle &value) : value(value.ptr()) {} +}; /// Annotation indicating that a class derives from another given type template struct base { @@ -70,7 +80,9 @@ struct metaclass { }; /// Annotation that marks a class as local to the module: -struct module_local { const bool value; constexpr module_local(bool v = true) : value(v) { } }; +struct module_local { const bool value; + constexpr explicit module_local(bool v = true) : value(v) {} +}; /// Annotation to mark enums as an arithmetic type struct arithmetic { }; diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 79bf506d81..db79f57cda 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -82,7 +82,7 @@ template class type_caster> { return caster_t::cast(&src.get(), policy, parent); } template using cast_op_type = std::reference_wrapper; - operator std::reference_wrapper() { return cast_op(subcaster); } + explicit operator std::reference_wrapper() { return cast_op(subcaster); } }; #define PYBIND11_TYPE_CASTER(type, py_name) \ @@ -279,7 +279,7 @@ template <> class type_caster : public type_caster { } template using cast_op_type = void*&; - operator void *&() { return value; } + explicit operator void *&() { return value; } static constexpr auto name = _("capsule"); private: void *value = nullptr; @@ -487,8 +487,10 @@ template struct type_caster(static_cast(str_caster).c_str()); } - operator CharT&() { + explicit operator CharT *() { + return none ? nullptr : const_cast(static_cast(str_caster).c_str()); + } + explicit operator CharT &() { if (none) throw value_error("Cannot convert None to a character"); @@ -581,8 +583,8 @@ template class Tuple, typename... Ts> class tuple_caster template using cast_op_type = type; - operator type() & { return implicit_cast(indices{}); } - operator type() && { return std::move(*this).implicit_cast(indices{}); } + explicit operator type() & { return implicit_cast(indices{}); } + explicit operator type() && { return std::move(*this).implicit_cast(indices{}); } protected: template diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index e4f00293ec..a8fd7d917e 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -918,6 +918,7 @@ class any_container { // Implicit conversion constructor from any arbitrary container type with values convertible to T template ())), T>::value>> + // NOLINTNEXTLINE(google-explicit-constructor) any_container(const Container &c) : any_container(std::begin(c), std::end(c)) { } // initializer_list's aren't deducible, so don't get matched by the above template; we need this @@ -926,9 +927,11 @@ class any_container { any_container(const std::initializer_list &c) : any_container(c.begin(), c.end()) { } // Avoid copying if given an rvalue vector of the correct type. + // NOLINTNEXTLINE(google-explicit-constructor) any_container(std::vector &&v) : v(std::move(v)) { } // Moves the vector out of an rvalue any_container + // NOLINTNEXTLINE(google-explicit-constructor) operator std::vector &&() && { return std::move(v); } // Dereferencing obtains a reference to the underlying vector diff --git a/include/pybind11/detail/descr.h b/include/pybind11/detail/descr.h index 0acfc7db37..0b498e5e72 100644 --- a/include/pybind11/detail/descr.h +++ b/include/pybind11/detail/descr.h @@ -26,12 +26,14 @@ struct descr { char text[N + 1]{'\0'}; constexpr descr() = default; + // NOLINTNEXTLINE(google-explicit-constructor) constexpr descr(char const (&s)[N+1]) : descr(s, make_index_sequence()) { } template constexpr descr(char const (&s)[N+1], index_sequence) : text{s[Is]..., '\0'} { } template + // NOLINTNEXTLINE(google-explicit-constructor) constexpr descr(char c, Chars... cs) : text{c, static_cast(cs)..., '\0'} { } static constexpr std::array types() { diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index e795da7d75..cace352964 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -23,7 +23,7 @@ class type_caster { } template using cast_op_type = value_and_holder &; - operator value_and_holder &() { return *value; } + explicit operator value_and_holder &() { return *value; } static constexpr auto name = _(); private: @@ -222,7 +222,8 @@ template struct factory { remove_reference_t class_factory; - factory(Func &&f) : class_factory(std::forward(f)) { } + // NOLINTNEXTLINE(google-explicit-constructor) + factory(Func &&f) : class_factory(std::forward(f)) {} // The given class either has no alias or has no separate alias factory; // this always constructs the class itself. If the class is registered with an alias diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 5a5acc2e6a..4c9ac7b8f6 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -225,7 +225,7 @@ struct value_and_holder { value_and_holder() = default; // Used for past-the-end iterator - value_and_holder(size_t index) : index{index} {} + explicit value_and_holder(size_t index) : index{index} {} template V *&value_ptr() const { return reinterpret_cast(vh[0]); @@ -274,7 +274,8 @@ struct values_and_holders { const type_vec &tinfo; public: - values_and_holders(instance *inst) : inst{inst}, tinfo(all_type_info(Py_TYPE(inst))) {} + explicit values_and_holders(instance *inst) + : inst{inst}, tinfo(all_type_info(Py_TYPE(inst))) {} struct iterator { private: @@ -290,7 +291,8 @@ struct values_and_holders { 0 /* index */) {} // Past-the-end iterator: - iterator(size_t end) : curr(end) {} + explicit iterator(size_t end) : curr(end) {} + public: bool operator==(const iterator &other) const { return curr.index == other.curr.index; } bool operator!=(const iterator &other) const { return curr.index != other.curr.index; } @@ -491,11 +493,11 @@ inline PyObject *make_new_instance(PyTypeObject *type); class type_caster_generic { public: - PYBIND11_NOINLINE type_caster_generic(const std::type_info &type_info) - : typeinfo(get_type_info(type_info)), cpptype(&type_info) { } + PYBIND11_NOINLINE explicit type_caster_generic(const std::type_info &type_info) + : typeinfo(get_type_info(type_info)), cpptype(&type_info) {} - type_caster_generic(const type_info *typeinfo) - : typeinfo(typeinfo), cpptype(typeinfo ? typeinfo->cpptype : nullptr) { } + explicit type_caster_generic(const type_info *typeinfo) + : typeinfo(typeinfo), cpptype(typeinfo ? typeinfo->cpptype : nullptr) {} bool load(handle src, bool convert) { return load_impl(src, convert); @@ -923,7 +925,9 @@ template class type_caster_base : public type_caster_generic { template using cast_op_type = detail::cast_op_type; + // NOLINTNEXTLINE(google-explicit-constructor) operator itype*() { return (type *) value; } + // NOLINTNEXTLINE(google-explicit-constructor) operator itype&() { if (!value) throw reference_cast_error(); return *((itype *) value); } protected: diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index b03e15f624..c0363827c3 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -61,6 +61,7 @@ template struct EigenConformable { EigenDStride stride{0, 0}; // Only valid if negativestrides is false! bool negativestrides = false; // If true, do not use stride! + // NOLINTNEXTLINE(google-explicit-constructor) EigenConformable(bool fits = false) : conformable{fits} {} // Matrix type: EigenConformable(EigenIndex r, EigenIndex c, @@ -88,6 +89,7 @@ template struct EigenConformable { (props::outer_stride == Eigen::Dynamic || props::outer_stride == stride.outer() || (EigenRowMajor ? rows : cols) == 1); } + // NOLINTNEXTLINE(google-explicit-constructor) operator bool() const { return conformable; } }; @@ -326,8 +328,11 @@ struct type_caster::value>> { static constexpr auto name = props::descriptor; + // NOLINTNEXTLINE(google-explicit-constructor) operator Type*() { return &value; } + // NOLINTNEXTLINE(google-explicit-constructor) operator Type&() { return value; } + // NOLINTNEXTLINE(google-explicit-constructor) operator Type&&() && { return std::move(value); } template using cast_op_type = movable_cast_op_type; @@ -451,7 +456,9 @@ struct type_caster< return true; } + // NOLINTNEXTLINE(google-explicit-constructor) operator Type*() { return ref.get(); } + // NOLINTNEXTLINE(google-explicit-constructor) operator Type&() { return *ref; } template using cast_op_type = pybind11::detail::cast_op_type<_T>; diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index 7b5d7cd24a..9843f0f973 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -260,10 +260,10 @@ inline void finalize_interpreter() { \endrst */ class scoped_interpreter { public: - scoped_interpreter(bool init_signal_handlers = true, - int argc = 0, - const char *const *argv = nullptr, - bool add_program_dir_to_path = true) { + explicit scoped_interpreter(bool init_signal_handlers = true, + int argc = 0, + const char *const *argv = nullptr, + bool add_program_dir_to_path = true) { initialize_interpreter(init_signal_handlers, argc, argv, add_program_dir_to_path); } diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index bc8a8af821..24141ce388 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -69,6 +69,10 @@ struct type_caster> { // ensure GIL is held during functor destruction struct func_handle { function f; +#if !(defined(_MSC_VER) && _MSC_VER == 1916 && defined(PYBIND11_CPP17) && PY_MAJOR_VERSION < 3) + // This triggers a syntax error under very special conditions (very weird indeed). + explicit +#endif func_handle(function &&f_) noexcept : f(std::move(f_)) {} func_handle(const func_handle &f_) { operator=(f_); } func_handle &operator=(const func_handle &f_) { @@ -85,7 +89,7 @@ struct type_caster> { // to emulate 'move initialization capture' in C++11 struct func_wrapper { func_handle hfunc; - func_wrapper(func_handle &&hf) noexcept : hfunc(std::move(hf)) {} + explicit func_wrapper(func_handle &&hf) noexcept : hfunc(std::move(hf)) {} Return operator()(Args... args) const { gil_scoped_acquire acq; object retval(hfunc.f(std::forward(args)...)); diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h index e4d2095857..95449a07ba 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -123,7 +123,7 @@ class pythonbuf : public std::streambuf { } public: - pythonbuf(const object &pyostream, size_t buffer_size = 1024) + explicit pythonbuf(const object &pyostream, size_t buffer_size = 1024) : buf_size(buffer_size), d_buffer(new char[buf_size]), pywrite(pyostream.attr("write")), pyflush(pyostream.attr("flush")) { setp(d_buffer.get(), d_buffer.get() + buf_size - 1); @@ -171,8 +171,9 @@ class scoped_ostream_redirect { detail::pythonbuf buffer; public: - scoped_ostream_redirect(std::ostream &costream = std::cout, - const object &pyostream = module_::import("sys").attr("stdout")) + explicit scoped_ostream_redirect(std::ostream &costream = std::cout, + const object &pyostream + = module_::import("sys").attr("stdout")) : costream(costream), buffer(pyostream) { old = costream.rdbuf(&buffer); } @@ -201,8 +202,9 @@ class scoped_ostream_redirect { \endrst */ class scoped_estream_redirect : public scoped_ostream_redirect { public: - scoped_estream_redirect(std::ostream &costream = std::cerr, - const object &pyostream = module_::import("sys").attr("stderr")) + explicit scoped_estream_redirect(std::ostream &costream = std::cerr, + const object &pyostream + = module_::import("sys").attr("stderr")) : scoped_ostream_redirect(costream, pyostream) {} }; @@ -217,7 +219,7 @@ class OstreamRedirect { std::unique_ptr redirect_stderr; public: - OstreamRedirect(bool do_stdout = true, bool do_stderr = true) + explicit OstreamRedirect(bool do_stdout = true, bool do_stderr = true) : do_stdout_(do_stdout), do_stderr_(do_stderr) {} void enter() { diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index fa128efdd0..d55844086d 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -477,7 +477,7 @@ class dtype : public object { m_ptr = from_args(pybind11::str(format)).release().ptr(); } - dtype(const char *format) : dtype(std::string(format)) { } + explicit dtype(const char *format) : dtype(std::string(format)) {} dtype(list names, list formats, list offsets, ssize_t itemsize) { dict args; @@ -894,6 +894,7 @@ template class array_t : public if (!is_borrowed) Py_XDECREF(h.ptr()); } + // NOLINTNEXTLINE(google-explicit-constructor) array_t(const object &o) : array(raw_array_t(o.ptr()), stolen_t{}) { if (!m_ptr) throw error_already_set(); } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 990c213049..82d339751e 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -81,10 +81,12 @@ PYBIND11_NAMESPACE_END(detail) class cpp_function : public function { public: cpp_function() = default; + // NOLINTNEXTLINE(google-explicit-constructor) cpp_function(std::nullptr_t) { } /// Construct a cpp_function from a vanilla function pointer template + // NOLINTNEXTLINE(google-explicit-constructor) cpp_function(Return (*f)(Args...), const Extra&... extra) { initialize(f, f, extra...); } @@ -92,6 +94,7 @@ class cpp_function : public function { /// Construct a cpp_function from a lambda function (possibly with internal state) template ::value>> + // NOLINTNEXTLINE(google-explicit-constructor) cpp_function(Func &&f, const Extra&... extra) { initialize(std::forward(f), (detail::function_signature_t *) nullptr, extra...); @@ -99,6 +102,7 @@ class cpp_function : public function { /// Construct a cpp_function from a class method (non-const, no ref-qualifier) template + // NOLINTNEXTLINE(google-explicit-constructor) cpp_function(Return (Class::*f)(Arg...), const Extra&... extra) { initialize([f](Class *c, Arg... args) -> Return { return (c->*f)(std::forward(args)...); }, (Return (*) (Class *, Arg...)) nullptr, extra...); @@ -108,6 +112,7 @@ class cpp_function : public function { /// A copy of the overload for non-const functions without explicit ref-qualifier /// but with an added `&`. template + // NOLINTNEXTLINE(google-explicit-constructor) cpp_function(Return (Class::*f)(Arg...)&, const Extra&... extra) { initialize([f](Class *c, Arg... args) -> Return { return (c->*f)(args...); }, (Return (*) (Class *, Arg...)) nullptr, extra...); @@ -115,6 +120,7 @@ class cpp_function : public function { /// Construct a cpp_function from a class method (const, no ref-qualifier) template + // NOLINTNEXTLINE(google-explicit-constructor) cpp_function(Return (Class::*f)(Arg...) const, const Extra&... extra) { initialize([f](const Class *c, Arg... args) -> Return { return (c->*f)(std::forward(args)...); }, (Return (*)(const Class *, Arg ...)) nullptr, extra...); @@ -124,6 +130,7 @@ class cpp_function : public function { /// A copy of the overload for const functions without explicit ref-qualifier /// but with an added `&`. template + // NOLINTNEXTLINE(google-explicit-constructor) cpp_function(Return (Class::*f)(Arg...) const&, const Extra&... extra) { initialize([f](const Class *c, Arg... args) -> Return { return (c->*f)(args...); }, (Return (*)(const Class *, Arg ...)) nullptr, extra...); @@ -2034,7 +2041,7 @@ template void implicitly_convertible() { struct set_flag { bool &flag; - set_flag(bool &flag_) : flag(flag_) { flag_ = true; } + explicit set_flag(bool &flag_) : flag(flag_) { flag_ = true; } ~set_flag() { flag = false; } }; auto implicit_caster = [](PyObject *obj, PyTypeObject *type) -> PyObject * { diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index fa50eb1e3a..971db85f37 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -178,6 +178,7 @@ class handle : public detail::object_api { /// The default constructor creates a handle with a ``nullptr``-valued pointer handle() = default; /// Creates a ``handle`` from the given raw Python object pointer + // NOLINTNEXTLINE(google-explicit-constructor) handle(PyObject *ptr) : m_ptr(ptr) { } // Allow implicit conversion from PyObject* /// Return the underlying ``PyObject *`` pointer @@ -612,6 +613,7 @@ class accessor : public object_api> { return obj.contains(key); } + // NOLINTNEXTLINE(google-explicit-constructor) operator object() const { return get_cache(); } PyObject *ptr() const { return get_cache().ptr(); } template T cast() const { return get_cache().template cast(); } @@ -761,6 +763,7 @@ template struct arrow_proxy { T value; + // NOLINTNEXTLINE(google-explicit-constructor) arrow_proxy(T &&value) noexcept : value(std::move(value)) { } T *operator->() const { return &value; } }; @@ -909,14 +912,17 @@ PYBIND11_NAMESPACE_END(detail) bool check() const { return m_ptr != nullptr && (CheckFun(m_ptr) != 0); } \ static bool check_(handle h) { return h.ptr() != nullptr && CheckFun(h.ptr()); } \ template \ + /* NOLINTNEXTLINE(google-explicit-constructor) */ \ Name(const ::pybind11::detail::accessor &a) : Name(object(a)) { } #define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ConvertFun) \ PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \ + /* NOLINTNEXTLINE(google-explicit-constructor) */ \ Name(const object &o) \ : Parent(check_(o) ? o.inc_ref().ptr() : ConvertFun(o.ptr()), stolen_t{}) \ { if (!m_ptr) throw error_already_set(); } \ + /* NOLINTNEXTLINE(google-explicit-constructor) */ \ Name(object &&o) \ : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \ { if (!m_ptr) throw error_already_set(); } @@ -933,8 +939,10 @@ PYBIND11_NAMESPACE_END(detail) #define PYBIND11_OBJECT(Name, Parent, CheckFun) \ PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \ /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \ + /* NOLINTNEXTLINE(google-explicit-constructor) */ \ Name(const object &o) : Parent(o) \ { if (m_ptr && !check_(m_ptr)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, m_ptr); } \ + /* NOLINTNEXTLINE(google-explicit-constructor) */ \ Name(object &&o) : Parent(std::move(o)) \ { if (m_ptr && !check_(m_ptr)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, m_ptr); } @@ -1056,11 +1064,13 @@ class str : public object { } // 'explicit' is explicitly omitted from the following constructors to allow implicit conversion to py::str from C++ string-like objects + // NOLINTNEXTLINE(google-explicit-constructor) str(const char *c = "") : object(PyUnicode_FromString(c), stolen_t{}) { if (!m_ptr) pybind11_fail("Could not allocate string object!"); } + // NOLINTNEXTLINE(google-explicit-constructor) str(const std::string &s) : str(s.data(), s.size()) { } explicit str(const bytes &b); @@ -1071,6 +1081,7 @@ class str : public object { \endrst */ explicit str(handle h) : object(raw_str(h.ptr()), stolen_t{}) { if (!m_ptr) throw error_already_set(); } + // NOLINTNEXTLINE(google-explicit-constructor) operator std::string() const { object temp = *this; if (PyUnicode_Check(m_ptr)) { @@ -1118,6 +1129,7 @@ class bytes : public object { PYBIND11_OBJECT(bytes, object, PYBIND11_BYTES_CHECK) // Allow implicit conversion: + // NOLINTNEXTLINE(google-explicit-constructor) bytes(const char *c = "") : object(PYBIND11_BYTES_FROM_STRING(c), stolen_t{}) { if (!m_ptr) pybind11_fail("Could not allocate bytes object!"); @@ -1130,10 +1142,12 @@ class bytes : public object { } // Allow implicit conversion: + // NOLINTNEXTLINE(google-explicit-constructor) bytes(const std::string &s) : bytes(s.data(), s.size()) { } explicit bytes(const pybind11::str &s); + // NOLINTNEXTLINE(google-explicit-constructor) operator std::string() const { char *buffer = nullptr; ssize_t length = 0; @@ -1222,7 +1236,9 @@ class bool_ : public object { PYBIND11_OBJECT_CVT(bool_, object, PyBool_Check, raw_bool) bool_() : object(Py_False, borrowed_t{}) { } // Allow implicit conversion from and to `bool`: + // NOLINTNEXTLINE(google-explicit-constructor) bool_(bool value) : object(value ? Py_True : Py_False, borrowed_t{}) { } + // NOLINTNEXTLINE(google-explicit-constructor) operator bool() const { return (m_ptr != nullptr) && PyLong_AsLong(m_ptr) != 0; } private: @@ -1261,6 +1277,7 @@ class int_ : public object { // Allow implicit conversion from C++ integral types: template ::value, int> = 0> + // NOLINTNEXTLINE(google-explicit-constructor) int_(T value) { if (PYBIND11_SILENCE_MSVC_C4127(sizeof(T) <= sizeof(long))) { if (std::is_signed::value) @@ -1278,6 +1295,7 @@ class int_ : public object { template ::value, int> = 0> + // NOLINTNEXTLINE(google-explicit-constructor) operator T() const { return std::is_unsigned::value ? detail::as_unsigned(m_ptr) @@ -1291,13 +1309,17 @@ class float_ : public object { public: PYBIND11_OBJECT_CVT(float_, object, PyFloat_Check, PyNumber_Float) // Allow implicit conversion from float/double: + // NOLINTNEXTLINE(google-explicit-constructor) float_(float value) : object(PyFloat_FromDouble((double) value), stolen_t{}) { if (!m_ptr) pybind11_fail("Could not allocate float object!"); } + // NOLINTNEXTLINE(google-explicit-constructor) float_(double value = .0) : object(PyFloat_FromDouble((double) value), stolen_t{}) { if (!m_ptr) pybind11_fail("Could not allocate float object!"); } + // NOLINTNEXTLINE(google-explicit-constructor) operator float() const { return (float) PyFloat_AsDouble(m_ptr); } + // NOLINTNEXTLINE(google-explicit-constructor) operator double() const { return (double) PyFloat_AsDouble(m_ptr); } }; @@ -1372,7 +1394,7 @@ class capsule : public object { pybind11_fail("Could not set capsule context!"); } - capsule(void (*destructor)()) { + explicit capsule(void (*destructor)()) { m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, nullptr)); destructor(); @@ -1382,6 +1404,7 @@ class capsule : public object { pybind11_fail("Could not allocate capsule object!"); } + // NOLINTNEXTLINE(google-explicit-constructor) template operator T *() const { return get_pointer(); } diff --git a/tests/local_bindings.h b/tests/local_bindings.h index f11c08e616..4c936c19a5 100644 --- a/tests/local_bindings.h +++ b/tests/local_bindings.h @@ -6,7 +6,7 @@ /// Simple class used to test py::local: template class LocalBase { public: - LocalBase(int i) : i(i) { } + explicit LocalBase(int i) : i(i) { } int i = -1; }; @@ -75,11 +75,11 @@ py::class_ bind_local(Args && ...args) { namespace pets { class Pet { public: - Pet(std::string name) : name_(std::move(name)) {} + explicit Pet(std::string name) : name_(std::move(name)) {} std::string name_; const std::string &name() const { return name_; } }; } // namespace pets -struct MixGL { int i; MixGL(int i) : i{i} {} }; -struct MixGL2 { int i; MixGL2(int i) : i{i} {} }; +struct MixGL { int i; explicit MixGL(int i) : i{i} {} }; +struct MixGL2 { int i; explicit MixGL2(int i) : i{i} {} }; diff --git a/tests/object.h b/tests/object.h index 6851fc6247..be21bf6316 100644 --- a/tests/object.h +++ b/tests/object.h @@ -65,7 +65,7 @@ template class ref { ref() : m_ptr(nullptr) { print_default_created(this); track_default_created((ref_tag*) this); } /// Construct a reference from a pointer - ref(T *ptr) : m_ptr(ptr) { + explicit ref(T *ptr) : m_ptr(ptr) { if (m_ptr) ((Object *) m_ptr)->incRef(); print_created(this, "from pointer", m_ptr); track_created((ref_tag*) this, "from pointer"); @@ -165,7 +165,7 @@ template class ref { const T& operator*() const { return *m_ptr; } /// Return a pointer to the referenced object - operator T* () { return m_ptr; } + explicit operator T* () { return m_ptr; } /// Return a const pointer to the referenced object T* get_ptr() { return m_ptr; } diff --git a/tests/pybind11_cross_module_tests.cpp b/tests/pybind11_cross_module_tests.cpp index 4bfd5302d5..5838cb2746 100644 --- a/tests/pybind11_cross_module_tests.cpp +++ b/tests/pybind11_cross_module_tests.cpp @@ -123,7 +123,7 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { class Dog : public pets::Pet { public: - Dog(std::string name) : Pet(std::move(name)) {} + explicit Dog(std::string name) : Pet(std::move(name)) {} }; py::class_(m, "Pet", py::module_local()) .def("name", &pets::Pet::name); diff --git a/tests/pybind11_tests.h b/tests/pybind11_tests.h index 8da0a670f2..800ddda48b 100644 --- a/tests/pybind11_tests.h +++ b/tests/pybind11_tests.h @@ -16,7 +16,7 @@ class test_initializer { using Initializer = void (*)(py::module_ &); public: - test_initializer(Initializer init); + explicit test_initializer(Initializer init); test_initializer(const char *submodule_name, Initializer init); }; @@ -32,7 +32,7 @@ struct UnregisteredType { }; class UserType { public: UserType() = default; - UserType(int i) : i(i) { } + explicit UserType(int i) : i(i) { } int value() const { return i; } void set(int set) { i = set; } diff --git a/tests/test_buffers.cpp b/tests/test_buffers.cpp index c7e2c7df30..3a8e3e7b75 100644 --- a/tests/test_buffers.cpp +++ b/tests/test_buffers.cpp @@ -122,7 +122,7 @@ TEST_SUBMODULE(buffers, m) { // test_inherited_protocol class SquareMatrix : public Matrix { public: - SquareMatrix(py::ssize_t n) : Matrix(n, n) { } + explicit SquareMatrix(py::ssize_t n) : Matrix(n, n) {} }; // Derived classes inherit the buffer protocol and the buffer access function py::class_(m, "SquareMatrix") @@ -173,7 +173,7 @@ TEST_SUBMODULE(buffers, m) { struct BufferReadOnly { const uint8_t value = 0; - BufferReadOnly(uint8_t value): value(value) {} + explicit BufferReadOnly(uint8_t value) : value(value) {} py::buffer_info get_buffer_info() { return py::buffer_info(&value, 1); diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index be4431aad6..71c778e8e2 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -25,16 +25,28 @@ class type_caster { // cast operator. bool load(handle, bool) { return true; } - operator ConstRefCasted &&() { + explicit operator ConstRefCasted &&() { value = {1}; // NOLINTNEXTLINE(performance-move-const-arg) return std::move(value); } - operator ConstRefCasted&() { value = {2}; return value; } - operator ConstRefCasted*() { value = {3}; return &value; } + explicit operator ConstRefCasted &() { + value = {2}; + return value; + } + explicit operator ConstRefCasted *() { + value = {3}; + return &value; + } - operator const ConstRefCasted&() { value = {4}; return value; } - operator const ConstRefCasted*() { value = {5}; return &value; } + explicit operator const ConstRefCasted &() { + value = {4}; + return value; + } + explicit operator const ConstRefCasted *() { + value = {5}; + return &value; + } // custom cast_op to explicitly propagate types to the conversion operators. template diff --git a/tests/test_class.cpp b/tests/test_class.cpp index dff758392d..246b6c1fea 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -27,7 +27,7 @@ // test_brace_initialization struct NoBraceInitialization { - NoBraceInitialization(std::vector v) : vec{std::move(v)} {} + explicit NoBraceInitialization(std::vector v) : vec{std::move(v)} {} template NoBraceInitialization(std::initializer_list l) : vec(l) {} @@ -65,18 +65,18 @@ TEST_SUBMODULE(class_, m) { class Dog : public Pet { public: - Dog(const std::string &name) : Pet(name, "dog") {} + explicit Dog(const std::string &name) : Pet(name, "dog") {} std::string bark() const { return "Woof!"; } }; class Rabbit : public Pet { public: - Rabbit(const std::string &name) : Pet(name, "parrot") {} + explicit Rabbit(const std::string &name) : Pet(name, "parrot") {} }; class Hamster : public Pet { public: - Hamster(const std::string &name) : Pet(name, "rodent") {} + explicit Hamster(const std::string &name) : Pet(name, "rodent") {} }; class Chimera : public Pet { @@ -208,7 +208,7 @@ TEST_SUBMODULE(class_, m) { struct ConvertibleFromUserType { int i; - ConvertibleFromUserType(UserType u) : i(u.value()) { } + explicit ConvertibleFromUserType(UserType u) : i(u.value()) {} }; py::class_(m, "AcceptsUserType") @@ -263,7 +263,7 @@ TEST_SUBMODULE(class_, m) { }; struct PyAliasedHasOpNewDelSize : AliasedHasOpNewDelSize { PyAliasedHasOpNewDelSize() = default; - PyAliasedHasOpNewDelSize(int) { } + explicit PyAliasedHasOpNewDelSize(int) {} std::uint64_t j; }; struct HasOpNewDelBoth { diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index 859da9df7e..5fb0dd810c 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -37,7 +37,7 @@ template <> lacking_move_ctor empty::instance_ = {}; class MoveOnlyInt { public: MoveOnlyInt() { print_default_created(this); } - MoveOnlyInt(int v) : value{v} { print_created(this, value); } + explicit MoveOnlyInt(int v) : value{v} { print_created(this, value); } MoveOnlyInt(MoveOnlyInt &&m) noexcept { print_move_created(this, m.value); std::swap(value, m.value); @@ -56,7 +56,7 @@ class MoveOnlyInt { class MoveOrCopyInt { public: MoveOrCopyInt() { print_default_created(this); } - MoveOrCopyInt(int v) : value{v} { print_created(this, value); } + explicit MoveOrCopyInt(int v) : value{v} { print_created(this, value); } MoveOrCopyInt(MoveOrCopyInt &&m) noexcept { print_move_created(this, m.value); std::swap(value, m.value); @@ -75,7 +75,7 @@ class MoveOrCopyInt { class CopyOnlyInt { public: CopyOnlyInt() { print_default_created(this); } - CopyOnlyInt(int v) : value{v} { print_created(this, value); } + explicit CopyOnlyInt(int v) : value{v} { print_created(this, value); } CopyOnlyInt(const CopyOnlyInt &c) { print_copy_created(this, c.value); value = c.value; } CopyOnlyInt &operator=(const CopyOnlyInt &c) { print_copy_assigned(this, c.value); value = c.value; return *this; } ~CopyOnlyInt() { print_destroyed(this); } @@ -107,8 +107,8 @@ template <> struct type_caster { if (!src) return none().release(); return cast(*src, policy, parent); } - operator CopyOnlyInt*() { return &value; } - operator CopyOnlyInt&() { return value; } + explicit operator CopyOnlyInt *() { return &value; } + explicit operator CopyOnlyInt &() { return value; } template using cast_op_type = pybind11::detail::cast_op_type; }; PYBIND11_NAMESPACE_END(detail) @@ -219,7 +219,7 @@ TEST_SUBMODULE(copy_move_policies, m) { // #389: rvp::move should fall-through to copy on non-movable objects struct MoveIssue1 { int v; - MoveIssue1(int v) : v{v} {} + explicit MoveIssue1(int v) : v{v} {} MoveIssue1(const MoveIssue1 &c) = default; MoveIssue1(MoveIssue1 &&) = delete; }; @@ -227,7 +227,7 @@ TEST_SUBMODULE(copy_move_policies, m) { struct MoveIssue2 { int v; - MoveIssue2(int v) : v{v} {} + explicit MoveIssue2(int v) : v{v} {} MoveIssue2(MoveIssue2 &&) = default; }; py::class_(m, "MoveIssue2").def(py::init()).def_readwrite("value", &MoveIssue2::v); diff --git a/tests/test_embed/external_module.cpp b/tests/test_embed/external_module.cpp index e9a6058b17..4909522993 100644 --- a/tests/test_embed/external_module.cpp +++ b/tests/test_embed/external_module.cpp @@ -9,7 +9,7 @@ namespace py = pybind11; PYBIND11_MODULE(external_module, m) { class A { public: - A(int value) : v{value} {}; + explicit A(int value) : v{value} {}; int v; }; diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 78b64be6b0..fae14f7510 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -18,7 +18,7 @@ using namespace py::literals; class Widget { public: - Widget(std::string message) : message(std::move(message)) {} + explicit Widget(std::string message) : message(std::move(message)) {} virtual ~Widget() = default; std::string the_message() const { return message; } diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 4805a02490..25adb32ed1 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -81,7 +81,7 @@ class MyException6 : public std::exception { struct PythonCallInDestructor { - PythonCallInDestructor(const py::dict &d) : d(d) {} + explicit PythonCallInDestructor(const py::dict &d) : d(d) {} ~PythonCallInDestructor() { d["good"] = true; } py::dict d; @@ -90,7 +90,7 @@ struct PythonCallInDestructor { struct PythonAlreadySetInDestructor { - PythonAlreadySetInDestructor(const py::str &s) : s(s) {} + explicit PythonAlreadySetInDestructor(const py::str &s) : s(s) {} ~PythonAlreadySetInDestructor() { py::dict foo; try { diff --git a/tests/test_factory_constructors.cpp b/tests/test_factory_constructors.cpp index 235ec4dca6..660e2896af 100644 --- a/tests/test_factory_constructors.cpp +++ b/tests/test_factory_constructors.cpp @@ -19,8 +19,9 @@ class TestFactory1 { friend class TestFactoryHelper; TestFactory1() : value("(empty)") { print_default_created(this); } - TestFactory1(int v) : value(std::to_string(v)) { print_created(this, value); } - TestFactory1(std::string v) : value(std::move(v)) { print_created(this, value); } + explicit TestFactory1(int v) : value(std::to_string(v)) { print_created(this, value); } + explicit TestFactory1(std::string v) : value(std::move(v)) { print_created(this, value); } + public: std::string value; TestFactory1(TestFactory1 &&) = delete; @@ -33,8 +34,9 @@ class TestFactory1 { class TestFactory2 { friend class TestFactoryHelper; TestFactory2() : value("(empty2)") { print_default_created(this); } - TestFactory2(int v) : value(std::to_string(v)) { print_created(this, value); } - TestFactory2(std::string v) : value(std::move(v)) { print_created(this, value); } + explicit TestFactory2(int v) : value(std::to_string(v)) { print_created(this, value); } + explicit TestFactory2(std::string v) : value(std::move(v)) { print_created(this, value); } + public: TestFactory2(TestFactory2 &&m) noexcept { value = std::move(m.value); @@ -53,9 +55,10 @@ class TestFactory3 { protected: friend class TestFactoryHelper; TestFactory3() : value("(empty3)") { print_default_created(this); } - TestFactory3(int v) : value(std::to_string(v)) { print_created(this, value); } + explicit TestFactory3(int v) : value(std::to_string(v)) { print_created(this, value); } + public: - TestFactory3(std::string v) : value(std::move(v)) { print_created(this, value); } + explicit TestFactory3(std::string v) : value(std::move(v)) { print_created(this, value); } TestFactory3(TestFactory3 &&m) noexcept { value = std::move(m.value); print_move_created(this); @@ -72,13 +75,13 @@ class TestFactory3 { class TestFactory4 : public TestFactory3 { public: TestFactory4() : TestFactory3() { print_default_created(this); } - TestFactory4(int v) : TestFactory3(v) { print_created(this, v); } + explicit TestFactory4(int v) : TestFactory3(v) { print_created(this, v); } ~TestFactory4() override { print_destroyed(this); } }; // Another class for an invalid downcast test class TestFactory5 : public TestFactory3 { public: - TestFactory5(int i) : TestFactory3(i) { print_created(this, i); } + explicit TestFactory5(int i) : TestFactory3(i) { print_created(this, i); } ~TestFactory5() override { print_destroyed(this); } }; @@ -87,7 +90,7 @@ class TestFactory6 { int value; bool alias = false; public: - TestFactory6(int i) : value{i} { print_created(this, i); } + explicit TestFactory6(int i) : value{i} { print_created(this, i); } TestFactory6(TestFactory6 &&f) noexcept { print_move_created(this); value = f.value; @@ -102,11 +105,20 @@ class PyTF6 : public TestFactory6 { public: // Special constructor that allows the factory to construct a PyTF6 from a TestFactory6 only // when an alias is needed: - PyTF6(TestFactory6 &&base) : TestFactory6(std::move(base)) { alias = true; print_created(this, "move", value); } - PyTF6(int i) : TestFactory6(i) { alias = true; print_created(this, i); } + explicit PyTF6(TestFactory6 &&base) : TestFactory6(std::move(base)) { + alias = true; + print_created(this, "move", value); + } + explicit PyTF6(int i) : TestFactory6(i) { + alias = true; + print_created(this, i); + } PyTF6(PyTF6 &&f) noexcept : TestFactory6(std::move(f)) { print_move_created(this); } PyTF6(const PyTF6 &f) : TestFactory6(f) { print_copy_created(this); } - PyTF6(std::string s) : TestFactory6((int) s.size()) { alias = true; print_created(this, s); } + explicit PyTF6(std::string s) : TestFactory6((int) s.size()) { + alias = true; + print_created(this, s); + } ~PyTF6() override { print_destroyed(this); } int get() override { PYBIND11_OVERRIDE(int, TestFactory6, get, /*no args*/); } }; @@ -116,7 +128,7 @@ class TestFactory7 { int value; bool alias = false; public: - TestFactory7(int i) : value{i} { print_created(this, i); } + explicit TestFactory7(int i) : value{i} { print_created(this, i); } TestFactory7(TestFactory7 &&f) noexcept { print_move_created(this); value = f.value; @@ -129,7 +141,10 @@ class TestFactory7 { }; class PyTF7 : public TestFactory7 { public: - PyTF7(int i) : TestFactory7(i) { alias = true; print_created(this, i); } + explicit PyTF7(int i) : TestFactory7(i) { + alias = true; + print_created(this, i); + } PyTF7(PyTF7 &&f) noexcept : TestFactory7(std::move(f)) { print_move_created(this); } PyTF7(const PyTF7 &f) : TestFactory7(f) { print_copy_created(this); } ~PyTF7() override { print_destroyed(this); } @@ -300,7 +315,7 @@ TEST_SUBMODULE(factory_constructors, m) { // Class with a custom new operator but *without* a placement new operator (issue #948) class NoPlacementNew { public: - NoPlacementNew(int i) : i(i) { } + explicit NoPlacementNew(int i) : i(i) {} static void *operator new(std::size_t s) { auto *p = ::operator new(s); py::print("operator new called, returning", reinterpret_cast(p)); @@ -324,8 +339,8 @@ TEST_SUBMODULE(factory_constructors, m) { // Class that has verbose operator_new/operator_delete calls struct NoisyAlloc { NoisyAlloc(const NoisyAlloc &) = default; - NoisyAlloc(int i) { py::print(py::str("NoisyAlloc(int {})").format(i)); } - NoisyAlloc(double d) { py::print(py::str("NoisyAlloc(double {})").format(d)); } + explicit NoisyAlloc(int i) { py::print(py::str("NoisyAlloc(int {})").format(i)); } + explicit NoisyAlloc(double d) { py::print(py::str("NoisyAlloc(double {})").format(d)); } ~NoisyAlloc() { py::print("~NoisyAlloc()"); } static void *operator new(size_t s) { py::print("noisy new"); return ::operator new(s); } diff --git a/tests/test_local_bindings.cpp b/tests/test_local_bindings.cpp index 8d6e33f790..a5808e2f2a 100644 --- a/tests/test_local_bindings.cpp +++ b/tests/test_local_bindings.cpp @@ -91,7 +91,7 @@ TEST_SUBMODULE(local_bindings, m) { class Cat : public pets::Pet { public: - Cat(std::string name) : Pet(std::move(name)) {} + explicit Cat(std::string name) : Pet(std::move(name)) {} }; py::class_(m, "Pet", py::module_local()) .def("get_name", &pets::Pet::name); diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp index 4cf6f08b85..2d303a44e3 100644 --- a/tests/test_methods_and_attributes.cpp +++ b/tests/test_methods_and_attributes.cpp @@ -19,9 +19,9 @@ using overload_cast_ = pybind11::detail::overload_cast_impl; class ExampleMandA { public: ExampleMandA() { print_default_created(this); } - ExampleMandA(int value) : value(value) { print_created(this, value); } + explicit ExampleMandA(int value) : value(value) { print_created(this, value); } ExampleMandA(const ExampleMandA &e) : value(e.value) { print_copy_created(this); } - ExampleMandA(std::string&&) {} + explicit ExampleMandA(std::string &&) {} ExampleMandA(ExampleMandA &&e) noexcept : value(e.value) { print_move_created(this); } ~ExampleMandA() { print_destroyed(this); } @@ -124,14 +124,14 @@ class NoneCastTester { public: int answer = -1; NoneCastTester() = default; - NoneCastTester(int v) : answer(v) {} + explicit NoneCastTester(int v) : answer(v) {} }; struct StrIssue { int val = -1; StrIssue() = default; - StrIssue(int i) : val{i} {} + explicit StrIssue(int i) : val{i} {} }; // Issues #854, #910: incompatible function args when member function/pointer is in unregistered base class diff --git a/tests/test_modules.cpp b/tests/test_modules.cpp index 5867135404..ce61c1a25c 100644 --- a/tests/test_modules.cpp +++ b/tests/test_modules.cpp @@ -20,7 +20,7 @@ TEST_SUBMODULE(modules, m) { // test_reference_internal class A { public: - A(int v) : v(v) { print_created(this, v); } + explicit A(int v) : v(v) { print_created(this, v); } ~A() { print_destroyed(this); } A(const A&) { print_copy_created(this); } A& operator=(const A ©) { print_copy_assigned(this); v = copy.v; return *this; } diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index b5ca298d91..6963197a5c 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -16,7 +16,7 @@ namespace { // Many bases for testing that multiple inheritance from many classes (i.e. requiring extra // space for holder constructed flags) works. template struct BaseN { - BaseN(int i) : i(i) { } + explicit BaseN(int i) : i(i) {} int i; }; @@ -47,12 +47,12 @@ int VanillaStaticMix2::static_value = 12; // test_multiple_inheritance_virtbase struct Base1a { - Base1a(int i) : i(i) { } + explicit Base1a(int i) : i(i) {} int foo() const { return i; } int i; }; struct Base2a { - Base2a(int i) : i(i) { } + explicit Base2a(int i) : i(i) {} int bar() const { return i; } int i; }; @@ -77,7 +77,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { // test_multiple_inheritance_mix1 // test_multiple_inheritance_mix2 struct Base1 { - Base1(int i) : i(i) { } + explicit Base1(int i) : i(i) {} int foo() const { return i; } int i; }; @@ -86,7 +86,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { .def("foo", &Base1::foo); struct Base2 { - Base2(int i) : i(i) { } + explicit Base2(int i) : i(i) {} int bar() const { return i; } int i; }; diff --git a/tests/test_numpy_vectorize.cpp b/tests/test_numpy_vectorize.cpp index b08a9f7edd..eb5281fb1d 100644 --- a/tests/test_numpy_vectorize.cpp +++ b/tests/test_numpy_vectorize.cpp @@ -52,7 +52,7 @@ TEST_SUBMODULE(numpy_vectorize, m) { // Passthrough test: references and non-pod types should be automatically passed through (in the // function definition below, only `b`, `d`, and `g` are vectorized): struct NonPODClass { - NonPODClass(int v) : value{v} {} + explicit NonPODClass(int v) : value{v} {} int value; }; py::class_(m, "NonPODClass") @@ -71,7 +71,7 @@ TEST_SUBMODULE(numpy_vectorize, m) { // test_method_vectorization struct VectorizeTestClass { - VectorizeTestClass(int v) : value{v} {}; + explicit VectorizeTestClass(int v) : value{v} {}; float method(int x, float y) const { return y + (float) (x + value); } int value = 0; }; diff --git a/tests/test_pickling.cpp b/tests/test_pickling.cpp index 0d5827315f..b77636dd1a 100644 --- a/tests/test_pickling.cpp +++ b/tests/test_pickling.cpp @@ -67,7 +67,7 @@ TEST_SUBMODULE(pickling, m) { // test_roundtrip class Pickleable { public: - Pickleable(const std::string &value) : m_value(value) { } + explicit Pickleable(const std::string &value) : m_value(value) { } const std::string &value() const { return m_value; } void setExtra1(int extra1) { m_extra1 = extra1; } @@ -132,7 +132,7 @@ TEST_SUBMODULE(pickling, m) { // test_roundtrip_with_dict class PickleableWithDict { public: - PickleableWithDict(const std::string &value) : value(value) { } + explicit PickleableWithDict(const std::string &value) : value(value) { } std::string value; int extra; diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp index d49fb1f459..b07fd197a9 100644 --- a/tests/test_sequences_and_iterators.cpp +++ b/tests/test_sequences_and_iterators.cpp @@ -20,7 +20,7 @@ template class NonZeroIterator { const T* ptr_; public: - NonZeroIterator(const T* ptr) : ptr_(ptr) {} + explicit NonZeroIterator(const T *ptr) : ptr_(ptr) {} const T& operator*() const { return *ptr_; } NonZeroIterator& operator++() { ++ptr_; return *this; } }; @@ -77,9 +77,9 @@ TEST_SUBMODULE(sequences_and_iterators, m) { // test_sliceable class Sliceable{ public: - Sliceable(int n): size(n) {} - int start,stop,step; - int size; + explicit Sliceable(int n) : size(n) {} + int start, stop, step; + int size; }; py::class_(m, "Sliceable") .def(py::init()) @@ -96,12 +96,12 @@ TEST_SUBMODULE(sequences_and_iterators, m) { // test_sequence class Sequence { public: - Sequence(size_t size) : m_size(size) { + explicit Sequence(size_t size) : m_size(size) { print_created(this, "of size", m_size); m_data = new float[size]; memset(m_data, 0, sizeof(float) * size); } - Sequence(const std::vector &value) : m_size(value.size()) { + explicit Sequence(const std::vector &value) : m_size(value.size()) { print_created(this, "of size", m_size, "from std::vector"); m_data = new float[m_size]; memcpy(m_data, &value[0], sizeof(float) * m_size); @@ -239,7 +239,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) { class StringMap { public: StringMap() = default; - StringMap(std::unordered_map init) + explicit StringMap(std::unordered_map init) : map(std::move(init)) {} void set(const std::string &key, std::string val) { map[key] = std::move(val); } @@ -276,7 +276,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) { // test_generalized_iterators class IntPairs { public: - IntPairs(std::vector> data) : data_(std::move(data)) {} + explicit IntPairs(std::vector> data) : data_(std::move(data)) {} const std::pair* begin() const { return data_.data(); } private: std::vector> data_; diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index eeaa44147a..94f04330a2 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -24,7 +24,7 @@ template class huge_unique_ptr { std::unique_ptr ptr; uint64_t padding[10]; public: - huge_unique_ptr(T *p) : ptr(p) {} + explicit huge_unique_ptr(T *p) : ptr(p) {} T *get() { return ptr.get(); } }; @@ -33,7 +33,7 @@ template class custom_unique_ptr { std::unique_ptr impl; public: - custom_unique_ptr(T* p) : impl(p) { } + explicit custom_unique_ptr(T *p) : impl(p) {} T* get() const { return impl.get(); } T* release_ptr() { return impl.release(); } }; @@ -46,7 +46,7 @@ class shared_ptr_with_addressof_operator { std::shared_ptr impl; public: shared_ptr_with_addressof_operator( ) = default; - shared_ptr_with_addressof_operator(T* p) : impl(p) { } + explicit shared_ptr_with_addressof_operator(T *p) : impl(p) {} T* get() const { return impl.get(); } T** operator&() { throw std::logic_error("Call of overloaded operator& is not expected"); } }; @@ -59,7 +59,7 @@ class unique_ptr_with_addressof_operator { std::unique_ptr impl; public: unique_ptr_with_addressof_operator() = default; - unique_ptr_with_addressof_operator(T* p) : impl(p) { } + explicit unique_ptr_with_addressof_operator(T *p) : impl(p) {} T* get() const { return impl.get(); } T* release_ptr() { return impl.release(); } T** operator&() { throw std::logic_error("Call of overloaded operator& is not expected"); } @@ -68,7 +68,7 @@ class unique_ptr_with_addressof_operator { // Custom object with builtin reference counting (see 'object.h' for the implementation) class MyObject1 : public Object { public: - MyObject1(int value) : value(value) { print_created(this, toString()); } + explicit MyObject1(int value) : value(value) { print_created(this, toString()); } std::string toString() const override { return "MyObject1[" + std::to_string(value) + "]"; } protected: ~MyObject1() override { print_destroyed(this); } @@ -80,7 +80,7 @@ class MyObject1 : public Object { class MyObject2 { public: MyObject2(const MyObject2 &) = default; - MyObject2(int value) : value(value) { print_created(this, toString()); } + explicit MyObject2(int value) : value(value) { print_created(this, toString()); } std::string toString() const { return "MyObject2[" + std::to_string(value) + "]"; } virtual ~MyObject2() { print_destroyed(this); } private: @@ -91,7 +91,7 @@ class MyObject2 { class MyObject3 : public std::enable_shared_from_this { public: MyObject3(const MyObject3 &) = default; - MyObject3(int value) : value(value) { print_created(this, toString()); } + explicit MyObject3(int value) : value(value) { print_created(this, toString()); } std::string toString() const { return "MyObject3[" + std::to_string(value) + "]"; } virtual ~MyObject3() { print_destroyed(this); } private: @@ -104,7 +104,7 @@ class MyObject4; std::unordered_set myobject4_instances; class MyObject4 { public: - MyObject4(int value) : value{value} { + explicit MyObject4(int value) : value{value} { print_created(this); myobject4_instances.insert(this); } @@ -130,7 +130,7 @@ class MyObject4a; std::unordered_set myobject4a_instances; class MyObject4a { public: - MyObject4a(int i) { + explicit MyObject4a(int i) { value = i; print_created(this); myobject4a_instances.insert(this); @@ -153,14 +153,14 @@ class MyObject4a { // Object derived but with public destructor and no Deleter in default holder class MyObject4b : public MyObject4a { public: - MyObject4b(int i) : MyObject4a(i) { print_created(this); } + explicit MyObject4b(int i) : MyObject4a(i) { print_created(this); } ~MyObject4b() override { print_destroyed(this); } }; // test_large_holder class MyObject5 { // managed by huge_unique_ptr public: - MyObject5(int value) : value{value} { print_created(this); } + explicit MyObject5(int value) : value{value} { print_created(this); } ~MyObject5() { print_destroyed(this); } int value; }; @@ -222,7 +222,7 @@ struct TypeForHolderWithAddressOf { // test_move_only_holder_with_addressof_operator struct TypeForMoveOnlyHolderWithAddressOf { - TypeForMoveOnlyHolderWithAddressOf(int value) : value{value} { print_created(this); } + explicit TypeForMoveOnlyHolderWithAddressOf(int value) : value{value} { print_created(this); } ~TypeForMoveOnlyHolderWithAddressOf() { print_destroyed(this); } std::string toString() const { return "MoveOnlyHolderWithAddressOf[" + std::to_string(value) + "]"; @@ -242,7 +242,7 @@ struct ElementBase { }; struct ElementA : ElementBase { - ElementA(int v) : v(v) { } + explicit ElementA(int v) : v(v) {} int value() const { return v; } int v; }; @@ -291,9 +291,9 @@ TEST_SUBMODULE(smart_ptr, m) { py::implicitly_convertible(); m.def("make_object_1", []() -> Object * { return new MyObject1(1); }); - m.def("make_object_2", []() -> ref { return new MyObject1(2); }); + m.def("make_object_2", []() -> ref { return ref(new MyObject1(2)); }); m.def("make_myobject1_1", []() -> MyObject1 * { return new MyObject1(4); }); - m.def("make_myobject1_2", []() -> ref { return new MyObject1(5); }); + m.def("make_myobject1_2", []() -> ref { return ref(new MyObject1(5)); }); m.def("print_object_1", [](const Object *obj) { py::print(obj->toString()); }); m.def("print_object_2", [](ref obj) { py::print(obj->toString()); }); m.def("print_object_3", [](const ref &obj) { py::print(obj->toString()); }); @@ -328,7 +328,7 @@ TEST_SUBMODULE(smart_ptr, m) { // test_smart_ptr_refcounting m.def("test_object1_refcounting", []() { - ref o = new MyObject1(0); + auto o = ref(new MyObject1(0)); bool good = o->getRefCount() == 1; py::object o2 = py::cast(o, py::return_value_policy::reference); // always request (partial) ownership for objects with intrusive diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 23e2c07b32..7e3363c5ea 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -45,7 +45,8 @@ PYBIND11_MAKE_OPAQUE(std::vector>); /// Issue #528: templated constructor struct TplCtorClass { - template TplCtorClass(const T &) { } + template + explicit TplCtorClass(const T &) {} bool operator==(const TplCtorClass &) const { return true; } }; diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp index 22847eb7a1..6b23e3529f 100644 --- a/tests/test_stl_binders.cpp +++ b/tests/test_stl_binders.cpp @@ -18,7 +18,7 @@ class El { public: El() = delete; - El(int v) : a(v) { } + explicit El(int v) : a(v) {} int a; }; diff --git a/tests/test_tagbased_polymorphic.cpp b/tests/test_tagbased_polymorphic.cpp index 90f40e14cc..2c7bad8bbc 100644 --- a/tests/test_tagbased_polymorphic.cpp +++ b/tests/test_tagbased_polymorphic.cpp @@ -37,33 +37,35 @@ struct Animal struct Dog : Animal { - Dog(const std::string& _name, Kind _kind = Kind::Dog) : Animal(_name, _kind) {} + explicit Dog(const std::string &_name, Kind _kind = Kind::Dog) : Animal(_name, _kind) {} std::string bark() const { return name_of_kind(kind) + " " + name + " goes " + sound; } std::string sound = "WOOF!"; }; struct Labrador : Dog { - Labrador(const std::string& _name, int _excitement = 9001) + explicit Labrador(const std::string &_name, int _excitement = 9001) : Dog(_name, Kind::Labrador), excitement(_excitement) {} int excitement; }; struct Chihuahua : Dog { - Chihuahua(const std::string& _name) : Dog(_name, Kind::Chihuahua) { sound = "iyiyiyiyiyi"; } + explicit Chihuahua(const std::string &_name) : Dog(_name, Kind::Chihuahua) { + sound = "iyiyiyiyiyi"; + } std::string bark() const { return Dog::bark() + " and runs in circles"; } }; struct Cat : Animal { - Cat(const std::string& _name, Kind _kind = Kind::Cat) : Animal(_name, _kind) {} + explicit Cat(const std::string &_name, Kind _kind = Kind::Cat) : Animal(_name, _kind) {} std::string purr() const { return "mrowr"; } }; struct Panther : Cat { - Panther(const std::string& _name) : Cat(_name, Kind::Panther) {} + explicit Panther(const std::string &_name) : Cat(_name, Kind::Panther) {} std::string purr() const { return "mrrrRRRRRR"; } }; diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index f83a7364b2..1eba534dd2 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -15,7 +15,7 @@ /* This is an example class that we'll want to be able to extend from Python */ class ExampleVirt { public: - ExampleVirt(int state) : state(state) { print_created(this, state); } + explicit ExampleVirt(int state) : state(state) { print_created(this, state); } ExampleVirt(const ExampleVirt &e) : state(e.state) { print_copy_created(this); } ExampleVirt(ExampleVirt &&e) noexcept : state(e.state) { print_move_created(this);