-
Notifications
You must be signed in to change notification settings - Fork 294
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
Drop op_table parameter in analysis() #167
Conversation
Codecov Report
@@ Coverage Diff @@
## master #167 +/- ##
==========================================
- Coverage 85.09% 85.04% -0.06%
==========================================
Files 21 21
Lines 2241 2240 -1
Branches 219 218 -1
==========================================
- Hits 1907 1905 -2
- Misses 307 308 +1
Partials 27 27 |
} | ||
(); | ||
|
||
const auto& op_table = get_op_table(rev); |
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.
Danger of static init fiasco? in case op_tables
in instructions.cpp is initialized after this
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.
The problem does not apply to constexpr
globals, but still I can move it inside the function.
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.
Ah right, haven't noticed constexpr
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.
Good notice anyway.
const exec_fn_table& fns, evmc_revision rev, const uint8_t* code, size_t code_size) noexcept; | ||
evmc_revision rev, const uint8_t* code, size_t code_size) noexcept; | ||
|
||
EVMC_EXPORT const exec_fn_table& get_op_table(evmc_revision rev) noexcept; |
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.
Why do you make it exported?
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.
Used in analysis tests (evmone is usually built as shared library).
I believe the
instruction_table
is more precise name, butop_table
is shorter and might be less mistaken with the table of instructions in the loaded program.