[k2] refactor pcre2 functions #1501
Conversation
…atched_as_null> dump_matches
| #include <optional> | ||
| #include <string_view> | ||
|
|
||
| #include "runtime-light/stdlib/diagnostics/logs.h" |
There was a problem hiding this comment.
Please, remove unused includes and add ones you actually use
| } | ||
|
|
||
| private: | ||
| const PCRE2_UCHAR8* m_ptr; |
There was a problem hiding this comment.
nit: let's move all member declarations to the a type declaration's beginning
| using value_type = group_name; | ||
| using difference_type = std::ptrdiff_t; | ||
| using pointer = group_name*; | ||
| using reference = group_name; |
There was a problem hiding this comment.
Should here be group_name&?
|
|
||
| group_name operator*() const noexcept { | ||
| const auto index{static_cast<size_t>(m_ptr[0] << 8 | m_ptr[1])}; | ||
| const auto* name_ptr{reinterpret_cast<const char*>(std::next(m_ptr, 2))}; |
There was a problem hiding this comment.
nit: can we name this constants somehow to show what offsets they represent?
| return *this; | ||
| } | ||
|
|
||
| group_name_iterator operator++(int) noexcept { |
There was a problem hiding this comment.
Let's add // NOLINT here
|
|
||
| struct group_name { | ||
| std::string_view name; | ||
| size_t index; |
There was a problem hiding this comment.
Let's add default initializers to all members of primitive types
| size_t& buffer_len, uint32_t match_options, pcre2_match_context_8* ctx = nullptr) const noexcept; | ||
|
|
||
| private: | ||
| explicit regex(pcre2_code_8& code) |
There was a problem hiding this comment.
- I think it's safer to pass
regex_pcre2_code_there. Thus, it's explicit that we accept a managed object - Please, add
noexceptto all functions
| private: | ||
| explicit regex(pcre2_code_8& code) | ||
| : m_code{std::addressof(code), pcre2_code_free_8} {} | ||
| regex_pcre2_code_t m_code; |
There was a problem hiding this comment.
I wonder if it's better for readability to move those types into kphp::pcre2 namespace. For example, it could be kphp::pcre2::code
| public: | ||
| friend class matcher; | ||
|
|
||
| static std::expected<regex, compile_error> compile(std::string_view pattern, uint32_t options = 0, pcre2_compile_context_8* ctx = nullptr) noexcept; |
There was a problem hiding this comment.
I think we don't need the ctx = nullptr case
In this PR I'm adding pcre2-functions.h and pcre2-functions.cpp, into which I've extracted all PCRE2 specifics.