From 2ae024ce8f19e271b5fcb5f311f6d3d567ac12b0 Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Sun, 1 Dec 2024 20:39:30 -0500 Subject: [PATCH] refactor: Fix clang-tidy warnings in `Py_utils`. (#95) Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- lint-tasks.yml | 4 ++- src/clp_ffi_py/.clang-tidy | 6 +++++ src/clp_ffi_py/Py_utils.cpp | 49 +++++++++++++++++++++++-------------- src/clp_ffi_py/Py_utils.hpp | 2 +- src/clp_ffi_py/Python.hpp | 2 +- src/clp_ffi_py/utils.hpp | 10 ++++++++ 6 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 src/clp_ffi_py/.clang-tidy diff --git a/lint-tasks.yml b/lint-tasks.yml index 6c004666..362bee30 100644 --- a/lint-tasks.yml +++ b/lint-tasks.yml @@ -103,7 +103,9 @@ tasks: cmds: - task: ":utils:clang-tidy" vars: - FLAGS: "--config-file={{.ROOT_DIR}}/.clang-tidy -p {{.CLP_FFI_PY_COMPILE_COMMANDS_DB}}" + FLAGS: >- + --config-file "{{.CLP_FFI_PY_CPP_SRC_DIR}}/.clang-tidy" + -p "{{.CLP_FFI_PY_COMPILE_COMMANDS_DB}}" SRC_PATHS: ref: ".G_CPP_LINT_DIRS" VENV_DIR: "{{.G_LINT_VENV_DIR}}" diff --git a/src/clp_ffi_py/.clang-tidy b/src/clp_ffi_py/.clang-tidy new file mode 100644 index 00000000..66d28440 --- /dev/null +++ b/src/clp_ffi_py/.clang-tidy @@ -0,0 +1,6 @@ +InheritParentConfig: true + +CheckOptions: + # Variable naming rules + # Allow PyObject global variables to start with `Py_` + readability-identifier-naming.GlobalVariableIgnoredRegexp: "Py(_[a-z0-9]+)+" diff --git a/src/clp_ffi_py/Py_utils.cpp b/src/clp_ffi_py/Py_utils.cpp index 266e0753..9e13e36b 100644 --- a/src/clp_ffi_py/Py_utils.cpp +++ b/src/clp_ffi_py/Py_utils.cpp @@ -2,25 +2,30 @@ #include "Py_utils.hpp" +#include #include +#include + #include #include +#include namespace clp_ffi_py { namespace { -constexpr char const* const cPyFuncNameGetFormattedTimestamp{"get_formatted_timestamp"}; -PyObjectStaticPtr Py_func_get_formatted_timestamp{nullptr}; +constexpr std::string_view cPyFuncNameGetFormattedTimestamp{"get_formatted_timestamp"}; +constexpr std::string_view cPyFuncNameGetTimezoneFromTimezoneId{"get_timezone_from_timezone_id"}; +constexpr std::string_view cPyFuncNameSerializeDictToMsgpack{"serialize_dict_to_msgpack"}; +constexpr std::string_view cPyFuncNameParseJsonStr{"parse_json_str"}; -constexpr char const* const cPyFuncNameGetTimezoneFromTimezoneId{"get_timezone_from_timezone_id"}; +// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables) +PyObjectStaticPtr Py_func_get_formatted_timestamp{nullptr}; PyObjectStaticPtr Py_func_get_timezone_from_timezone_id{nullptr}; - -constexpr std::string_view cPyFuncNameSerializeDictToMsgpack{"serialize_dict_to_msgpack"}; PyObjectStaticPtr Py_func_serialize_dict_to_msgpack{nullptr}; - -constexpr std::string_view cPyFuncNameParseJsonStr{"parse_json_str"}; PyObjectStaticPtr Py_func_parse_json_str{nullptr}; +// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables) + /** * Wrapper of PyObject_CallObject. * @param func PyObject that points to the calling function. @@ -39,28 +44,34 @@ auto py_utils_init() -> bool { return false; } - Py_func_get_timezone_from_timezone_id.reset( - PyObject_GetAttrString(py_utils, cPyFuncNameGetTimezoneFromTimezoneId) - ); + Py_func_get_timezone_from_timezone_id.reset(PyObject_GetAttrString( + py_utils, + get_c_str_from_constexpr_string_view(cPyFuncNameGetTimezoneFromTimezoneId) + )); if (nullptr == Py_func_get_timezone_from_timezone_id.get()) { return false; } - Py_func_get_formatted_timestamp.reset( - PyObject_GetAttrString(py_utils, cPyFuncNameGetFormattedTimestamp) - ); + Py_func_get_formatted_timestamp.reset(PyObject_GetAttrString( + py_utils, + get_c_str_from_constexpr_string_view(cPyFuncNameGetFormattedTimestamp) + )); if (nullptr == Py_func_get_formatted_timestamp.get()) { return false; } - Py_func_serialize_dict_to_msgpack.reset( - PyObject_GetAttrString(py_utils, cPyFuncNameSerializeDictToMsgpack.data()) - ); + Py_func_serialize_dict_to_msgpack.reset(PyObject_GetAttrString( + py_utils, + get_c_str_from_constexpr_string_view(cPyFuncNameSerializeDictToMsgpack) + )); if (nullptr == Py_func_serialize_dict_to_msgpack.get()) { return false; } - Py_func_parse_json_str.reset(PyObject_GetAttrString(py_utils, cPyFuncNameParseJsonStr.data())); + Py_func_parse_json_str.reset(PyObject_GetAttrString( + py_utils, + get_c_str_from_constexpr_string_view(cPyFuncNameParseJsonStr) + )); if (nullptr == Py_func_parse_json_str.get()) { return false; } @@ -112,7 +123,9 @@ auto py_utils_serialize_dict_to_msgpack(PyDictObject* py_dict) -> PyBytesObject* } auto py_utils_parse_json_str(std::string_view json_str) -> PyObject* { - PyObjectPtr const func_args_ptr{Py_BuildValue("(s)", json_str.data())}; + PyObjectPtr const func_args_ptr{ + Py_BuildValue("(s#)", json_str.data(), static_cast(json_str.size())) + }; auto* func_args{func_args_ptr.get()}; if (nullptr == func_args) { return nullptr; diff --git a/src/clp_ffi_py/Py_utils.hpp b/src/clp_ffi_py/Py_utils.hpp index bd687661..4a3f1586 100644 --- a/src/clp_ffi_py/Py_utils.hpp +++ b/src/clp_ffi_py/Py_utils.hpp @@ -6,7 +6,7 @@ #include #include -#include +#include namespace clp_ffi_py { /** diff --git a/src/clp_ffi_py/Python.hpp b/src/clp_ffi_py/Python.hpp index 931572a4..a5570fff 100644 --- a/src/clp_ffi_py/Python.hpp +++ b/src/clp_ffi_py/Python.hpp @@ -18,7 +18,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/clp_ffi_py/utils.hpp b/src/clp_ffi_py/utils.hpp index 3554034a..ad6ae2ba 100644 --- a/src/clp_ffi_py/utils.hpp +++ b/src/clp_ffi_py/utils.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -85,6 +86,15 @@ auto handle_traceable_exception(clp::TraceableException& exception) noexcept -> template [[maybe_unused]] constexpr bool cAlwaysFalse{false}; +/** + * @param sv + * @return The underlying C-string of the given constexpr string view. + */ +[[nodiscard]] consteval auto get_c_str_from_constexpr_string_view(std::string_view const& sv +) -> char const* { + return sv.data(); +} + template auto parse_py_int(PyObject* py_int, int_type& val) -> bool { if (false == static_cast(PyLong_Check(py_int))) {