diff --git a/.clang-tidy b/.clang-tidy index cefffba1e..a6437dd49 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/.pre-commit-config.yaml b/.pre-commit-config.yaml index 025b2ac56..e0df4aeb3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,7 +32,7 @@ repos: exclude: ^noxfile.py$ - repo: https://github.com/asottile/pyupgrade - rev: v2.24.0 + rev: v2.25.0 hooks: - id: pyupgrade @@ -43,7 +43,7 @@ repos: # Black, the code formatter, natively supports pre-commit - repo: https://github.com/psf/black - rev: 21.7b0 + rev: 21.8b0 hooks: - id: black # By default, this ignores pyi files, though black supports them diff --git a/docs/compiling.rst b/docs/compiling.rst index bf7acfc80..eaf3270e0 100644 --- a/docs/compiling.rst +++ b/docs/compiling.rst @@ -113,7 +113,7 @@ with the following: from pybind11.setup_helpers import ParallelCompile, naive_recompile - SmartCompile("NPY_NUM_BUILD_JOBS", needs_recompile=naive_recompile).install() + ParallelCompile("NPY_NUM_BUILD_JOBS", needs_recompile=naive_recompile).install() If you have a more complex build, you can implement a smarter function and pass diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index ab1fe8044..13f68bbe8 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 b02b05919..197a1d48c 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -104,7 +104,7 @@ public: 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) \ @@ -301,7 +301,7 @@ public: } template using cast_op_type = void*&; - operator void *&() { return value; } + explicit operator void *&() { return value; } static constexpr auto name = _("capsule"); private: void *value = nullptr; @@ -509,8 +509,10 @@ public: return StringCaster::cast(StringType(1, src), policy, parent); } - operator CharT*() { return none ? nullptr : const_cast(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"); @@ -603,8 +605,8 @@ public: 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 e4f00293e..a8fd7d917 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -918,6 +918,7 @@ public: // 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 @@ public: 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 0acfc7db3..0b498e5e7 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 bd7bdc398..a7bda4624 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -25,7 +25,7 @@ public: } 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: @@ -294,7 +294,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 5a5acc2e6..4c9ac7b8f 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 @@ private: 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 @@ public: 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 @@ public: 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 b03e15f62..c0363827c 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 @@ public: 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 @@ public: 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 7b5d7cd24..9843f0f97 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 bc8a8af82..24141ce38 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -69,6 +69,10 @@ public: // 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 @@ public: // 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 e4d209585..95449a07b 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -123,7 +123,7 @@ private: } 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 @@ protected: 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 @@ public: \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 fa128efdd..d55844086 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -477,7 +477,7 @@ public: 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 @@ 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 f72374b23..8bc592613 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -83,10 +83,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...); } @@ -94,6 +96,7 @@ public: /// 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...); @@ -101,6 +104,7 @@ public: /// 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...); @@ -110,6 +114,7 @@ public: /// 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...); @@ -117,6 +122,7 @@ public: /// 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...); @@ -126,6 +132,7 @@ public: /// 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...); @@ -177,7 +184,7 @@ protected: #endif // UB without std::launder, but without breaking ABI and/or // a significant refactoring it's "impossible" to solve. - if (!std::is_trivially_destructible::value) + if (!std::is_trivially_destructible::value) rec->free_data = [](function_record *r) { auto data = PYBIND11_STD_LAUNDER((capture *) &r->data); (void) data; @@ -2148,7 +2155,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 5e699e53f..971db85f3 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -178,6 +178,7 @@ public: /// 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 @@ public: 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,7 +763,8 @@ template struct arrow_proxy { T value; - arrow_proxy(T &&value) : value(std::move(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 @@ public: } // '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 @@ public: \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 @@ public: 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 @@ public: } // 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 @@ public: 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 @@ public: // 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 @@ public: 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 @@ public: 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 @@ public: 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 f11c08e61..4c936c19a 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 6851fc624..be21bf631 100644 --- a/tests/object.h +++ b/tests/object.h @@ -65,7 +65,7 @@ public: 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 @@ public: 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 4bfd5302d..5838cb274 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 12d8a777f..800ddda48 100644 --- a/tests/pybind11_tests.h +++ b/tests/pybind11_tests.h @@ -1,9 +1,6 @@ #pragma once -// This must be kept first for MSVC 2015. -// Do not remove the empty line between the #includes. #include - #include #if defined(_MSC_VER) && _MSC_VER < 1910 @@ -19,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); }; @@ -35,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 c7e2c7df3..3a8e3e7b7 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 be4431aad..71c778e8e 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_callbacks.cpp b/tests/test_callbacks.cpp index a50771038..58688b6e8 100644 --- a/tests/test_callbacks.cpp +++ b/tests/test_callbacks.cpp @@ -81,16 +81,55 @@ TEST_SUBMODULE(callbacks, m) { }; // Export the payload constructor statistics for testing purposes: m.def("payload_cstats", &ConstructorStats::get); - /* Test cleanup of lambda closure */ - m.def("test_cleanup", []() -> std::function { + m.def("test_lambda_closure_cleanup", []() -> std::function { Payload p; + // In this situation, `Func` in the implementation of + // `cpp_function::initialize` is NOT trivially destructible. return [p]() { /* p should be cleaned up when the returned function is garbage collected */ (void) p; }; }); + class CppCallable { + public: + CppCallable() { track_default_created(this); } + ~CppCallable() { track_destroyed(this); } + CppCallable(const CppCallable &) { track_copy_created(this); } + CppCallable(CppCallable &&) noexcept { track_move_created(this); } + void operator()() {} + }; + + m.def("test_cpp_callable_cleanup", []() { + // Related issue: https://github.com/pybind/pybind11/issues/3228 + // Related PR: https://github.com/pybind/pybind11/pull/3229 + py::list alive_counts; + ConstructorStats &stat = ConstructorStats::get(); + alive_counts.append(stat.alive()); + { + CppCallable cpp_callable; + alive_counts.append(stat.alive()); + { + // In this situation, `Func` in the implementation of + // `cpp_function::initialize` IS trivially destructible, + // only `capture` is not. + py::cpp_function py_func(cpp_callable); + py::detail::silence_unused_warnings(py_func); + alive_counts.append(stat.alive()); + } + alive_counts.append(stat.alive()); + { + py::cpp_function py_func(std::move(cpp_callable)); + py::detail::silence_unused_warnings(py_func); + alive_counts.append(stat.alive()); + } + alive_counts.append(stat.alive()); + } + alive_counts.append(stat.alive()); + return alive_counts; + }); + // test_cpp_function_roundtrip /* Test if passing a function pointer from C++ -> Python -> C++ yields the original pointer */ m.def("dummy_function", &dummy_function); diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index 9dc272a2d..edbb1890c 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -79,13 +79,18 @@ def test_keyword_args_and_generalized_unpacking(): def test_lambda_closure_cleanup(): - m.test_cleanup() + m.test_lambda_closure_cleanup() cstats = m.payload_cstats() assert cstats.alive() == 0 assert cstats.copy_constructions == 1 assert cstats.move_constructions >= 1 +def test_cpp_callable_cleanup(): + alive_counts = m.test_cpp_callable_cleanup() + assert alive_counts == [0, 1, 2, 1, 2, 1, 0] + + def test_cpp_function_roundtrip(): """Test if passing a function pointer from C++ -> Python -> C++ yields the original pointer""" diff --git a/tests/test_class.cpp b/tests/test_class.cpp index d2c689654..7f4f373df 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -29,7 +29,7 @@ namespace { // 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) {} @@ -84,18 +84,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 { @@ -222,7 +222,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") @@ -277,7 +277,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 859da9df7..5fb0dd810 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 @@ public: 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 @@ public: 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 @@ public: 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 e9a6058b1..490952299 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 78b64be6b..fae14f751 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_enum.py b/tests/test_enum.py index 11cab6ddf..85302b080 100644 --- a/tests/test_enum.py +++ b/tests/test_enum.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import pytest +import env from pybind11_tests import enums as m @@ -238,20 +239,26 @@ def test_duplicate_enum_name(): def test_char_underlying_enum(): # Issue #1331/PR #1334: assert type(m.ScopedCharEnum.Positive.__int__()) is int - assert int(m.ScopedChar16Enum.Zero) == 0 # int() call should successfully return + assert int(m.ScopedChar16Enum.Zero) == 0 assert hash(m.ScopedChar32Enum.Positive) == 1 - assert m.ScopedCharEnum.Positive.__getstate__() == 1 # return type is long in py2.x + if env.PY2: + assert m.ScopedCharEnum.Positive.__getstate__() == 1 # long + else: + assert type(m.ScopedCharEnum.Positive.__getstate__()) is int assert m.ScopedWCharEnum(1) == m.ScopedWCharEnum.Positive with pytest.raises(TypeError): - # Enum should construct with a int, even with char underlying type - m.ScopedWCharEnum("0") + # Even if the underlying type is char, only an int can be used to construct the enum: + m.ScopedCharEnum("0") def test_bool_underlying_enum(): assert type(m.ScopedBoolEnum.TRUE.__int__()) is int assert int(m.ScopedBoolEnum.FALSE) == 0 assert hash(m.ScopedBoolEnum.TRUE) == 1 - assert m.ScopedBoolEnum.TRUE.__getstate__() == 1 + if env.PY2: + assert m.ScopedBoolEnum.TRUE.__getstate__() == 1 # long + else: + assert type(m.ScopedBoolEnum.TRUE.__getstate__()) is int assert m.ScopedBoolEnum(1) == m.ScopedBoolEnum.TRUE # Enum could construct with a bool # (bool is a strict subclass of int, and False will be converted to 0) diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 4805a0249..25adb32ed 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -81,7 +81,7 @@ private: 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 06ad61c03..729715d01 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 @@ public: 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 @@ public: 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 @@ protected: 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 @@ protected: 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 @@ public: }; 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 8d6e33f79..a5808e2f2 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 d37588e52..03f6d6a24 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 586713540..ce61c1a25 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 809cb2964..6e357b791 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 b08a9f7ed..eb5281fb1 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 0d5827315..b77636dd1 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 d49fb1f45..b07fd197a 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 a47bda32b..eef17b402 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 @@ public: // 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 @@ private: 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 @@ private: 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 @@ protected: // 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; }; @@ -323,9 +323,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()); }); @@ -360,7 +360,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 23e2c07b3..7e3363c5e 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 22847eb7a..6b23e3529 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 90f40e14c..2c7bad8bb 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 f83a7364b..1eba534dd 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);