refactor: Use string_view in instructions and diagnostics #226
Conversation
slavek-kucera
force-pushed
the
refactor_instructions
branch
from
ee4e2d3 to
88b1328
Compare
|
SonarCloud Quality Gate failed.
|
| @@ -1123,7 +1125,9 @@ bool cattr::check(const std::vector<const asm_operand*>& to_check, | |||
| else if (auto complex_op = get_complex_operand(operand); complex_op) | |||
| { | |||
| // operand is complex | |||
| const static std::vector<std::string> complex_operands = { "RMODE", "ALIGN", "FILL", "PART", "PRIORITY" }; | |||
| const static std::vector<std::string_view> complex_operands = { | |||
| "RMODE", "ALIGN", "FILL", "PART", "PRIORITY" | |||
There was a problem hiding this comment.
Consider having e.g. each operand on a separate line to maintain the same look and feel as on lines 1102 - 1112
| "RMODE", "ALIGN", "FILL", "PART", "PRIORITY" | |
| "RMODE", | |
| "ALIGN", | |
| "FILL", | |
| "PART", | |
| "PRIORITY" |
There was a problem hiding this comment.
This is really handled by clang-format. While it can be forced to the string-per-line format by attaching , to the last argument, I actually prefer these lists to be as short as possible.
| std::vector<label_types> allowed_types; | ||
| std::string_view name_of_instruction; | ||
| int min_operands = 0; | ||
| int max_operands = 0; // maximum number of operands, if not specified, value is -1 |
There was a problem hiding this comment.
It seems to me that the comment is no longer accurate
| { | ||
| std::string name; | ||
| bool operandless; | ||
| inline_string<6> m_name; |
There was a problem hiding this comment.
I don't really get why exactly 6 chars are needed here. Consider adding a comment about where this number comes from and also consider creating a separate constant for this instead of using magic numbers.
Nevertheless, keep in mind that I don't know the context yet and the numbers might be completely clear to someone with more experience.
The same comment applies also to line 371 and 376.
There was a problem hiding this comment.
These constants are the smallest ones that can be currently used. The various instructions are now instantiated in constexpr so if the value is too small, the compilation will simply fail.
|
|
||
| reladdr_transform_mask reladdr_mask; | ||
| // Generates a bitmask for an arbitrary mnemonit indicating which operands |
There was a problem hiding this comment.
Typo: mnemonit -> mnemonic
| const diagnostic_collector& add_diagnostic) const | ||
| { | ||
| const static std::vector<std::string> other_pair_options = { | ||
| const static std::vector<std::string_view> other_pair_options = { | ||
| "NOPUSH", "NORECORD", "NOPU", "NORC", "PUSH", "RECORD", "PU", "RC" |
There was a problem hiding this comment.
Consider having e.g. each operand on a separate line to maintain the same look and feel as on lines 90 - 109
| const std::string, | ||
| const diagnostic_collector& add_diagnostic) const | ||
| { | ||
| const static std::vector<std::string> typecheck_operands = { | ||
| const static std::vector<std::string_view> typecheck_operands = { | ||
| "MAGNITUDE", "REGISTER", "MAG", "REG", "NOMAGNITUDE", "NOREGISTER", "NOMAG", "NOREG" |
There was a problem hiding this comment.
Consider having e.g. each operand on a separate line to maintain the same look and feel as on lines 90 - 109
parser_library/src/diagnostic.cpp
Outdated
| template<typename T> | ||
| std::enable_if_t<!std::is_convertible_v<T&&, std::string_view>, size_t> len(T&&) const | ||
| { | ||
| return 8; |
There was a problem hiding this comment.
Consider adding a comment why 8 is returned
|
SonarCloud Quality Gate failed.
|
|
🎉 This PR is included in version 1.1.0-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.