diff --git a/modules/core/include/opencv2/core/bindings_utils.hpp b/modules/core/include/opencv2/core/bindings_utils.hpp index 92f27f0e87..5c8df7a52b 100644 --- a/modules/core/include/opencv2/core/bindings_utils.hpp +++ b/modules/core/include/opencv2/core/bindings_utils.hpp @@ -64,6 +64,20 @@ String dumpString(const String& argument) return cv::format("String: %s", argument.c_str()); } +CV_WRAP static inline +String testOverloadResolution(int value, const Point& point = Point(42, 24)) +{ + return format("overload (int=%d, point=(x=%d, y=%d))", value, point.x, + point.y); +} + +CV_WRAP static inline +String testOverloadResolution(const Rect& rect) +{ + return format("overload (rect=(x=%d, y=%d, w=%d, h=%d))", rect.x, rect.y, + rect.width, rect.height); +} + CV_WRAP static inline AsyncArray testAsyncArray(InputArray argument) { diff --git a/modules/python/src2/cv2.cpp b/modules/python/src2/cv2.cpp index d5ecd0e20a..0a986c1a44 100644 --- a/modules/python/src2/cv2.cpp +++ b/modules/python/src2/cv2.cpp @@ -33,6 +33,7 @@ #include "opencv2/core/utils/configuration.private.hpp" #include "opencv2/core/utils/logger.hpp" +#include "opencv2/core/utils/tls.hpp" #include "pyopencv_generated_include.h" #include "opencv2/core/types_c.h" @@ -138,6 +139,51 @@ private: PyGILState_STATE _state; }; +/** + * Light weight RAII wrapper for `PyObject*` owning references. + * In comparisson to C++11 `std::unique_ptr` with custom deleter, it provides + * implicit conversion functions that might be useful to initialize it with + * Python functions those returns owning references through the `PyObject**` + * e.g. `PyErr_Fetch` or directly pass it to functions those want to borrow + * reference to object (doesn't extend object lifetime) e.g. `PyObject_Str`. + */ +class PySafeObject +{ +public: + PySafeObject() : obj_(NULL) {} + + explicit PySafeObject(PyObject* obj) : obj_(obj) {} + + ~PySafeObject() + { + Py_CLEAR(obj_); + } + + operator PyObject*() + { + return obj_; + } + + operator PyObject**() + { + return &obj_; + } + + PyObject* release() + { + PyObject* obj = obj_; + obj_ = NULL; + return obj; + } + +private: + PyObject* obj_; + + // Explicitly disable copy operations + PySafeObject(const PySafeObject*); // = delete + PySafeObject& operator=(const PySafeObject&); // = delete +}; + static void pyRaiseCVException(const cv::Exception &e) { PyObject_SetAttrString(opencv_error, "file", PyString_FromString(e.file.c_str())); @@ -290,6 +336,74 @@ bool parseNumpyScalar(PyObject* obj, T& value) return false; } +TLSData > conversionErrorsTLS; + +inline void pyPrepareArgumentConversionErrorsStorage(std::size_t size) +{ + std::vector& conversionErrors = conversionErrorsTLS.getRef(); + conversionErrors.clear(); + conversionErrors.reserve(size); +} + +void pyRaiseCVOverloadException(const std::string& functionName) +{ + const std::vector& conversionErrors = conversionErrorsTLS.getRef(); + const std::size_t conversionErrorsCount = conversionErrors.size(); + if (conversionErrorsCount > 0) + { + // In modern std libraries small string optimization is used = no dynamic memory allocations, + // but it can be applied only for string with length < 18 symbols (in GCC) + const std::string bullet = "\n - "; + + // Estimate required buffer size - save dynamic memory allocations = faster + std::size_t requiredBufferSize = bullet.size() * conversionErrorsCount; + for (std::size_t i = 0; i < conversionErrorsCount; ++i) + { + requiredBufferSize += conversionErrors[i].size(); + } + + // Only string concatenation is required so std::string is way faster than + // std::ostringstream + std::string errorMessage("Overload resolution failed:"); + errorMessage.reserve(errorMessage.size() + requiredBufferSize); + for (std::size_t i = 0; i < conversionErrorsCount; ++i) + { + errorMessage += bullet; + errorMessage += conversionErrors[i]; + } + cv::Exception exception(CV_StsBadArg, errorMessage, functionName, "", -1); + pyRaiseCVException(exception); + } + else + { + cv::Exception exception(CV_StsInternal, "Overload resolution failed, but no errors reported", + functionName, "", -1); + pyRaiseCVException(exception); + } +} + +void pyPopulateArgumentConversionErrors() +{ + if (PyErr_Occurred()) + { + PySafeObject exception_type; + PySafeObject exception_value; + PySafeObject exception_traceback; + PyErr_Fetch(exception_type, exception_value, exception_traceback); + PyErr_NormalizeException(exception_type, exception_value, + exception_traceback); + + PySafeObject exception_message(PyObject_Str(exception_value)); + std::string message; + getUnicodeString(exception_message, message); +#ifdef CV_CXX11 + conversionErrorsTLS.getRef().push_back(std::move(message)); +#else + conversionErrorsTLS.getRef().push_back(message); +#endif + } +} + } // namespace typedef std::vector vector_uchar; diff --git a/modules/python/src2/gen2.py b/modules/python/src2/gen2.py index d3c8ec39cf..4acca07ada 100755 --- a/modules/python/src2/gen2.py +++ b/modules/python/src2/gen2.py @@ -174,6 +174,14 @@ gen_template_prop_init = Template(""" gen_template_rw_prop_init = Template(""" {(char*)"${member}", (getter)pyopencv_${name}_get_${member}, (setter)pyopencv_${name}_set_${member}, (char*)"${member}", NULL},""") +gen_template_overloaded_function_call = Template(""" + { +${variant} + + pyPopulateArgumentConversionErrors(); + } +""") + class FormatStrings: string = 's' unsigned_char = 'b' @@ -768,8 +776,12 @@ class FuncInfo(object): # if the function/method has only 1 signature, then just put it code += all_code_variants[0] else: - # try to execute each signature - code += " PyErr_Clear();\n\n".join([" {\n" + v + " }\n" for v in all_code_variants]) + # try to execute each signature, add an interlude between function + # calls to collect error from all conversions + code += ' pyPrepareArgumentConversionErrorsStorage({});\n'.format(len(all_code_variants)) + code += ' \n'.join(gen_template_overloaded_function_call.substitute(variant=v) + for v in all_code_variants) + code += ' pyRaiseCVOverloadException("{}");\n'.format(self.name) def_ret = "NULL" if self.isconstructor: diff --git a/modules/python/test/test_misc.py b/modules/python/test/test_misc.py index 9ab85be46c..50761e7f46 100644 --- a/modules/python/test/test_misc.py +++ b/modules/python/test/test_misc.py @@ -73,6 +73,44 @@ class Bindings(NewOpenCVTests): except cv.error as _e: pass + def test_overload_resolution_can_choose_correct_overload(self): + val = 123 + point = (51, 165) + self.assertEqual(cv.utils.testOverloadResolution(val, point), + 'overload (int={}, point=(x={}, y={}))'.format(val, *point), + "Can't select first overload if all arguments are provided as positional") + + self.assertEqual(cv.utils.testOverloadResolution(val, point=point), + 'overload (int={}, point=(x={}, y={}))'.format(val, *point), + "Can't select first overload if one of the arguments are provided as keyword") + + self.assertEqual(cv.utils.testOverloadResolution(val), + 'overload (int={}, point=(x=42, y=24))'.format(val), + "Can't select first overload if one of the arguments has default value") + + rect = (1, 5, 10, 23) + self.assertEqual(cv.utils.testOverloadResolution(rect), + 'overload (rect=(x={}, y={}, w={}, h={}))'.format(*rect), + "Can't select second overload if all arguments are provided") + + def test_overload_resolution_fails(self): + def test_overload_resolution(msg, *args, **kwargs): + no_exception_msg = 'Overload resolution failed without any exception for: "{}"'.format(msg) + wrong_exception_msg = 'Overload resolution failed with wrong exception type for: "{}"'.format(msg) + with self.assertRaises((cv.error, Exception), msg=no_exception_msg) as cm: + cv.utils.testOverloadResolution(*args, **kwargs) + self.assertEqual(type(cm.exception), cv.error, wrong_exception_msg) + + test_overload_resolution('wrong second arg type (keyword arg)', 5, point=(1, 2, 3)) + test_overload_resolution('wrong second arg type', 5, 2) + test_overload_resolution('wrong first arg', 3.4, (12, 21)) + test_overload_resolution('wrong first arg, no second arg', 4.5) + test_overload_resolution('wrong args number for first overload', 3, (12, 21), 123) + test_overload_resolution('wrong args number for second overload', (3, 12, 12, 1), (12, 21)) + # One of the common problems + test_overload_resolution('rect with float coordinates', (4.5, 4, 2, 1)) + test_overload_resolution('rect with wrong number of coordinates', (4, 4, 1)) + class Arguments(NewOpenCVTests):