From 12d76600f8b9092707ff3fe4aab61baf8dc2adb8 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sun, 16 Oct 2016 16:27:42 -0400 Subject: [PATCH] Disable most implicit conversion constructors We have various classes that have non-explicit constructors that accept a single argument, which is implicitly making them implicitly convertible from the argument. In a few cases, this is desirable (e.g. implicit conversion of std::string to py::str, or conversion of double to py::float_); in many others, however, it is unintended (e.g. implicit conversion of size_t to some pre-declared py::array_t type). This disables most of the unwanted implicit conversions by marking them `explicit`, and comments the ones that are deliberately left implicit. --- include/pybind11/cast.h | 10 +++++----- include/pybind11/common.h | 2 +- include/pybind11/numpy.h | 18 +++++++++--------- include/pybind11/pybind11.h | 12 ++++++------ include/pybind11/pytypes.h | 31 +++++++++++++++++++------------ 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 00c29acae..91912de75 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -311,7 +311,7 @@ public: static PYBIND11_DESCR name() { return type_descr(_()); } type_caster_base() : type_caster_base(typeid(type)) { } - type_caster_base(const std::type_info &info) : type_caster_generic(info) { } + explicit type_caster_base(const std::type_info &info) : type_caster_generic(info) { } static handle cast(const itype &src, return_value_policy policy, handle parent) { if (policy == return_value_policy::automatic || policy == return_value_policy::automatic_reference) @@ -1154,7 +1154,7 @@ template class simple_collector { public: template - simple_collector(Ts &&...values) + explicit simple_collector(Ts &&...values) : m_args(pybind11::make_tuple(std::forward(values)...)) { } const tuple &args() const & { return m_args; } @@ -1179,7 +1179,7 @@ template class unpacking_collector { public: template - unpacking_collector(Ts &&...values) { + explicit unpacking_collector(Ts &&...values) { // Tuples aren't (easily) resizable so a list is needed for collection, // but the actual function call strictly requires a tuple. auto args_list = list(); @@ -1283,7 +1283,7 @@ private: template ::value>> simple_collector collect_arguments(Args &&...args) { - return {std::forward(args)...}; + return simple_collector(std::forward(args)...); } /// Collect all arguments, including keywords and unpacking (only instantiated when needed) @@ -1297,7 +1297,7 @@ unpacking_collector collect_arguments(Args &&...args) { "Invalid function call: positional args must precede keywords and ** unpacking; " "* unpacking must precede ** unpacking" ); - return { std::forward(args)... }; + return unpacking_collector(std::forward(args)...); } template diff --git a/include/pybind11/common.h b/include/pybind11/common.h index 2ee469121..40cee8703 100644 --- a/include/pybind11/common.h +++ b/include/pybind11/common.h @@ -248,7 +248,7 @@ struct buffer_info { : buffer_info(ptr, itemsize, format, 1, std::vector { size }, std::vector { itemsize }) { } - buffer_info(Py_buffer *view, bool ownview = true) + explicit buffer_info(Py_buffer *view, bool ownview = true) : ptr(view->buf), itemsize((size_t) view->itemsize), size(1), format(view->format), ndim((size_t) view->ndim), shape((size_t) view->ndim), strides((size_t) view->ndim), view(view), ownview(ownview) { for (size_t i = 0; i < (size_t) view->ndim; ++i) { diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index d437c922f..cee40c817 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -165,16 +165,16 @@ class dtype : public object { public: PYBIND11_OBJECT_DEFAULT(dtype, object, detail::npy_api::get().PyArrayDescr_Check_); - dtype(const buffer_info &info) { + explicit dtype(const buffer_info &info) { dtype descr(_dtype_from_pep3118()(PYBIND11_STR_TYPE(info.format))); m_ptr = descr.strip_padding().release().ptr(); } - dtype(std::string format) { + explicit dtype(std::string format) { 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, size_t itemsize) { dict args; @@ -317,7 +317,7 @@ public: array(size_t count, const T *ptr, handle base = handle()) : array(std::vector{ count }, ptr, base) { } - array(const buffer_info &info) + explicit array(const buffer_info &info) : array(pybind11::dtype(info), info.shape, info.strides, info.ptr) { } /// Array descriptor (dtype) @@ -477,18 +477,18 @@ public: array_t() : array() { } - array_t(const buffer_info& info) : array(info) { } + explicit array_t(const buffer_info& info) : array(info) { } array_t(const std::vector &shape, const std::vector &strides, const T *ptr = nullptr, handle base = handle()) : array(shape, strides, ptr, base) { } - array_t(const std::vector &shape, const T *ptr = nullptr, + explicit array_t(const std::vector &shape, const T *ptr = nullptr, handle base = handle()) : array(shape, ptr, base) { } - array_t(size_t count, const T *ptr = nullptr, handle base = handle()) + explicit array_t(size_t count, const T *ptr = nullptr, handle base = handle()) : array(count, ptr, base) { } constexpr size_t itemsize() const { @@ -607,7 +607,7 @@ DECL_FMT(std::complex, NPY_CDOUBLE_, "complex128"); #define DECL_CHAR_FMT \ static PYBIND11_DESCR name() { return _("S") + _(); } \ - static pybind11::dtype dtype() { return std::string("S") + std::to_string(N); } + static pybind11::dtype dtype() { return pybind11::dtype(std::string("S") + std::to_string(N)); } template struct npy_format_descriptor { DECL_CHAR_FMT }; template struct npy_format_descriptor> { DECL_CHAR_FMT }; #undef DECL_CHAR_FMT @@ -883,7 +883,7 @@ struct vectorize_helper { typename std::remove_reference::type f; template - vectorize_helper(T&&f) : f(std::forward(f)) { } + explicit vectorize_helper(T&&f) : f(std::forward(f)) { } object operator()(array_t... args) { return run(args..., typename make_index_sequence::type()); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index f19435d34..752f77564 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -44,12 +44,12 @@ public: /// Construct a cpp_function from a vanilla function pointer template - cpp_function(Return (*f)(Args...), const Extra&... extra) { + explicit cpp_function(Return (*f)(Args...), const Extra&... extra) { initialize(f, f, extra...); } /// Construct a cpp_function from a lambda function (possibly with internal state) - template cpp_function(Func &&f, const Extra&... extra) { + template explicit cpp_function(Func &&f, const Extra&... extra) { initialize(std::forward(f), (typename detail::remove_class::type::operator())>::type *) nullptr, extra...); @@ -57,14 +57,14 @@ public: /// Construct a cpp_function from a class method (non-const) template - cpp_function(Return (Class::*f)(Arg...), const Extra&... extra) { + explicit 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...); } /// Construct a cpp_function from a class method (const) template - cpp_function(Return (Class::*f)(Arg...) const, const Extra&... extra) { + explicit 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...); } @@ -528,7 +528,7 @@ class module : public object { public: PYBIND11_OBJECT_DEFAULT(module, object, PyModule_Check) - module(const char *name, const char *doc = nullptr) { + explicit module(const char *name, const char *doc = nullptr) { #if PY_MAJOR_VERSION >= 3 PyModuleDef *def = new PyModuleDef(); memset(def, 0, sizeof(PyModuleDef)); @@ -1524,7 +1524,7 @@ private: class gil_scoped_release { public: - gil_scoped_release(bool disassoc = false) : disassoc(disassoc) { + explicit gil_scoped_release(bool disassoc = false) : disassoc(disassoc) { tstate = PyEval_SaveThread(); if (disassoc) { auto key = detail::get_internals().tstate; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 61599d2b3..6ea7f7a5c 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -317,7 +317,7 @@ NAMESPACE_END(accessor_policies) struct dict_iterator { public: - dict_iterator(handle dict = handle(), ssize_t pos = -1) : dict(dict), pos(pos) { } + explicit dict_iterator(handle dict = handle(), ssize_t pos = -1) : dict(dict), pos(pos) { } dict_iterator& operator++() { if (!PyDict_Next(dict.ptr(), &pos, &key.ptr(), &value.ptr())) pos = -1; @@ -350,12 +350,12 @@ inline bool PyUnicode_Check_Permissive(PyObject *o) { return PyUnicode_Check(o) class kwargs_proxy : public handle { public: - kwargs_proxy(handle h) : handle(h) { } + explicit kwargs_proxy(handle h) : handle(h) { } }; class args_proxy : public handle { public: - args_proxy(handle h) : handle(h) { } + explicit args_proxy(handle h) : handle(h) { } kwargs_proxy operator*() const { return kwargs_proxy(*this); } }; @@ -381,6 +381,7 @@ NAMESPACE_END(detail) #define PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, CvtStmt) \ public: \ Name(const handle &h, bool borrowed) : Parent(h, borrowed) { CvtStmt; } \ + /* These are deliberately not 'explicit' to allow implicit conversion from object: */ \ Name(const object& o): Parent(o) { CvtStmt; } \ Name(object&& o) noexcept : Parent(std::move(o)) { CvtStmt; } \ Name& operator=(object&& o) noexcept { (void) object::operator=(std::move(o)); CvtStmt; return *this; } \ @@ -470,6 +471,7 @@ public: if (!m_ptr) pybind11_fail("Could not allocate string object!"); } + // 'explicit' is explicitly omitted from the following constructors to allow implicit conversion to py::str from C++ string-like objects str(const char *c) : object(PyUnicode_FromString(c), false) { if (!m_ptr) pybind11_fail("Could not allocate string object!"); @@ -477,7 +479,7 @@ public: str(const std::string &s) : str(s.data(), s.size()) { } - str(const bytes &b); + explicit str(const bytes &b); operator std::string() const { object temp = *this; @@ -508,6 +510,7 @@ class bytes : public object { public: PYBIND11_OBJECT_DEFAULT(bytes, object, PYBIND11_BYTES_CHECK) + // Allow implicit conversion: bytes(const char *c) : object(PYBIND11_BYTES_FROM_STRING(c), false) { if (!m_ptr) pybind11_fail("Could not allocate bytes object!"); @@ -518,9 +521,10 @@ public: if (!m_ptr) pybind11_fail("Could not allocate bytes object!"); } + // Allow implicit conversion: bytes(const std::string &s) : bytes(s.data(), s.size()) { } - bytes(const pybind11::str &s); + explicit bytes(const pybind11::str &s); operator std::string() const { char *buffer; @@ -568,6 +572,7 @@ public: class bool_ : public object { public: PYBIND11_OBJECT_DEFAULT(bool_, object, PyBool_Check) + // Allow implicit conversion from `bool`: bool_(bool value) : object(value ? Py_True : Py_False, true) { } operator bool() const { return m_ptr && PyLong_AsLong(m_ptr) != 0; } }; @@ -575,6 +580,7 @@ public: class int_ : public object { public: PYBIND11_OBJECT_DEFAULT(int_, object, PYBIND11_LONG_CHECK) + // Allow implicit conversion from C++ integral types: template ::value, int> = 0> int_(T value) { @@ -612,6 +618,7 @@ public: class float_ : public object { public: PYBIND11_OBJECT_DEFAULT(float_, object, PyFloat_Check) + // Allow implicit conversion from float/double: float_(float value) : object(PyFloat_FromDouble((double) value), false) { if (!m_ptr) pybind11_fail("Could not allocate float object!"); } @@ -625,7 +632,7 @@ public: class weakref : public object { public: PYBIND11_OBJECT_DEFAULT(weakref, object, PyWeakref_Check) - weakref(handle obj, handle callback = handle()) : object(PyWeakref_NewRef(obj.ptr(), callback.ptr()), false) { + explicit weakref(handle obj, handle callback = handle()) : object(PyWeakref_NewRef(obj.ptr(), callback.ptr()), false) { if (!m_ptr) pybind11_fail("Could not allocate weak reference!"); } }; @@ -651,7 +658,7 @@ class capsule : public object { public: PYBIND11_OBJECT_DEFAULT(capsule, object, PyCapsule_CheckExact) capsule(PyObject *obj, bool borrowed) : object(obj, borrowed) { } - capsule(const void *value, void (*destruct)(PyObject *) = nullptr) + explicit capsule(const void *value, void (*destruct)(PyObject *) = nullptr) : object(PyCapsule_New(const_cast(value), nullptr, destruct), false) { if (!m_ptr) pybind11_fail("Could not allocate capsule object!"); } @@ -665,7 +672,7 @@ public: class tuple : public object { public: PYBIND11_OBJECT(tuple, object, PyTuple_Check) - tuple(size_t size = 0) : object(PyTuple_New((ssize_t) size), false) { + explicit tuple(size_t size = 0) : object(PyTuple_New((ssize_t) size), false) { if (!m_ptr) pybind11_fail("Could not allocate tuple object!"); } size_t size() const { return (size_t) PyTuple_Size(m_ptr); } @@ -682,7 +689,7 @@ public: typename = detail::enable_if_t::value>, // MSVC workaround: it can't compile an out-of-line definition, so defer the collector typename collector = detail::deferred_t, Args...>> - dict(Args &&...args) : dict(collector(std::forward(args)...).kwargs()) { } + explicit dict(Args &&...args) : dict(collector(std::forward(args)...).kwargs()) { } size_t size() const { return (size_t) PyDict_Size(m_ptr); } detail::dict_iterator begin() const { return (++detail::dict_iterator(*this, 0)); } @@ -702,7 +709,7 @@ public: class list : public object { public: PYBIND11_OBJECT(list, object, PyList_Check) - list(size_t size = 0) : object(PyList_New((ssize_t) size), false) { + explicit list(size_t size = 0) : object(PyList_New((ssize_t) size), false) { if (!m_ptr) pybind11_fail("Could not allocate list object!"); } size_t size() const { return (size_t) PyList_Size(m_ptr); } @@ -749,7 +756,7 @@ public: class memoryview : public object { public: - memoryview(const buffer_info& info) { + explicit memoryview(const buffer_info& info) { static Py_buffer buf { }; // Py_buffer uses signed sizes, strides and shape!.. static std::vector py_strides { }; @@ -793,7 +800,7 @@ template item_accessor object_api::operator[](handle key) const template item_accessor object_api::operator[](const char *key) const { return {derived(), pybind11::str(key)}; } template obj_attr_accessor object_api::attr(handle key) const { return {derived(), object(key, true)}; } template str_attr_accessor object_api::attr(const char *key) const { return {derived(), key}; } -template args_proxy object_api::operator*() const { return {derived().ptr()}; } +template args_proxy object_api::operator*() const { return args_proxy(derived().ptr()); } template template bool object_api::contains(T &&key) const { return attr("__contains__")(std::forward(key)).template cast(); }