Skip to content

Commit

Permalink
refactor: Fix clang-tidy warnings for the rest of files directly unde…
Browse files Browse the repository at this point in the history
…r `src/clp_ffi_py`; Use `std::string_view` for static error message strings. (#102)
  • Loading branch information
LinZhihao-723 authored Dec 6, 2024
1 parent b0df0dc commit b1cbedb
Show file tree
Hide file tree
Showing 11 changed files with 111 additions and 45 deletions.
6 changes: 6 additions & 0 deletions lint-tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,14 @@ tasks:
# TODO: Before all clang-tidy violations are resolved, we should only run clang-tidy on
# the files whose violations we've fixed, both to ensure they remain free of violations
# and so that the workflow doesn't fail due to violations in other files.
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/api_decoration.hpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/error_messages.hpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/ExceptionFFI.hpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/Py_utils.cpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/Py_utils.hpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/PyExceptionContext.hpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/PyObjectCast.hpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/PyObjectUtils.hpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/utils.cpp"
- "{{.CLP_FFI_PY_CPP_SRC_DIR}}/utils.hpp"
- "{{.G_CPP_WRAPPED_FACADE_HEADERS_DIR}}"
Expand Down
10 changes: 5 additions & 5 deletions src/clp_ffi_py/PyObjectCast.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,26 +57,26 @@ auto py_releasebufferproc_cast(Src src) noexcept -> releasebufferproc {
* @tparam T
*/
template <typename T>
struct is_python_object {
struct IsPythonObject {
static constexpr bool cValue = false;
};

/**
* This template const expression is a wrapper of underlying `cValue` stored in `is_python_object`,
* This template const expression is a wrapper of underlying `cValue` stored in `IsPythonObject`,
* which is used to determine whether a type T is a valid Python object type.
* @tparam T
*/
template <typename T> // NOLINTNEXTLINE(readability-identifier-naming)
constexpr bool is_python_object_v{is_python_object<T>::cValue};
constexpr bool is_python_object_v{IsPythonObject<T>::cValue};

/**
* The macro to create a specialization of is_python_object for a given type T. Only types that are
* The macro to create a specialization of `IsPythonObject` for a given type T. Only types that are
* marked with this macro will be considered as a valid Python object type.
*/
// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
#define CLP_FFI_PY_MARK_AS_PYOBJECT(T) \
template <> \
struct is_python_object<T> { \
struct IsPythonObject<T> { \
static constexpr bool cValue = true; \
}

Expand Down
18 changes: 11 additions & 7 deletions src/clp_ffi_py/error_messages.hpp
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
#ifndef CLP_FFI_PY_ERROR_MESSAGES
#define CLP_FFI_PY_ERROR_MESSAGES

#include <string_view>

namespace clp_ffi_py {
constexpr char const* const cOutofMemoryError = "Failed to allocate memory.";
constexpr char const* const cPyTypeError = "Wrong Python Type received.";
constexpr char const* const cSetstateInputError
= "Python dictionary is expected to be the input of __setstate__ method.";
constexpr char const* const cSetstateKeyErrorTemplate = "\"%s\" not found in the state dictionary.";
constexpr char const* const cTimezoneObjectNotInitialzed
= "Timezone (tzinfo) object is not yet initialized.";
constexpr std::string_view cOutOfMemoryError{"Failed to allocate memory."};
constexpr std::string_view cPyTypeError{"Wrong Python Type received."};
constexpr std::string_view cSetstateInputError{
"Python dictionary is expected to be the input of __setstate__ method."
};
constexpr std::string_view cSetstateKeyErrorTemplate{"\"%s\" not found in the state dictionary."};
constexpr std::string_view cTimezoneObjectNotInitialized{
"Timezone (tzinfo) object is not yet initialized."
};
} // namespace clp_ffi_py

#endif // CLP_FFI_PY_ERROR_MESSAGES
5 changes: 4 additions & 1 deletion src/clp_ffi_py/ir/native/PyDeserializerBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,10 @@ auto PyDeserializerBuffer::create(PyObject* input_stream, Py_ssize_t buf_capacit
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast)
PyDeserializerBuffer* self{PyObject_New(PyDeserializerBuffer, get_py_type())};
if (nullptr == self) {
PyErr_SetString(PyExc_MemoryError, clp_ffi_py::cOutofMemoryError);
PyErr_SetString(
PyExc_MemoryError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cOutOfMemoryError)
);
return nullptr;
}
self->default_init();
Expand Down
6 changes: 4 additions & 2 deletions src/clp_ffi_py/ir/native/PyKeyValuePairLogEvent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,10 @@ auto decode_as_encoded_text_ast(Value const& val) -> std::optional<std::string>
auto PyKeyValuePairLogEvent::init(clp::ffi::KeyValuePairLogEvent kv_pair_log_event) -> bool {
m_kv_pair_log_event = new clp::ffi::KeyValuePairLogEvent{std::move(kv_pair_log_event)};
if (nullptr == m_kv_pair_log_event) {
PyErr_SetString(PyExc_RuntimeError, clp_ffi_py::cOutofMemoryError);
PyErr_SetString(
PyExc_RuntimeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cOutOfMemoryError)
);
return false;
}
return true;
Expand Down Expand Up @@ -679,7 +682,6 @@ auto PyKeyValuePairLogEvent::create(clp::ffi::KeyValuePairLogEvent kv_log_event
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast)
PyKeyValuePairLogEvent* self{PyObject_New(PyKeyValuePairLogEvent, get_py_type())};
if (nullptr == self) {
PyErr_SetString(PyExc_MemoryError, clp_ffi_py::cOutofMemoryError);
return nullptr;
}
self->default_init();
Expand Down
38 changes: 29 additions & 9 deletions src/clp_ffi_py/ir/native/PyLogEvent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ auto PyLogEvent_init(PyLogEvent* self, PyObject* args, PyObject* keywords) -> in
if (has_metadata
&& false == static_cast<bool>(PyObject_TypeCheck(metadata, PyMetadata::get_py_type())))
{
PyErr_SetString(PyExc_TypeError, clp_ffi_py::cPyTypeError);
PyErr_SetString(
PyExc_TypeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cPyTypeError)
);
return -1;
}

Expand Down Expand Up @@ -175,13 +178,20 @@ auto PyLogEvent_setstate(PyLogEvent* self, PyObject* state) -> PyObject* {
self->default_init();

if (false == static_cast<bool>(PyDict_CheckExact(state))) {
PyErr_SetString(PyExc_ValueError, clp_ffi_py::cSetstateInputError);
PyErr_SetString(
PyExc_ValueError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateInputError)
);
return nullptr;
}

auto* log_message_obj{PyDict_GetItemString(state, cStateLogMessage)};
if (nullptr == log_message_obj) {
PyErr_Format(PyExc_KeyError, clp_ffi_py::cSetstateKeyErrorTemplate, cStateLogMessage);
PyErr_Format(
PyExc_KeyError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateKeyErrorTemplate),
cStateLogMessage
);
return nullptr;
}
std::string log_message;
Expand All @@ -193,7 +203,7 @@ auto PyLogEvent_setstate(PyLogEvent* self, PyObject* state) -> PyObject* {
if (nullptr == formatted_timestamp_obj) {
PyErr_Format(
PyExc_KeyError,
clp_ffi_py::cSetstateKeyErrorTemplate,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateKeyErrorTemplate),
cStateFormattedTimestamp
);
return nullptr;
Expand All @@ -205,7 +215,11 @@ auto PyLogEvent_setstate(PyLogEvent* self, PyObject* state) -> PyObject* {

auto* timestamp_obj{PyDict_GetItemString(state, cStateTimestamp)};
if (nullptr == timestamp_obj) {
PyErr_Format(PyExc_KeyError, clp_ffi_py::cSetstateKeyErrorTemplate, cStateTimestamp);
PyErr_Format(
PyExc_KeyError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateKeyErrorTemplate),
cStateTimestamp
);
return nullptr;
}
clp::ir::epoch_time_ms_t timestamp{0};
Expand All @@ -215,7 +229,11 @@ auto PyLogEvent_setstate(PyLogEvent* self, PyObject* state) -> PyObject* {

auto* index_obj{PyDict_GetItemString(state, cStateIndex)};
if (nullptr == index_obj) {
PyErr_Format(PyExc_KeyError, clp_ffi_py::cSetstateKeyErrorTemplate, cStateIndex);
PyErr_Format(
PyExc_KeyError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateKeyErrorTemplate),
cStateIndex
);
return nullptr;
}
size_t index{0};
Expand Down Expand Up @@ -301,7 +319,7 @@ PyDoc_STRVAR(

auto PyLogEvent_match_query(PyLogEvent* self, PyObject* query) -> PyObject* {
if (false == static_cast<bool>(PyObject_TypeCheck(query, PyQuery::get_py_type()))) {
PyErr_SetString(PyExc_TypeError, cPyTypeError);
PyErr_SetString(PyExc_TypeError, get_c_str_from_constexpr_string_view(cPyTypeError));
return nullptr;
}
auto* py_query{py_reinterpret_cast<PyQuery>(query)};
Expand Down Expand Up @@ -478,7 +496,10 @@ auto PyLogEvent::init(
// NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
m_log_event = new LogEvent(log_message, timestamp, index, formatted_timestamp);
if (nullptr == m_log_event) {
PyErr_SetString(PyExc_RuntimeError, clp_ffi_py::cOutofMemoryError);
PyErr_SetString(
PyExc_RuntimeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cOutOfMemoryError)
);
return false;
}
set_metadata(metadata);
Expand Down Expand Up @@ -510,7 +531,6 @@ auto PyLogEvent::create_new_log_event(
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast)
PyLogEvent* self{PyObject_New(PyLogEvent, get_py_type())};
if (nullptr == self) {
PyErr_SetString(PyExc_MemoryError, clp_ffi_py::cOutofMemoryError);
return nullptr;
}
self->default_init();
Expand Down
15 changes: 12 additions & 3 deletions src/clp_ffi_py/ir/native/PyMetadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ PyDoc_STRVAR(
auto PyMetadata_get_timezone(PyMetadata* self) -> PyObject* {
auto* timezone{self->get_py_timezone()};
if (nullptr == timezone) {
PyErr_SetString(PyExc_RuntimeError, clp_ffi_py::cTimezoneObjectNotInitialzed);
PyErr_SetString(
PyExc_RuntimeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cTimezoneObjectNotInitialized)
);
return nullptr;
}
Py_INCREF(timezone);
Expand Down Expand Up @@ -227,7 +230,10 @@ auto PyMetadata::init(
// NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
m_metadata = new Metadata(ref_timestamp, input_timestamp_format, input_timezone);
if (nullptr == m_metadata) {
PyErr_SetString(PyExc_RuntimeError, clp_ffi_py::cOutofMemoryError);
PyErr_SetString(
PyExc_RuntimeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cOutOfMemoryError)
);
return false;
}
return init_py_timezone();
Expand All @@ -243,7 +249,10 @@ auto PyMetadata::init(nlohmann::json const& metadata, bool is_four_byte_encoding
return false;
}
if (nullptr == m_metadata) {
PyErr_SetString(PyExc_RuntimeError, clp_ffi_py::cOutofMemoryError);
PyErr_SetString(
PyExc_RuntimeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cOutOfMemoryError)
);
return false;
}
return init_py_timezone();
Expand Down
40 changes: 28 additions & 12 deletions src/clp_ffi_py/ir/native/PyQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ auto deserialize_wildcard_queries(
}

if (false == static_cast<bool>(PyObject_TypeCheck(py_wildcard_queries, &PyList_Type))) {
PyErr_SetString(PyExc_TypeError, clp_ffi_py::cPyTypeError);
PyErr_SetString(
PyExc_TypeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cPyTypeError)
);
return false;
}

Expand All @@ -44,7 +47,10 @@ auto deserialize_wildcard_queries(
for (Py_ssize_t idx{0}; idx < wildcard_queries_size; ++idx) {
auto* wildcard_query{PyList_GetItem(py_wildcard_queries, idx)};
if (1 != PyObject_IsInstance(wildcard_query, PyQuery::get_py_wildcard_query_type())) {
PyErr_SetString(PyExc_TypeError, clp_ffi_py::cPyTypeError);
PyErr_SetString(
PyExc_TypeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cPyTypeError)
);
return false;
}
auto* wildcard_query_py_str{PyObject_GetAttrString(wildcard_query, "wildcard_query")};
Expand Down Expand Up @@ -275,15 +281,18 @@ auto PyQuery_setstate(PyQuery* self, PyObject* state) -> PyObject* {
self->default_init();

if (false == static_cast<bool>(PyDict_CheckExact(state))) {
PyErr_SetString(PyExc_ValueError, clp_ffi_py::cSetstateInputError);
PyErr_SetString(
PyExc_ValueError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateInputError)
);
return nullptr;
}

auto* search_time_lower_bound_obj{PyDict_GetItemString(state, cStateSearchTimeLowerBound)};
if (nullptr == search_time_lower_bound_obj) {
PyErr_Format(
PyExc_KeyError,
clp_ffi_py::cSetstateKeyErrorTemplate,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateKeyErrorTemplate),
cStateSearchTimeLowerBound
);
return nullptr;
Expand All @@ -302,7 +311,7 @@ auto PyQuery_setstate(PyQuery* self, PyObject* state) -> PyObject* {
if (nullptr == search_time_upper_bound_obj) {
PyErr_Format(
PyExc_KeyError,
clp_ffi_py::cSetstateKeyErrorTemplate,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateKeyErrorTemplate),
cStateSearchTimeUpperBound
);
return nullptr;
Expand All @@ -319,7 +328,11 @@ auto PyQuery_setstate(PyQuery* self, PyObject* state) -> PyObject* {

auto* py_wildcard_queries{PyDict_GetItemString(state, cStateWildcardQueries)};
if (nullptr == py_wildcard_queries) {
PyErr_Format(PyExc_KeyError, clp_ffi_py::cSetstateKeyErrorTemplate, cStateWildcardQueries);
PyErr_Format(
PyExc_KeyError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateKeyErrorTemplate),
cStateWildcardQueries
);
return nullptr;
}
std::vector<WildcardQuery> wildcard_queries;
Expand All @@ -333,7 +346,7 @@ auto PyQuery_setstate(PyQuery* self, PyObject* state) -> PyObject* {
if (nullptr == search_time_termination_margin_obj) {
PyErr_Format(
PyExc_KeyError,
clp_ffi_py::cSetstateKeyErrorTemplate,
get_c_str_from_constexpr_string_view(clp_ffi_py::cSetstateKeyErrorTemplate),
cStateSearchTimeTerminationMargin
);
return nullptr;
Expand Down Expand Up @@ -377,7 +390,7 @@ PyDoc_STRVAR(

auto PyQuery_match_log_event(PyQuery* self, PyObject* log_event) -> PyObject* {
if (false == static_cast<bool>(PyObject_TypeCheck(log_event, PyLogEvent::get_py_type()))) {
PyErr_SetString(PyExc_TypeError, cPyTypeError);
PyErr_SetString(PyExc_TypeError, get_c_str_from_constexpr_string_view(cPyTypeError));
return nullptr;
}
auto* py_log_event{py_reinterpret_cast<PyLogEvent>(log_event)};
Expand Down Expand Up @@ -620,15 +633,18 @@ auto PyQuery::init(
wildcard_queries,
search_time_termination_margin
);
if (nullptr == m_query) {
PyErr_SetString(
PyExc_RuntimeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cOutOfMemoryError)
);
return false;
}
} catch (clp_ffi_py::ExceptionFFI& ex) {
handle_traceable_exception(ex);
m_query = nullptr;
return false;
}
if (nullptr == m_query) {
PyErr_SetString(PyExc_RuntimeError, clp_ffi_py::cOutofMemoryError);
return false;
}
return true;
}

Expand Down
6 changes: 5 additions & 1 deletion src/clp_ffi_py/ir/native/PySerializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <array>
#include <cstddef>
#include <cstdint>
#include <new>
#include <optional>
#include <span>
#include <type_traits>
Expand Down Expand Up @@ -403,7 +404,10 @@ auto PySerializer::init(
m_serializer = new PySerializer::ClpIrSerializer{std::move(serializer)};
m_buffer_size_limit = buffer_size_limit;
if (nullptr == m_serializer) {
PyErr_SetString(PyExc_RuntimeError, clp_ffi_py::cOutofMemoryError);
PyErr_SetString(
PyExc_RuntimeError,
get_c_str_from_constexpr_string_view(clp_ffi_py::cOutOfMemoryError)
);
return false;
}
auto const preamble_size{get_ir_buf_size()};
Expand Down
4 changes: 2 additions & 2 deletions src/clp_ffi_py/ir/native/deserialization_methods.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ auto deserialize_preamble(PyObject* Py_UNUSED(self), PyObject* py_deserializer_b
PyObject_TypeCheck(py_deserializer_buffer, PyDeserializerBuffer::get_py_type())
))
{
PyErr_SetString(PyExc_TypeError, cPyTypeError);
PyErr_SetString(PyExc_TypeError, get_c_str_from_constexpr_string_view(cPyTypeError));
return nullptr;
}

Expand Down Expand Up @@ -329,7 +329,7 @@ auto deserialize_next_log_event(PyObject* Py_UNUSED(self), PyObject* args, PyObj
if (is_query_given
&& false == static_cast<bool>(PyObject_TypeCheck(query_obj, PyQuery::get_py_type())))
{
PyErr_SetString(PyExc_TypeError, cPyTypeError);
PyErr_SetString(PyExc_TypeError, get_c_str_from_constexpr_string_view(cPyTypeError));
return nullptr;
}

Expand Down
Loading

0 comments on commit b1cbedb

Please sign in to comment.