Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang-tidy fix clp::ffi namespace top level folder files and related test cases #509

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions components/core/src/clp/TraceableException.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class TraceableException : public std::exception {
// Macros
// Define a version of __FILE__ that's relative to the source directory
#ifdef SOURCE_PATH_SIZE
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
#define __FILENAME__ ((__FILE__) + SOURCE_PATH_SIZE)
#else
// We don't know the source path size, so just default to __FILE__
Expand Down
68 changes: 68 additions & 0 deletions components/core/src/clp/ffi/defs.hpp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about constants.hpp?

Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#ifndef CLP_FFI_DEFS_HPP
#define CLP_FFI_DEFS_HPP

#include <cstddef>
#include <cstdint>
#include <string>
#include <string_view>
#include <utility>

#include "../ErrorCode.hpp"
#include "../TraceableException.hpp"

namespace clp::ffi {
/*
* These constants can be used by callers to store the version of the schemas and encoding methods
* they're using. At some point, we may update and/or add built-in schemas/encoding methods. So
* callers must store the versions they used for encoding to ensure that they can choose the same
* versions for decoding.
*
* We use versions which look like package names in anticipation of users writing their own custom
* schemas and encoding methods.
*/
constexpr std::string_view cVariableEncodingMethodsVersion
= "com.yscope.clp.VariableEncodingMethodsV1";
constexpr std::string_view cVariablesSchemaVersion = "com.yscope.clp.VariablesSchemaV2";

constexpr std::string_view cTooFewDictionaryVarsErrorMessage
= "There are fewer dictionary variables than dictionary variable placeholders in the "
"logtype.";
constexpr std::string_view cTooFewEncodedVarsErrorMessage
= "There are fewer encoded variables than encoded variable placeholders in the logtype.";
constexpr std::string_view cUnexpectedEscapeCharacterMessage
= "Unexpected escape character without escaped value at the end of the logtype.";
constexpr std::string_view cTooManyDigitsErrorMsg = "Encoded number of digits doesn't match "
"encoded digits in encoded float.";

constexpr size_t cMaxDigitsInRepresentableEightByteFloatVar = 16;
constexpr size_t cMaxDigitsInRepresentableFourByteFloatVar = 8;
constexpr uint64_t cEightByteEncodedFloatNumDigits = 54;
constexpr uint64_t cFourByteEncodedFloatNumDigits = 25;
constexpr uint64_t cEightByteEncodedFloatDigitsBitMask = (1ULL << 54) - 1;
constexpr uint32_t cFourByteEncodedFloatDigitsBitMask = (1UL << 25) - 1;

constexpr size_t cDecimalBase = 10;
constexpr uint32_t cLowerFourDigitsBitMask = (1UL << 4) - 1;
constexpr uint32_t cLowerThreeDigitsBitMask = (1UL << 3) - 1;
Comment on lines +14 to +46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use {} for initialization instead?


class EncodingException : public TraceableException {
public:
// Constructors
EncodingException(
ErrorCode error_code,
char const* const filename,
int line_number,
std::string message
)
: TraceableException(error_code, filename, line_number),
m_message(std::move(message)) {}
Comment on lines +57 to +58
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
: TraceableException(error_code, filename, line_number),
m_message(std::move(message)) {}
: TraceableException{error_code, filename, line_number},
m_message{std::move(message)} {}


// Methods
[[nodiscard]] auto what() const noexcept -> char const* override { return m_message.c_str(); }

private:
std::string m_message;
};
Comment on lines +48 to +65
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about moving this class to a dedicated header?

} // namespace clp::ffi

#endif // CLP_FFI_DEFS_HPP
12 changes: 5 additions & 7 deletions components/core/src/clp/ffi/encoding_methods.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "encoding_methods.hpp"

#include <algorithm>
#include <cstdint>
#include <string_view>

#include "../ir/types.hpp"
Expand All @@ -10,9 +10,8 @@ using clp::ir::four_byte_encoded_variable_t;
using std::string_view;

namespace clp::ffi {
eight_byte_encoded_variable_t encode_four_byte_float_as_eight_byte(
four_byte_encoded_variable_t four_byte_encoded_var
) {
auto encode_four_byte_float_as_eight_byte(four_byte_encoded_variable_t four_byte_encoded_var
) -> eight_byte_encoded_variable_t {
uint8_t decimal_point_pos{};
uint8_t num_digits{};
uint32_t digits{};
Expand All @@ -33,9 +32,8 @@ eight_byte_encoded_variable_t encode_four_byte_float_as_eight_byte(
);
}

eight_byte_encoded_variable_t encode_four_byte_integer_as_eight_byte(
four_byte_encoded_variable_t four_byte_encoded_var
) {
auto encode_four_byte_integer_as_eight_byte(four_byte_encoded_variable_t four_byte_encoded_var
) -> eight_byte_encoded_variable_t {
return static_cast<eight_byte_encoded_variable_t>(four_byte_encoded_var);
}
} // namespace clp::ffi
117 changes: 35 additions & 82 deletions components/core/src/clp/ffi/encoding_methods.hpp
Original file line number Diff line number Diff line change
@@ -1,62 +1,19 @@
#ifndef CLP_FFI_ENCODING_METHODS_HPP
#define CLP_FFI_ENCODING_METHODS_HPP

#include <cstddef>
#include <cstdint>
#include <span>
#include <string>
#include <string_view>
#include <type_traits>
#include <vector>

#include "../ir/parsing.hpp"
#include "../ir/types.hpp"
#include "../TraceableException.hpp"

// TODO Some of the methods in this file are mostly duplicated from code that exists elsewhere in
// the repo. They should be consolidated in a future commit.
namespace clp::ffi {
class EncodingException : public TraceableException {
public:
// Constructors
EncodingException(
ErrorCode error_code,
char const* const filename,
int line_number,
std::string message
)
: TraceableException(error_code, filename, line_number),
m_message(std::move(message)) {}

// Methods
[[nodiscard]] char const* what() const noexcept override { return m_message.c_str(); }

private:
std::string m_message;
};

// Constants
/*
* These constants can be used by callers to store the version of the schemas and encoding methods
* they're using. At some point, we may update and/or add built-in schemas/encoding methods. So
* callers must store the versions they used for encoding to ensure that they can choose the same
* versions for decoding.
*
* We use versions which look like package names in anticipation of users writing their own custom
* schemas and encoding methods.
*/
static constexpr char cVariableEncodingMethodsVersion[]
= "com.yscope.clp.VariableEncodingMethodsV1";
static constexpr char cVariablesSchemaVersion[] = "com.yscope.clp.VariablesSchemaV2";

static constexpr char cTooFewDictionaryVarsErrorMessage[]
= "There are fewer dictionary variables than dictionary variable placeholders in the "
"logtype.";
static constexpr char cTooFewEncodedVarsErrorMessage[]
= "There are fewer encoded variables than encoded variable placeholders in the logtype.";
static constexpr char cUnexpectedEscapeCharacterMessage[]
= "Unexpected escape character without escaped value at the end of the logtype.";

constexpr size_t cMaxDigitsInRepresentableEightByteFloatVar = 16;
constexpr size_t cMaxDigitsInRepresentableFourByteFloatVar = 8;
constexpr uint64_t cEightByteEncodedFloatDigitsBitMask = (1ULL << 54) - 1;
constexpr uint32_t cFourByteEncodedFloatDigitsBitMask = (1UL << 25) - 1;

/**
* Encodes the given string into a representable float variable if possible
* @tparam encoded_variable_t Type of the encoded variable
Expand All @@ -65,16 +22,17 @@ constexpr uint32_t cFourByteEncodedFloatDigitsBitMask = (1UL << 25) - 1;
* @return true on success, false otherwise
*/
template <typename encoded_variable_t>
bool encode_float_string(std::string_view str, encoded_variable_t& encoded_var);
[[nodiscard]] auto
encode_float_string(std::string_view str, encoded_variable_t& encoded_var) -> bool;

/**
* Encodes the given four-byte encoded float using the eight-byte encoding
* @param four_byte_encoded_var
* @return The float using the eight-byte encoding
*/
ir::eight_byte_encoded_variable_t encode_four_byte_float_as_eight_byte(
[[nodiscard]] auto encode_four_byte_float_as_eight_byte(
ir::four_byte_encoded_variable_t four_byte_encoded_var
);
) -> ir::eight_byte_encoded_variable_t;

/**
* Encodes a float value with the given properties into an encoded variable.
Expand All @@ -87,15 +45,15 @@ ir::eight_byte_encoded_variable_t encode_four_byte_float_as_eight_byte(
* @return The encoded variable
*/
template <typename encoded_variable_t>
encoded_variable_t encode_float_properties(
[[nodiscard]] auto encode_float_properties(
bool is_negative,
std::conditional_t<
std::is_same_v<encoded_variable_t, ir::four_byte_encoded_variable_t>,
uint32_t,
uint64_t> digits,
size_t num_digits,
size_t decimal_point_pos
);
) -> encoded_variable_t;

/**
* Decodes an encoded float variable into its properties
Expand All @@ -107,7 +65,7 @@ encoded_variable_t encode_float_properties(
* @param decimal_point_pos Returns the position of the decimal point from the right of the value
*/
template <typename encoded_variable_t>
void decode_float_properties(
auto decode_float_properties(
encoded_variable_t encoded_var,
bool& is_negative,
std::conditional_t<
Expand All @@ -116,7 +74,7 @@ void decode_float_properties(
uint64_t>& digits,
uint8_t& num_digits,
uint8_t& decimal_point_pos
);
) -> void;

/**
* Decodes the given encoded float variable into a string
Expand All @@ -125,7 +83,7 @@ void decode_float_properties(
* @return The decoded value as a string
*/
template <typename encoded_variable_t>
std::string decode_float_var(encoded_variable_t encoded_var);
[[nodiscard]] auto decode_float_var(encoded_variable_t encoded_var) -> std::string;

/**
* Encodes the given string into a representable integer variable if possible
Expand All @@ -135,16 +93,17 @@ std::string decode_float_var(encoded_variable_t encoded_var);
* @return true if successfully converted, false otherwise
*/
template <typename encoded_variable_t>
bool encode_integer_string(std::string_view str, encoded_variable_t& encoded_var);
[[nodiscard]] auto
encode_integer_string(std::string_view str, encoded_variable_t& encoded_var) -> bool;

/**
* Encodes the given four-byte encoded integer using the eight-byte encoding
* @param four_byte_encoded_var
* @return The integer using the eight-byte encoding
*/
ir::eight_byte_encoded_variable_t encode_four_byte_integer_as_eight_byte(
[[nodiscard]] auto encode_four_byte_integer_as_eight_byte(
ir::four_byte_encoded_variable_t four_byte_encoded_var
);
) -> ir::eight_byte_encoded_variable_t;

/**
* Decodes the given encoded integer variable into a string
Expand All @@ -153,7 +112,7 @@ ir::eight_byte_encoded_variable_t encode_four_byte_integer_as_eight_byte(
* @return The decoded value as a string
*/
template <typename encoded_variable_t>
std::string decode_integer_var(encoded_variable_t encoded_var);
[[nodiscard]] auto decode_integer_var(encoded_variable_t encoded_var) -> std::string;

/**
* Encodes the given message and calls the given methods to handle specific components of the
Expand All @@ -177,13 +136,13 @@ template <
typename ConstantHandler,
typename EncodedVariableHandler,
typename DictionaryVariableHandler>
bool encode_message_generically(
[[nodiscard]] auto encode_message_generically(
std::string_view message,
std::string& logtype,
ConstantHandler constant_handler,
EncodedVariableHandler encoded_variable_handler,
DictionaryVariableHandler dictionary_variable_handler
);
) -> bool;

/**
* Encodes the given message. The simplistic interface is to make it efficient to transfer data
Expand All @@ -197,12 +156,12 @@ bool encode_message_generically(
* @return false if the message contains variable placeholders, true otherwise
*/
template <typename encoded_variable_t>
bool encode_message(
[[nodiscard]] auto encode_message(
std::string_view message,
std::string& logtype,
std::vector<encoded_variable_t>& encoded_vars,
std::vector<int32_t>& dictionary_var_bounds
);
) -> bool;

/**
* Decodes the message from the given logtype, encoded variables, and dictionary variables. The
Expand All @@ -211,23 +170,19 @@ bool encode_message(
* @tparam encoded_variable_t Type of the encoded variable
* @param logtype
* @param encoded_vars
* @param encoded_vars_length
* @param all_dictionary_vars The message's dictionary variables, stored back-to-back in a single
* byte-array
* @param dictionary_var_end_offsets The end-offset of each dictionary variable in
* ``all_dictionary_vars``
* @param dictionary_var_end_offsets_length
* @return The decoded message
*/
template <typename encoded_variable_t>
std::string decode_message(
[[nodiscard]] auto decode_message(
std::string_view logtype,
encoded_variable_t* encoded_vars,
size_t encoded_vars_length,
std::span<encoded_variable_t> encoded_vars,
std::string_view all_dictionary_vars,
int32_t const* dictionary_var_end_offsets,
size_t dictionary_var_end_offsets_length
);
std::span<int32_t const> dictionary_var_end_offsets
) -> std::string;

/**
* Checks if any encoded variable matches the given wildcard query
Expand All @@ -239,16 +194,14 @@ std::string decode_message(
* @param wildcard_query
* @param logtype
* @param encoded_vars
* @param encoded_vars_length
* @return true if a match was found, false otherwise
*/
template <ir::VariablePlaceholder var_placeholder, typename encoded_variable_t>
bool wildcard_query_matches_any_encoded_var(
[[nodiscard]] auto wildcard_query_matches_any_encoded_var(
std::string_view wildcard_query,
std::string_view logtype,
encoded_variable_t* encoded_vars,
size_t encoded_vars_length
);
std::span<encoded_variable_t> encoded_vars
) -> bool;

/**
* Checks whether the given wildcard strings match the given encoded variables (from a message).
Expand All @@ -262,7 +215,6 @@ bool wildcard_query_matches_any_encoded_var(
* @tparam encoded_variable_t Type of the encoded variable
* @param logtype The message's logtype
* @param encoded_vars The message's encoded variables
* @param encoded_vars_length The number of encoded variables in \p encoded_vars
* @param wildcard_var_placeholders String of variable placeholders, where each one indicates how
* the corresponding wildcard string should be interpreted.
* @param wildcard_var_queries The wildcard strings to compare with the encoded variables. Callers
Expand All @@ -271,15 +223,16 @@ bool wildcard_query_matches_any_encoded_var(
* @return Whether the wildcard strings match the encoded variables
*/
template <typename encoded_variable_t>
bool wildcard_match_encoded_vars(
[[nodiscard]] auto wildcard_match_encoded_vars(
std::string_view logtype,
encoded_variable_t* encoded_vars,
size_t encoded_vars_length,
std::span<encoded_variable_t> encoded_vars,
std::string_view wildcard_var_placeholders,
std::vector<std::string_view> const& wildcard_var_queries
);
) -> bool;
} // namespace clp::ffi

// TODO Refactor nested headers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO Refactor nested headers
// TODO: Refactor nested headers

Just wanna make sure: you will send a PR immediately after this one to get rid of .inc right?

// NOLINTNEXTLINE(misc-include-cleaner)
#include "encoding_methods.inc"

#endif // CLP_FFI_ENCODING_METHODS_HPP
Loading
Loading