fix(clang-tidy): Add cppcoreguidelines-init-vars,slicing, and throw-by-value-catch-by-reference checks (#3094)

* clang-tidy: guard against more UB behavior

* Remove slicing check for now
This commit is contained in:
Aaron Gokaslan 2021-07-13 09:54:32 -04:00 committed by GitHub
parent 6a644c8f04
commit 25e470c57d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 33 additions and 30 deletions

View File

@ -1,10 +1,12 @@
FormatStyle: file FormatStyle: file
Checks: ' Checks: '
cppcoreguidelines-init-variables,
clang-analyzer-optin.cplusplus.VirtualCall, clang-analyzer-optin.cplusplus.VirtualCall,
llvm-namespace-comment, llvm-namespace-comment,
misc-misplaced-const, misc-misplaced-const,
misc-static-assert, misc-static-assert,
misc-throw-by-value-catch-by-reference,
misc-uniqueptr-reset-release, misc-uniqueptr-reset-release,
modernize-avoid-bind, modernize-avoid-bind,
modernize-redundant-void-arg, modernize-redundant-void-arg,
@ -33,6 +35,8 @@ readability-uniqueptr-delete-release,
' '
CheckOptions: CheckOptions:
- key: performance-for-range-copy.WarnOnAllAutoCopies
value: true
- key: performance-unnecessary-value-param.AllowedTypes - key: performance-unnecessary-value-param.AllowedTypes
value: 'exception_ptr$;' value: 'exception_ptr$;'

View File

@ -55,7 +55,7 @@ object eval(const str &expr, object global = globals(), object local = object())
this seems to be the only alternative */ this seems to be the only alternative */
std::string buffer = "# -*- coding: utf-8 -*-\n" + (std::string) expr; std::string buffer = "# -*- coding: utf-8 -*-\n" + (std::string) expr;
int start; int start = 0;
switch (mode) { switch (mode) {
case eval_expr: start = Py_eval_input; break; case eval_expr: start = Py_eval_input; break;
case eval_single_statement: start = Py_single_input; break; case eval_single_statement: start = Py_single_input; break;
@ -107,7 +107,7 @@ object eval_file(str fname, object global = globals(), object local = object())
detail::ensure_builtins_in_globals(global); detail::ensure_builtins_in_globals(global);
int start; int start = 0;
switch (mode) { switch (mode) {
case eval_expr: start = Py_eval_input; break; case eval_expr: start = Py_eval_input; break;
case eval_single_statement: start = Py_single_input; break; case eval_single_statement: start = Py_single_input; break;

View File

@ -490,7 +490,7 @@ inline handle get_function(handle value) {
inline PyObject * dict_getitemstring(PyObject *v, const char *key) inline PyObject * dict_getitemstring(PyObject *v, const char *key)
{ {
#if PY_MAJOR_VERSION >= 3 #if PY_MAJOR_VERSION >= 3
PyObject *kv, *rv; PyObject *kv = nullptr, *rv = nullptr;
kv = PyUnicode_FromString(key); kv = PyUnicode_FromString(key);
if (kv == NULL) { if (kv == NULL) {
throw error_already_set(); throw error_already_set();
@ -1016,8 +1016,8 @@ public:
if (!temp) if (!temp)
pybind11_fail("Unable to extract string contents! (encoding issue)"); pybind11_fail("Unable to extract string contents! (encoding issue)");
} }
char *buffer; char *buffer = nullptr;
ssize_t length; ssize_t length = 0;
if (PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), &buffer, &length)) if (PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), &buffer, &length))
pybind11_fail("Unable to extract string contents! (invalid type)"); pybind11_fail("Unable to extract string contents! (invalid type)");
return std::string(buffer, (size_t) length); return std::string(buffer, (size_t) length);
@ -1072,8 +1072,8 @@ public:
explicit bytes(const pybind11::str &s); explicit bytes(const pybind11::str &s);
operator std::string() const { operator std::string() const {
char *buffer; char *buffer = nullptr;
ssize_t length; ssize_t length = 0;
if (PYBIND11_BYTES_AS_STRING_AND_SIZE(m_ptr, &buffer, &length)) if (PYBIND11_BYTES_AS_STRING_AND_SIZE(m_ptr, &buffer, &length))
pybind11_fail("Unable to extract bytes contents!"); pybind11_fail("Unable to extract bytes contents!");
return std::string(buffer, (size_t) length); return std::string(buffer, (size_t) length);
@ -1090,8 +1090,8 @@ inline bytes::bytes(const pybind11::str &s) {
if (!temp) if (!temp)
pybind11_fail("Unable to extract string contents! (encoding issue)"); pybind11_fail("Unable to extract string contents! (encoding issue)");
} }
char *buffer; char *buffer = nullptr;
ssize_t length; ssize_t length = 0;
if (PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), &buffer, &length)) if (PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), &buffer, &length))
pybind11_fail("Unable to extract string contents! (invalid type)"); pybind11_fail("Unable to extract string contents! (invalid type)");
auto obj = reinterpret_steal<object>(PYBIND11_BYTES_FROM_STRING_AND_SIZE(buffer, length)); auto obj = reinterpret_steal<object>(PYBIND11_BYTES_FROM_STRING_AND_SIZE(buffer, length));
@ -1101,8 +1101,8 @@ inline bytes::bytes(const pybind11::str &s) {
} }
inline str::str(const bytes& b) { inline str::str(const bytes& b) {
char *buffer; char *buffer = nullptr;
ssize_t length; ssize_t length = 0;
if (PYBIND11_BYTES_AS_STRING_AND_SIZE(b.ptr(), &buffer, &length)) if (PYBIND11_BYTES_AS_STRING_AND_SIZE(b.ptr(), &buffer, &length))
pybind11_fail("Unable to extract bytes contents!"); pybind11_fail("Unable to extract bytes contents!");
auto obj = reinterpret_steal<object>(PyUnicode_FromStringAndSize(buffer, (ssize_t) length)); auto obj = reinterpret_steal<object>(PyUnicode_FromStringAndSize(buffer, (ssize_t) length));

View File

@ -217,9 +217,10 @@ void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_t
); );
/// Slicing protocol /// Slicing protocol
cl.def("__getitem__", cl.def(
"__getitem__",
[](const Vector &v, slice slice) -> Vector * { [](const Vector &v, slice slice) -> Vector * {
size_t start, stop, step, slicelength; size_t start = 0, stop = 0, step = 0, slicelength = 0;
if (!slice.compute(v.size(), &start, &stop, &step, &slicelength)) if (!slice.compute(v.size(), &start, &stop, &step, &slicelength))
throw error_already_set(); throw error_already_set();
@ -234,12 +235,12 @@ void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_t
return seq; return seq;
}, },
arg("s"), arg("s"),
"Retrieve list elements using a slice object" "Retrieve list elements using a slice object");
);
cl.def("__setitem__", cl.def(
"__setitem__",
[](Vector &v, slice slice, const Vector &value) { [](Vector &v, slice slice, const Vector &value) {
size_t start, stop, step, slicelength; size_t start = 0, stop = 0, step = 0, slicelength = 0;
if (!slice.compute(v.size(), &start, &stop, &step, &slicelength)) if (!slice.compute(v.size(), &start, &stop, &step, &slicelength))
throw error_already_set(); throw error_already_set();
@ -251,8 +252,7 @@ void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_t
start += step; start += step;
} }
}, },
"Assign list elements using a slice object" "Assign list elements using a slice object");
);
cl.def("__delitem__", cl.def("__delitem__",
[wrap_i](Vector &v, DiffType i) { [wrap_i](Vector &v, DiffType i) {
@ -262,9 +262,10 @@ void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_t
"Delete the list elements at index ``i``" "Delete the list elements at index ``i``"
); );
cl.def("__delitem__", cl.def(
"__delitem__",
[](Vector &v, slice slice) { [](Vector &v, slice slice) {
size_t start, stop, step, slicelength; size_t start = 0, stop = 0, step = 0, slicelength = 0;
if (!slice.compute(v.size(), &start, &stop, &step, &slicelength)) if (!slice.compute(v.size(), &start, &stop, &step, &slicelength))
throw error_already_set(); throw error_already_set();
@ -278,9 +279,7 @@ void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_t
} }
} }
}, },
"Delete list elements using a slice object" "Delete list elements using a slice object");
);
} }
// If the type has an operator[] that doesn't return a reference (most notably std::vector<bool>), // If the type has an operator[] that doesn't return a reference (most notably std::vector<bool>),

View File

@ -66,7 +66,7 @@ TEST_SUBMODULE(eval_, m) {
auto local = py::dict(); auto local = py::dict();
local["y"] = py::int_(43); local["y"] = py::int_(43);
int val_out; int val_out = 0;
local["call_test2"] = py::cpp_function([&](int value) { val_out = value; }); local["call_test2"] = py::cpp_function([&](int value) { val_out = value; });
auto result = py::eval_file(std::move(filename), global, local); auto result = py::eval_file(std::move(filename), global, local);

View File

@ -92,7 +92,7 @@ TEST_SUBMODULE(numpy_vectorize, m) {
[](const py::array_t<int, py::array::forcecast> &arg1, [](const py::array_t<int, py::array::forcecast> &arg1,
const py::array_t<float, py::array::forcecast> &arg2, const py::array_t<float, py::array::forcecast> &arg2,
const py::array_t<double, py::array::forcecast> &arg3) { const py::array_t<double, py::array::forcecast> &arg3) {
py::ssize_t ndim; py::ssize_t ndim = 0;
std::vector<py::ssize_t> shape; std::vector<py::ssize_t> shape;
std::array<py::buffer_info, 3> buffers{ std::array<py::buffer_info, 3> buffers{
{arg1.request(), arg2.request(), arg3.request()}}; {arg1.request(), arg2.request(), arg3.request()}};

View File

@ -84,7 +84,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
py::class_<Sliceable>(m, "Sliceable") py::class_<Sliceable>(m, "Sliceable")
.def(py::init<int>()) .def(py::init<int>())
.def("__getitem__", [](const Sliceable &s, const py::slice &slice) { .def("__getitem__", [](const Sliceable &s, const py::slice &slice) {
py::ssize_t start, stop, step, slicelength; py::ssize_t start = 0, stop = 0, step = 0, slicelength = 0;
if (!slice.compute(s.size, &start, &stop, &step, &slicelength)) if (!slice.compute(s.size, &start, &stop, &step, &slicelength))
throw py::error_already_set(); throw py::error_already_set();
int istart = static_cast<int>(start); int istart = static_cast<int>(start);
@ -204,7 +204,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
/// Slicing protocol (optional) /// Slicing protocol (optional)
.def("__getitem__", .def("__getitem__",
[](const Sequence &s, const py::slice &slice) -> Sequence * { [](const Sequence &s, const py::slice &slice) -> Sequence * {
size_t start, stop, step, slicelength; size_t start = 0, stop = 0, step = 0, slicelength = 0;
if (!slice.compute(s.size(), &start, &stop, &step, &slicelength)) if (!slice.compute(s.size(), &start, &stop, &step, &slicelength))
throw py::error_already_set(); throw py::error_already_set();
auto *seq = new Sequence(slicelength); auto *seq = new Sequence(slicelength);
@ -216,7 +216,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
}) })
.def("__setitem__", .def("__setitem__",
[](Sequence &s, const py::slice &slice, const Sequence &value) { [](Sequence &s, const py::slice &slice, const Sequence &value) {
size_t start, stop, step, slicelength; size_t start = 0, stop = 0, step = 0, slicelength = 0;
if (!slice.compute(s.size(), &start, &stop, &step, &slicelength)) if (!slice.compute(s.size(), &start, &stop, &step, &slicelength))
throw py::error_already_set(); throw py::error_already_set();
if (slicelength != value.size()) if (slicelength != value.size())