From b00e577ad470b2c5ce1ff5140ece8b6d1f24b6e2 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 25 Aug 2023 15:53:19 -0700 Subject: [PATCH 1/3] Change variable name `it` to `curr_overl` to improve readability, add `assert()` in the only place where `curr_overl` could become `nullptr`. --- include/pybind11/pybind11.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index db91639dc0..c998a4a8f8 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -719,7 +719,7 @@ class cpp_function : public function { /* Iterator over the list of potentially admissible overloads */ const function_record *overloads = reinterpret_cast( PyCapsule_GetPointer(self, get_function_record_capsule_name())), - *it = overloads; + *curr_overl = overloads; assert(overloads != nullptr); /* Need to know how many arguments + keyword arguments there are to pick the right @@ -757,9 +757,9 @@ class cpp_function : public function { std::vector second_pass; // However, if there are no overloads, we can just skip the no-convert pass entirely - const bool overloaded = it != nullptr && it->next != nullptr; + const bool overloaded = curr_overl != nullptr && curr_overl->next != nullptr; - for (; it != nullptr; it = it->next) { + for (; curr_overl != nullptr; curr_overl = curr_overl->next) { /* For each overload: 1. Copy all positional arguments we were given, also checking to make sure that @@ -780,7 +780,7 @@ class cpp_function : public function { a result other than PYBIND11_TRY_NEXT_OVERLOAD. */ - const function_record &func = *it; + const function_record &func = *curr_overl; size_t num_args = func.nargs; // Number of positional arguments that we need if (func.has_args) { --num_args; // (but don't count py::args @@ -1018,10 +1018,11 @@ class cpp_function : public function { } if (result.ptr() != PYBIND11_TRY_NEXT_OVERLOAD) { - // The error reporting logic below expects 'it' to be valid, as it would be - // if we'd encountered this failure in the first-pass loop. + // The error reporting logic below expects 'curr_overl' to be valid, as it + // would be if we'd encountered this failure in the first-pass loop. if (!result) { - it = &call.func; + curr_overl = &call.func; + assert(curr_overl != nullptr); } break; } @@ -1168,7 +1169,7 @@ class cpp_function : public function { if (!result) { std::string msg = "Unable to convert function return value to a " "Python type! The signature was\n\t"; - msg += it->signature; + msg += curr_overl->signature; append_note_if_missing_header_is_suspected(msg); // Attach additional error info to the exception if supported if (PyErr_Occurred()) { From fae188c3d737e7e9f11e24bb233cb9495f46cb39 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 31 Aug 2023 12:21:54 -0700 Subject: [PATCH 2/3] Move the `assert()` to where Coverty generates a false positive. --- include/pybind11/pybind11.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index d6b1ed507d..6d8a221f52 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1022,7 +1022,6 @@ class cpp_function : public function { // would be if we'd encountered this failure in the first-pass loop. if (!result) { curr_overl = &call.func; - assert(curr_overl != nullptr); } break; } @@ -1169,6 +1168,7 @@ class cpp_function : public function { if (!result) { std::string msg = "Unable to convert function return value to a " "Python type! The signature was\n\t"; + assert(curr_overl != nullptr); msg += curr_overl->signature; append_note_if_missing_header_is_suspected(msg); // Attach additional error info to the exception if supported From bce63c84059c8bef22f678773d0a64a17ed55210 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 1 Sep 2023 09:05:03 -0700 Subject: [PATCH 3/3] variable name as suggested by @henryiii --- include/pybind11/pybind11.h | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 6d8a221f52..0a82f8dd75 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -719,7 +719,7 @@ class cpp_function : public function { /* Iterator over the list of potentially admissible overloads */ const function_record *overloads = reinterpret_cast( PyCapsule_GetPointer(self, get_function_record_capsule_name())), - *curr_overl = overloads; + *current_overload = overloads; assert(overloads != nullptr); /* Need to know how many arguments + keyword arguments there are to pick the right @@ -757,9 +757,10 @@ class cpp_function : public function { std::vector second_pass; // However, if there are no overloads, we can just skip the no-convert pass entirely - const bool overloaded = curr_overl != nullptr && curr_overl->next != nullptr; + const bool overloaded + = current_overload != nullptr && current_overload->next != nullptr; - for (; curr_overl != nullptr; curr_overl = curr_overl->next) { + for (; current_overload != nullptr; current_overload = current_overload->next) { /* For each overload: 1. Copy all positional arguments we were given, also checking to make sure that @@ -780,7 +781,7 @@ class cpp_function : public function { a result other than PYBIND11_TRY_NEXT_OVERLOAD. */ - const function_record &func = *curr_overl; + const function_record &func = *current_overload; size_t num_args = func.nargs; // Number of positional arguments that we need if (func.has_args) { --num_args; // (but don't count py::args @@ -1018,10 +1019,10 @@ class cpp_function : public function { } if (result.ptr() != PYBIND11_TRY_NEXT_OVERLOAD) { - // The error reporting logic below expects 'curr_overl' to be valid, as it - // would be if we'd encountered this failure in the first-pass loop. + // The error reporting logic below expects 'current_overload' to be valid, + // as it would be if we'd encountered this failure in the first-pass loop. if (!result) { - curr_overl = &call.func; + current_overload = &call.func; } break; } @@ -1168,8 +1169,8 @@ class cpp_function : public function { if (!result) { std::string msg = "Unable to convert function return value to a " "Python type! The signature was\n\t"; - assert(curr_overl != nullptr); - msg += curr_overl->signature; + assert(current_overload != nullptr); + msg += current_overload->signature; append_note_if_missing_header_is_suspected(msg); // Attach additional error info to the exception if supported if (PyErr_Occurred()) {