-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: separate the concept of captures and tags; lexer now tracks mapping from variables to capture to tags to registers. #72
base: main
Are you sure you want to change the base?
Changes from 250 commits
08b7548
449133e
7b837bf
f0eb56b
a945915
c757ded
2eb7477
08060ed
0c2c1d1
b0b951a
3c2a2ab
98c5b95
f59cf41
85a2d69
4c602d4
2b01433
e37b29a
c5beca3
fe4a7b3
8993088
aaf720a
0017512
336f2ae
3449df2
ef62df1
a085650
2be06c0
9ec01dd
019e675
83a411a
c88fbb5
7c91ddc
4c50769
98200b4
afaf01a
8dea476
dbb1e16
e054825
792ce96
cab6e81
ffda5e6
ff11672
536b50b
77c20f7
f43759c
01e8881
fbb3d36
e1f2b18
a51b49d
002577e
52a155c
a6beafc
ec1f757
f35741f
e8e5e55
f1ece30
08997ae
0910c62
ede680e
c7b047c
6fa8fcb
18b9160
3f08fa3
e281f04
a03734e
9123c7a
fe35fe0
de58e08
1426179
3301f14
c9b1369
e2aee66
36c1810
48df8b0
a8605fc
15cb1b6
6b787d0
cd8f4e3
72da50c
3fc7ea7
6443d66
63aec4d
295f3ee
1186666
06ee38e
e103011
31b0346
4005e41
e38940c
d71368d
dd4b6e1
7322852
dba1a18
ee6efab
9ba980c
27b324c
f651a24
fc6f426
c8fb570
1f66918
a388c80
340eaf7
22cf931
e75c888
d5b20fe
69a7ad1
0b7ef3c
ada697f
4e08e2c
5916d33
c61f2d9
417bde8
738876d
93c03a0
6481e5f
6a9a4a4
052d86f
ed70bd5
fab801f
db84cb7
7f6fcd9
a346104
e90ae14
4d30509
1671e39
748dfc5
8abf35a
1e5fdcc
effac53
43aa3be
c0920bf
19fe130
61ceba6
0e1e2b2
9302ecb
71d926d
66ed13b
a12a360
244d122
176391b
012f61f
96a6363
7cd39f0
a4a93b4
421c3de
ecd4e4e
1b945a1
70cd43d
78c4125
3ce0b30
33623fa
bae0557
5bbeafc
0decaf5
0e2d593
cf5980d
6e65a3e
9c2ad81
c4fc96b
a6bbaef
ff2dac3
7a8982d
7b0a86c
2632616
992a2ec
a5419f0
ae64f64
9153a7c
fa0c098
845b088
2dd6f45
ee46719
4cebc06
827d39c
80979de
b38b6d7
1c2d7b3
11da5d5
3f6966f
11eab2b
538ae8b
74841b0
f84cd0b
efb7932
be3d483
b8afafa
68ed2b1
8bebee4
beb59aa
b4d25bb
4e07f8d
8f7a355
fe3b704
1380e53
6d5b920
9a82620
8abd2cf
a509323
6bf8c01
5dc5953
ed631b3
8410fb1
ff92325
b26d22b
9af72f3
6f3ce6c
1f1d3ea
d21e373
66cd3b3
ea78ed2
25237a2
3a94829
9902984
7ccdfd5
013997c
88e2cc2
4ebbc82
e6893d9
425a8e6
fd394af
7fe39db
6cb5c23
239872a
8bd9bae
473f8e5
626ad54
0fb0758
f7e1718
18c3e25
af2c8d4
3504ffb
8b43e83
719e722
082145e
e06cf44
2e99a91
94db0d6
9731fa3
bc78c49
2c89216
f06031d
90ce695
40a90bd
d9c20fe
391607d
e6273b2
f88a780
0cc6c24
54d9181
d38393d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,6 +15,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
#include <log_surgeon/finite_automata/DfaState.hpp> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
#include <log_surgeon/finite_automata/Nfa.hpp> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
#include <log_surgeon/finite_automata/RegexAST.hpp> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
#include <log_surgeon/finite_automata/RegisterHandler.hpp> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
#include <log_surgeon/LexicalRule.hpp> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
#include <log_surgeon/ParserInputBuffer.hpp> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
#include <log_surgeon/Token.hpp> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -23,6 +24,10 @@ namespace log_surgeon { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
template <typename TypedNfaState, typename TypedDfaState> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
class Lexer { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
public: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
using register_id_t = finite_automata::RegisterHandler::register_id_t; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
using symbol_id_t = uint32_t; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
using tag_id_t = finite_automata::tag_id_t; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
static inline std::vector<uint32_t> const cTokenEndTypes = {(uint32_t)SymbolId::TokenEnd}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
static inline std::vector<uint32_t> const cTokenUncaughtStringTypes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
= {(uint32_t)SymbolId::TokenUncaughtString}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -51,7 +56,8 @@ class Lexer { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
auto get_rule(uint32_t variable_id) -> finite_automata::RegexAST<TypedNfaState>*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Generate DFA for lexer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Generate DFA for lexer. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @throw std::invalid_argument if `m_rules` contains multipe captures with the same name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto generate() -> void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -122,8 +128,48 @@ class Lexer { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
return m_dfa; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::unordered_map<std::string, uint32_t> m_symbol_id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::unordered_map<uint32_t, std::string> m_id_symbol; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
[[nodiscard]] auto get_capture_ids_for_var_id(symbol_id_t const var_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about renaming to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add doc strings for these newly added functions for:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
) const -> std::optional<std::vector<symbol_id_t>> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto const capture_ids{m_var_id_to_capture_ids.find(var_id)}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (m_var_id_to_capture_ids.end() == capture_ids) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return std::nullopt; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return capture_ids->second; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+133
to
+137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
How about rewriting in this way to avoid using iterators (shouldn't have any performance difference). |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
[[nodiscard]] auto get_tag_ids_for_capture_id(symbol_id_t const capture_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
) const -> std::optional<std::pair<tag_id_t, tag_id_t>> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Headers for |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto const tag_ids{m_capture_id_to_tag_ids.find(capture_id)}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (m_capture_id_to_tag_ids.end() == tag_ids) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return std::nullopt; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return tag_ids->second; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
[[nodiscard]] auto get_register_for_tag_id(tag_id_t const tag_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
) const -> std::optional<register_id_t> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto const it{m_tag_to_register_id.find(tag_id)}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (m_tag_to_register_id.end() == it) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return std::nullopt; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return it->second; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
[[nodiscard]] auto get_registers_for_capture(symbol_id_t capture_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
) const -> std::optional<std::pair<register_id_t, register_id_t>> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto const tag_ids{get_tag_ids_for_capture_id(capture_id)}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (tag_ids.has_value()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto const start_reg{get_register_for_tag_id(tag_ids.value().first())}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto const end_reg{get_register_for_tag_id(tag_ids.value().second())}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (start_reg.has_value() && end_reg.has_value()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return std::make_pair(start_reg.value(), end_reg.value()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return std::nullopt; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+160
to
+168
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
How about rewriting in this way to:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::unordered_map<std::string, symbol_id_t> m_symbol_id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::unordered_map<symbol_id_t, std::string> m_id_symbol; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+171
to
+172
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is out of this PR's scope:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, these are relic's of the old code, I can create an issue for it unless its covered in the clang-tidy issue. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
private: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -148,6 +194,9 @@ class Lexer { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
std::unique_ptr<finite_automata::Dfa<TypedDfaState>> m_dfa; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
bool m_asked_for_more_data{false}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
TypedDfaState const* m_prev_state{nullptr}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::unordered_map<symbol_id_t, std::vector<symbol_id_t>> m_var_id_to_capture_ids; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::unordered_map<symbol_id_t, std::pair<tag_id_t, tag_id_t>> m_capture_id_to_tag_ids; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::unordered_map<tag_id_t, register_id_t> m_tag_to_register_id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
namespace lexers { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,6 +4,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||
#include <cassert> | ||||||||||||||||||||||||||||||||||||||||||||||
#include <memory> | ||||||||||||||||||||||||||||||||||||||||||||||
#include <stack> | ||||||||||||||||||||||||||||||||||||||||||||||
#include <stdexcept> | ||||||||||||||||||||||||||||||||||||||||||||||
#include <string> | ||||||||||||||||||||||||||||||||||||||||||||||
#include <vector> | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -358,17 +359,17 @@ void Lexer<TypedNfaState, TypedDfaState>::add_delimiters(std::vector<uint32_t> c | |||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
template <typename TypedNfaState, typename TypedDfaState> | ||||||||||||||||||||||||||||||||||||||||||||||
void Lexer<TypedNfaState, TypedDfaState>::add_rule( | ||||||||||||||||||||||||||||||||||||||||||||||
uint32_t const& id, | ||||||||||||||||||||||||||||||||||||||||||||||
symbol_id_t const& var_id, | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||||||||||||||||||||||||
std::unique_ptr<finite_automata::RegexAST<TypedNfaState>> rule | ||||||||||||||||||||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||||||||||||||||||||
m_rules.emplace_back(id, std::move(rule)); | ||||||||||||||||||||||||||||||||||||||||||||||
m_rules.emplace_back(var_id, std::move(rule)); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
template <typename TypedNfaState, typename TypedDfaState> | ||||||||||||||||||||||||||||||||||||||||||||||
auto Lexer<TypedNfaState, TypedDfaState>::get_rule(uint32_t const variable_id | ||||||||||||||||||||||||||||||||||||||||||||||
auto Lexer<TypedNfaState, TypedDfaState>::get_rule(symbol_id_t const var_id | ||||||||||||||||||||||||||||||||||||||||||||||
) -> finite_automata::RegexAST<TypedNfaState>* { | ||||||||||||||||||||||||||||||||||||||||||||||
for (auto const& rule : m_rules) { | ||||||||||||||||||||||||||||||||||||||||||||||
if (rule.get_variable_id() == variable_id) { | ||||||||||||||||||||||||||||||||||||||||||||||
if (rule.get_variable_id() == var_id) { | ||||||||||||||||||||||||||||||||||||||||||||||
return rule.get_regex(); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -377,8 +378,30 @@ auto Lexer<TypedNfaState, TypedDfaState>::get_rule(uint32_t const variable_id | |||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
template <typename TypedNfaState, typename TypedDfaState> | ||||||||||||||||||||||||||||||||||||||||||||||
void Lexer<TypedNfaState, TypedDfaState>::generate() { | ||||||||||||||||||||||||||||||||||||||||||||||
finite_automata::Nfa<TypedNfaState> nfa{std::move(m_rules)}; | ||||||||||||||||||||||||||||||||||||||||||||||
// TODO: DFA ignores tags. E.g., treats "capture:user=(?<user_id>\d+)" as "capture:user=\d+" | ||||||||||||||||||||||||||||||||||||||||||||||
for (auto const& rule : m_rules) { | ||||||||||||||||||||||||||||||||||||||||||||||
for (auto* capture : rule.get_captures()) { | ||||||||||||||||||||||||||||||||||||||||||||||
std::string const capture_name{capture->get_name()}; | ||||||||||||||||||||||||||||||||||||||||||||||
symbol_id_t capture_id{0}; | ||||||||||||||||||||||||||||||||||||||||||||||
if (m_symbol_id.find(capture_name) == m_symbol_id.end()) { | ||||||||||||||||||||||||||||||||||||||||||||||
capture_id = m_symbol_id.size(); | ||||||||||||||||||||||||||||||||||||||||||||||
m_symbol_id[capture_name] = capture_id; | ||||||||||||||||||||||||||||||||||||||||||||||
m_id_symbol[capture_id] = capture_name; | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+387
to
+388
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is disallowed to use
Comment on lines
+387
to
+388
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is very confusing: I don't really understand how
|
||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||
throw std::invalid_argument("`m_rules` contains capture names that are not unique." | ||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
m_var_id_to_capture_ids[rule.get_variable_id()].push_back(capture_id); | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+384
to
+393
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think we can rewrite in this way. The key change here (as well as the other suggestions) is to short-circuit the error case in a stand-alone if check, and leave the regular logic outside of any inner-if-else statements. |
||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
finite_automata::Nfa<TypedNfaState> nfa{m_rules}; | ||||||||||||||||||||||||||||||||||||||||||||||
for (auto const& [capture, tag_ids] : nfa.get_capture_to_tag_ids()) { | ||||||||||||||||||||||||||||||||||||||||||||||
std::string capture_name{capture->get_name()}; | ||||||||||||||||||||||||||||||||||||||||||||||
auto capture_id{m_symbol_id[capture_name]}; | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+399
to
+400
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Note: if we change the mapping (currently
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
m_capture_id_to_tag_ids.emplace(capture_id, tag_ids); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
// TODO: DFA ignores captures. E.g., treats "capture:user=(?<user_id>\d+)" as "capture:user=\d+" | ||||||||||||||||||||||||||||||||||||||||||||||
m_dfa = std::make_unique<finite_automata::Dfa<TypedDfaState>>(std::move(nfa)); | ||||||||||||||||||||||||||||||||||||||||||||||
auto const* state = m_dfa->get_root(); | ||||||||||||||||||||||||||||||||||||||||||||||
for (uint32_t i = 0; i < cSizeOfByte; i++) { | ||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,10 @@ class LexicalRule { | |
*/ | ||
auto add_to_nfa(finite_automata::Nfa<TypedNfaState>* nfa) const -> void; | ||
|
||
[[nodiscard]] auto get_captures() const -> std::vector<finite_automata::Capture const*> { | ||
return m_regex->get_subtree_positive_captures(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
SharafMohamed marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
[[nodiscard]] auto get_variable_id() const -> uint32_t { return m_variable_id; } | ||
|
||
[[nodiscard]] auto get_regex() const -> finite_automata::RegexAST<TypedNfaState>* { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file seems to be irrelevant to this PR. Can we defer it to the future PRs? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move these type definitions to one header?
The problem with aliasing here is: if we decide to change the name of one symbol, for example,
register_id_t
->reg_id_t
, it takes more effort to apply to all places since it's been aliasedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I swapped to in future PRs is introducing the aliases in the
log_surgeon::finite_automata
namespace. This way if a file needs to use register's, it'll also have access toregister_id_t
. I feel like this is similar to moving them into a single header, while keeping the definition inside the file that its most relevent to.Does this make sense, or am I missing what you're trying to say here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem is do we really need aliasing for register ID type? Like can we just have
register_id_t
defined in one place and everyone else will use it directly from one include. I'm fine with leaving the other two as they are in this PR.